Update TM to latest (+add test) #7442

amaury1093 · 2020-10-02T14:04:14Z

Description

closes: #7401

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

Targeted PR against correct branch (see CONTRIBUTING.md)
Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
Code follows the module structure standards.
Wrote unit and integration tests
Updated relevant documentation (docs/) or specification (x/<module>/spec/)
Added relevant godoc comments.
Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
Re-reviewed Files changed in the Github PR explorer
Review Codecov Report in the comment section below once CI passes

codecov · 2020-10-02T14:10:10Z

Codecov Report

Merging #7442 into master will increase coverage by 1.50%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #7442      +/-   ##
==========================================
+ Coverage   55.54%   57.05%   +1.50%     
==========================================
  Files         440      306     -134     
  Lines       29222    19477    -9745     
==========================================
- Hits        16232    11112    -5120     
+ Misses      11358     7168    -4190     
+ Partials     1632     1197     -435

…401-blockresults

std/codec.go

amaury1093 · 2020-10-05T11:29:21Z

testutil/network/network.go

+// NewAppConstructor returns a new simapp AppConstructor
+func NewAppConstructor(encodingCfg params.EncodingConfig) AppConstructor {
+	return func(val Validator) servertypes.Application {
+		return simapp.NewSimApp(
+			val.Ctx.Logger, dbm.NewMemDB(), nil, true, make(map[int64]bool), val.Ctx.Config.RootDir, 0,
+			encodingCfg,
+			baseapp.SetPruning(storetypes.NewPruningOptionsFromString(val.AppConfig.Pruning)),
+			baseapp.SetMinGasPrices(val.AppConfig.MinGasPrices),
+		)
+	}


just a refactor to avoid creating an additional simapp.MakeEncodingConfig()

aaronc

LGTM

x/staking/client/cli/cli_test.go

…401-blockresults

sahith-narahari

lgtm, pending tests pass

…-sdk into am-7401-blockresults

amaury1093 · 2020-10-07T14:33:01Z

I've restarted the tests 3 times, and this Tests / Code Coverage / test-race-3 (pull_request) always times out. Any ideas?

edit: hmm seems to time out on master too...

robert-zaremba

Good job.

…-sdk into am-7401-blockresults

amaury1093 · 2020-10-08T09:46:09Z

.github/workflows/test.yml

@@ -305,7 +305,7 @@ jobs:
          name: "${{ github.sha }}-ab"
        if: "env.GIT_DIFF != ''"
      - name: Run tests with race detector
-        run: cat xab.txt | xargs go test -mod=readonly -json -timeout 15m -race -tags='cgo ledger test_ledger_mock' > xab-race-output.txt
+        run: cat xab.txt | xargs go test -mod=readonly -json -timeout 30m -race -tags='cgo ledger test_ledger_mock' > xab-race-output.txt


@alessio I'm doubling the timeout here, because:

it often fails on master https://github.com/cosmos/cosmos-sdk/commits/master

when it doesn't fail, it finishes in ~14m, e.g. https://github.com/cosmos/cosmos-sdk/pull/7475/checks?check_run_id=1221058280

There seems to be a test or something else that is consuming a massive amount time. Increasing the timeout to 30m defeats the purpose of splitting tests into multiple jobs (decrease overall time)

Agreed 👍 . I see this more as a band-aid patch, as it's blocking ~3-4 automerge PRs. Unless someone has an idea for a quick fix?

okay! 👍 . If its hanging then 30m wont change it

my hypothesis is that it's not hanging, it passes in some PRs, the test I linked above finished in 14m39s.

~~but anyways, "Cancelled after 15m" https://github.com/cosmos/cosmos-sdk/pull/7442/checks?check_run_id=1225173890#step:7:1 :(~~ Edit: fixed.

it's probably due to the IBC tests. We rely heavily on simapp since it's kinda hard to unit test interactions between different chains and using simapp makes testing a lot more robust. I noticed recently the test times on my machine almost doubled in duration, so I wonder if there was a change to simapp that slowed things down?

the -race flag slows things down a lot. This plus the IBC tests could be the culprit.

colin-axner · 2020-10-08T10:22:49Z

can we automerge? Looks like tests are passing with the timeout bump

amaury1093 · 2020-10-08T10:26:11Z

@colin-axner

GitHub App like Mergify are not allowed to merge pull request where .github/workflows is changed. This pull request must be merged manually.

tac0turtle · 2020-10-08T10:28:24Z

@amaurymartiny want to fix liveness-test? makefile cmds seem to be changing but docker files aren't being updated

colin-axner · 2020-10-08T10:32:51Z

liveness test is failing on master as well. So maybe it is outside the scope of this pr?

tac0turtle · 2020-10-08T10:36:30Z

liveness test is failing on master as well. So maybe it is outside the scope of this pr?

another PR can be made, it's a one word change.

amaury1093 · 2020-10-08T10:39:48Z

Actually @marbar3778, could you do it? I spent 5min looking at Alessio's Makefile PR, have no idea what to change to make liveness pass 😅

amaury1093 · 2020-10-08T10:45:55Z

continuing the test timeout conversation here: #7482

amaury1093 added 2 commits October 2, 2020 16:02

Add test for /block_results

92278b4

Merge branch 'master' into am-7401-blockresults

7380c1d

amaury1093 mentioned this pull request Oct 2, 2020

Block results internal error with validator_updates #7401

Closed

4 tasks

amaury1093 added 6 commits October 2, 2020 16:26

Fix test

4c07e6e

Merge branch 'master' of ssh://github.com/cosmos/cosmos-sdk into am-7…

b383532

…401-blockresults

Use http RPC client

38aed91

Fix temporarily block_results

1edf106

Use init

b8e5cfa

Add flag back

62d920f

amaury1093 commented Oct 5, 2020

View reviewed changes

std/codec.go Outdated Show resolved Hide resolved

Remove init

b9ba9cd

amaury1093 commented Oct 5, 2020

View reviewed changes

Update TM master

9ddcf22

amaury1093 changed the title ~~Add test for /block_results RPC endpoint~~ Update TM & test for /block_results RPC endpoint Oct 5, 2020

Merge branch 'master' into am-7401-blockresults

3a43230

amaury1093 marked this pull request as ready for review October 5, 2020 14:12

amaury1093 requested review from aaronc, alessio and alexanderbez as code owners October 5, 2020 14:12

amaury1093 requested review from anilcse, atheeshp and robert-zaremba October 5, 2020 14:25

amaury1093 changed the title ~~Update TM & test for /block_results RPC endpoint~~ Update TM to latest (+add test) Oct 6, 2020

amaury1093 added the A:automerge Automatically merge PR once all prerequisites pass. label Oct 6, 2020

Merge branch 'master' into am-7401-blockresults

0b8f8c1

aaronc approved these changes Oct 6, 2020

View reviewed changes

Merge branch 'master' into am-7401-blockresults

a904c1b

robert-zaremba reviewed Oct 6, 2020

View reviewed changes

x/staking/client/cli/cli_test.go Outdated Show resolved Hide resolved

x/staking/client/cli/cli_test.go Show resolved Hide resolved

x/staking/client/cli/cli_test.go Show resolved Hide resolved

x/staking/client/cli/cli_test.go Outdated Show resolved Hide resolved

Merge branch 'master' of ssh://github.com/cosmos/cosmos-sdk into am-7…

9e1d19f

…401-blockresults

Address comments

48f10db

amaury1093 requested a review from robert-zaremba October 7, 2020 09:02

amaury1093 added 2 commits October 7, 2020 11:04

Fix build

40482fc

require refactor

fe3f9cf

sahith-narahari approved these changes Oct 7, 2020

View reviewed changes

mergify bot and others added 4 commits October 7, 2020 12:39

Merge branch 'master' into am-7401-blockresults

c4e200e

Merge branch 'master' into am-7401-blockresults

2e87d57

Add build flag back

0aeb0ef

Merge branch 'am-7401-blockresults' of ssh://github.com/cosmos/cosmos…

917fc36

…-sdk into am-7401-blockresults

robert-zaremba approved these changes Oct 7, 2020

View reviewed changes

mergify bot and others added 2 commits October 8, 2020 09:22

Merge branch 'master' into am-7401-blockresults

2352678

More timeout on test

3e1a27b

amaury1093 removed the A:automerge Automatically merge PR once all prerequisites pass. label Oct 8, 2020

Merge branch 'am-7401-blockresults' of ssh://github.com/cosmos/cosmos…

8ba50d0

…-sdk into am-7401-blockresults

amaury1093 commented Oct 8, 2020

View reviewed changes

tac0turtle mentioned this pull request Oct 8, 2020

add ConsensusParams to ClientState #7456

Merged

9 tasks

fix timeout-minutes

5fe0081

amaury1093 force-pushed the am-7401-blockresults branch from 670f2d4 to 5fe0081 Compare October 8, 2020 10:12

amaury1093 added the A:automerge Automatically merge PR once all prerequisites pass. label Oct 8, 2020

amaury1093 mentioned this pull request Oct 8, 2020

Decrease test-race-* timeout back to 15m #7482

Closed

4 tasks

fedekunze merged commit 87e3751 into master Oct 8, 2020

fedekunze deleted the am-7401-blockresults branch October 8, 2020 10:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update TM to latest (+add test) #7442

Update TM to latest (+add test) #7442

amaury1093 commented Oct 2, 2020 •

edited

Loading

codecov bot commented Oct 2, 2020 •

edited

Loading

amaury1093 Oct 5, 2020

aaronc left a comment

sahith-narahari left a comment

amaury1093 commented Oct 7, 2020 •

edited

Loading

robert-zaremba left a comment

amaury1093 Oct 8, 2020

tac0turtle Oct 8, 2020

amaury1093 Oct 8, 2020

tac0turtle Oct 8, 2020 •

edited

Loading

amaury1093 Oct 8, 2020 •

edited

Loading

colin-axner Oct 8, 2020

tac0turtle Oct 8, 2020

colin-axner commented Oct 8, 2020

amaury1093 commented Oct 8, 2020

tac0turtle commented Oct 8, 2020 •

edited

Loading

colin-axner commented Oct 8, 2020

tac0turtle commented Oct 8, 2020

amaury1093 commented Oct 8, 2020

amaury1093 commented Oct 8, 2020

Update TM to latest (+add test) #7442

Update TM to latest (+add test) #7442

Conversation

amaury1093 commented Oct 2, 2020 • edited Loading

Description

codecov bot commented Oct 2, 2020 • edited Loading

Codecov Report

amaury1093 Oct 5, 2020

Choose a reason for hiding this comment

aaronc left a comment

Choose a reason for hiding this comment

sahith-narahari left a comment

Choose a reason for hiding this comment

amaury1093 commented Oct 7, 2020 • edited Loading

robert-zaremba left a comment

Choose a reason for hiding this comment

amaury1093 Oct 8, 2020

Choose a reason for hiding this comment

tac0turtle Oct 8, 2020

Choose a reason for hiding this comment

amaury1093 Oct 8, 2020

Choose a reason for hiding this comment

tac0turtle Oct 8, 2020 • edited Loading

Choose a reason for hiding this comment

amaury1093 Oct 8, 2020 • edited Loading

Choose a reason for hiding this comment

colin-axner Oct 8, 2020

Choose a reason for hiding this comment

tac0turtle Oct 8, 2020

Choose a reason for hiding this comment

colin-axner commented Oct 8, 2020

amaury1093 commented Oct 8, 2020

tac0turtle commented Oct 8, 2020 • edited Loading

colin-axner commented Oct 8, 2020

tac0turtle commented Oct 8, 2020

amaury1093 commented Oct 8, 2020

amaury1093 commented Oct 8, 2020

amaury1093 commented Oct 2, 2020 •

edited

Loading

codecov bot commented Oct 2, 2020 •

edited

Loading

amaury1093 commented Oct 7, 2020 •

edited

Loading

tac0turtle Oct 8, 2020 •

edited

Loading

amaury1093 Oct 8, 2020 •

edited

Loading

tac0turtle commented Oct 8, 2020 •

edited

Loading