Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: fix test after merge latest develop #147

Merged
merged 5 commits into from
Apr 27, 2021

Conversation

brew0722
Copy link
Contributor

Description

closes: #131
After merging with the last latest develop, CI Test passed when rerun.
In reality, the test should fail.


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
Copy link

codecov bot commented Apr 26, 2021

Codecov Report

Merging #147 (e9727a3) into v2/develop (1f739c5) will increase coverage by 0.04%.
The diff coverage is 76.76%.

❗ Current head e9727a3 differs from pull request most recent head b03538a. Consider uploading reports for the commit b03538a to get more accurate results
Impacted file tree graph

@@              Coverage Diff               @@
##           v2/develop     #147      +/-   ##
==============================================
+ Coverage       54.99%   55.03%   +0.04%     
==============================================
  Files             812      811       -1     
  Lines           51487    51443      -44     
==============================================
- Hits            28317    28314       -3     
+ Misses          20211    20189      -22     
+ Partials         2959     2940      -19     
Impacted Files Coverage Δ
client/grpc/tmservice/query.pb.gw.go 30.06% <ø> (ø)
client/keys/root.go 100.00% <ø> (ø)
client/keys/utils.go 35.29% <ø> (-3.60%) ⬇️
crypto/armor.go 85.89% <ø> (ø)
crypto/keys/ed25519/ed25519.go 67.34% <ø> (ø)
crypto/keys/multisig/codec.go 100.00% <ø> (ø)
crypto/keys/secp256k1/secp256k1.go 86.79% <ø> (ø)
simapp/test_helpers.go 0.49% <0.00%> (-0.01%) ⬇️
types/address.go 65.71% <ø> (ø)
types/config.go 84.61% <ø> (ø)
... and 33 more

@brew0722
Copy link
Contributor Author

The test below failed. However, it seems to have nothing to do with wasm, so it tries to rerun.

Tests / Code Coverage / test-cosmovisor (pull_request)

--- FAIL: TestProcessTestSuite (5.13s)
    --- FAIL: TestProcessTestSuite/TestLaunchProcessWithDownloads (3.11s)
        testing.go:1038: race detected during execution of test
    testing.go:1038: race detected during execution of test

@brew0722 brew0722 self-assigned this Apr 26, 2021
@brew0722 brew0722 changed the title fix(wasm): fix test after merge latest develop test(wasm): fix test after merge latest develop Apr 26, 2021
@brew0722 brew0722 force-pushed the brew0722/v2/feat/fix-wasm-test branch from 207c490 to 53c4996 Compare April 26, 2021 05:51
go.mod Outdated Show resolved Hide resolved
@whylee259
Copy link
Contributor

@brew0722 brew0722 force-pushed the brew0722/v2/feat/fix-wasm-test branch from d3beeff to e5e8919 Compare April 27, 2021 00:33
@brew0722 brew0722 force-pushed the brew0722/v2/feat/fix-wasm-test branch from e5e8919 to b03538a Compare April 27, 2021 00:40
@brew0722
Copy link
Contributor Author

brew0722 commented Apr 27, 2021

Failing race test has been confirmed as occurred randomly from the previous time regardless of wasm.
The team decide to temporarily disable it.
I also disabled the cosmovisor test to disable the race test. and sqaushed it.

@brew0722 brew0722 changed the title test(wasm): fix test after merge latest develop test: fix test after merge latest develop Apr 27, 2021
@@ -390,6 +391,7 @@ func (s IntegrationTestSuite) TestBroadcastTx_GRPC() {
}
})
}
time.Sleep(1 * time.Second) // wait for block confirm time before executing next test
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it difficult to confirm block height and retry if the height is not succeeded? (And, if we can retry, I feel 1 second is a little long)

Copy link
Contributor Author

@brew0722 brew0722 Apr 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@egonspace, Would you please review this together when editing concurrent tx?
First of all, I will skip this PR.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even if we try block confirmation and continue retrying, it will also take about a second. The purpose was to make the test pass simpler than complex code.

@loloicci
Copy link
Contributor

loloicci commented Apr 27, 2021

Other than this, LGTM!

@brew0722 brew0722 merged commit c612d35 into v2/develop Apr 27, 2021
@brew0722 brew0722 deleted the brew0722/v2/feat/fix-wasm-test branch April 27, 2021 07:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants