testutils refactors and cleanup #472
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
shaspitz
requested review from
jtremback,
mpoke,
sainoe and
danwt
as code owners
MSalopek
approved these changes
Nov 15, 2022
| func AddDemocracyConsumer[T e2e.DemocConsumerApp](coordinator *ibctesting.Coordinator, t *testing.T, | ||
| appIniter ibctesting.AppIniter) (*ibctesting.TestChain, T) { | ||
|
|
||
| democConsumer := ibctesting.NewTestChain(t, coordinator, appIniter, democConsumerChainID) |
There was a problem hiding this comment.
Why is democConsumer setup different from initialization done in AddConsumers?
Provider's valset is copied to each of the instantiated consumers so I'm a bit confused. Isn't democracy chain intended to be used as a CCV consumer?
There was a problem hiding this comment.
It's notable that the democracy consumer e2e tests are separated from the normal e2e tests. The democracy consumer tests do not run with a provider, so they need to be setup slightly differently (no copying of provider val set).
The democracy consumer should be able to pass the normal set of e2e ccv tests tho, it's not currently able to do this. see #397
danwt
approved these changes
Nov 15, 2022
danwt
pushed a commit
that referenced
this pull request
Nov 16, 2022
commit fb83bb7 Author: MSalopek <[email protected]> Date: Tue Nov 15 22:54:06 2022 +0100 add multiple consumer chains in integration tests (#407) commit 33137b3 Author: Shawn Marshall-Spitzbart <[email protected]> Date: Tue Nov 15 12:05:45 2022 -0800 testutils refactors and cleanup (#472) * changes * mas * Update README.md * Update README.md * sorry for the friday night emails * Update instance_test.go * util naming commit 0096317 Author: Daniel T <[email protected]> Date: Tue Nov 15 19:24:40 2022 +0000 Adds Cryptographic Identity utility for working with various keys ect (#470) * Adds crypto.go Cryptographic Identity util * Changes mock to ibcmock Co-authored-by: Daniel <[email protected]> commit b10e132 Author: Shawn Marshall-Spitzbart <[email protected]> Date: Mon Nov 14 16:32:31 2022 -0800 circuit breaker params (#444) * changes * Update params.go commit df53566 Author: Shawn Marshall-Spitzbart <[email protected]> Date: Mon Nov 14 16:11:22 2022 -0800 Slashing related e2e test improvements (#461)
danwt
added a commit
that referenced
this pull request
Nov 16, 2022
Squashed commit of the following: commit fb83bb7 Author: MSalopek <[email protected]> Date: Tue Nov 15 22:54:06 2022 +0100 add multiple consumer chains in integration tests (#407) commit 33137b3 Author: Shawn Marshall-Spitzbart <[email protected]> Date: Tue Nov 15 12:05:45 2022 -0800 testutils refactors and cleanup (#472) * changes * mas * Update README.md * Update README.md * sorry for the friday night emails * Update instance_test.go * util naming commit 0096317 Author: Daniel T <[email protected]> Date: Tue Nov 15 19:24:40 2022 +0000 Adds Cryptographic Identity utility for working with various keys ect (#470) * Adds crypto.go Cryptographic Identity util * Changes mock to ibcmock Co-authored-by: Daniel <[email protected]> commit b10e132 Author: Shawn Marshall-Spitzbart <[email protected]> Date: Mon Nov 14 16:32:31 2022 -0800 circuit breaker params (#444) * changes * Update params.go commit df53566 Author: Shawn Marshall-Spitzbart <[email protected]> Date: Mon Nov 14 16:11:22 2022 -0800 Slashing related e2e test improvements (#461) Co-authored-by: Daniel <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Corrected version of #467
This PR should not change test behavior in anyway. It should allow the implementation of multiple consumers in e2e tests to be cleaner. Changes included:
testutil/samplefoldertestutil/simapp/simapp.goand is nowtestutil/ibc_testing/specific_setup.gotestutil/ibc_testing/generic_setup.gowith semantics closer to the design of the IBC testing package. I've also made those files more succint, and less boilerplate-y