Enabling additional tests to run in parallel #6766
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
* chore: adding proto files for ics20-v2 * chore: add newline
* add CurrentVersion, EscrowVersion, update tests * update hardcoded transfer channel version from interchaintest
--------- Co-authored-by: Charly <charly@interchain.berlin>
…shalling/conversion (#6226) * chore: adding proto files for ics20-v2 * chore: add newline * chore: modify MsgTransfer to accept coins instead of coin * chore: reverted unintentional comment changes * chore: reverted unintentional comment changes * chore: adding test for conversion fn * chore: fix e2e linter * chore: adding docs * chore: addressing PR feedback * chore: remove duplicate import * chore: addressing PR feedback * chore: moved extration logic into internal package * chore: commented out failing test * chore: adding link to issue * chore: remove duplicate import * chore: fix linting errors * FungibleTokenPacketData interface methods + tests * linter * wip: token validation * update trace identifier validation in Token + tests * rm Printf * update with pr review * add CurrentVersion, EscrowVersion, update tests * pr review * fix e2e tests * pr review * update e2e test version * linter * update hardcoded transfer channel version from interchaintest * update hardcoded transfer channel version from interchaintest * wip packet unmarshaller * wip * wip testing * update test * linter * rm unnecessary version changes * rm unnecessary artifacts * update callbacks test * update comment * pr review * rename getMultiDenomFungibleTokenPacketData to unmarshalPacketDataBytesToICS20V2 --------- Co-authored-by: chatton <github.qpeyb@simplelogin.fr> Co-authored-by: Carlos Rodriguez <carlos@interchain.io>
…allbacks (#6175) * add SupportedVersions array for different ics20 versions, add version checking on channel handshake application callbacks * add tests * update pr review * pr review * last few pr review nits * linter * add version counter proposing * fix missing app versino * update code + tests to return our proposed version if counterparty version is invalid * remove if statement * address review comments: return ics20-2 if counterparty version is not supported --------- Co-authored-by: Carlos Rodriguez <carlos@interchain.io>
…transfers (#6252) * add transfer authz code + tests * linter * address review comments --------- Co-authored-by: Carlos Rodriguez <carlos@interchain.io>
…ks (#6189) * chore: adding proto files for ics20-v2 * chore: add newline * chore: modify MsgTransfer to accept coins instead of coin * chore: reverted unintentional comment changes * chore: reverted unintentional comment changes * chore: adding test for conversion fn * chore: fix e2e linter * chore: adding docs * chore: addressing PR feedback * chore: remove duplicate import * chore: addressing PR feedback * chore: moved extration logic into internal package * chore: commented out failing test * chore: adding link to issue * chore: remove duplicate import * chore: fix linting errors * FungibleTokenPacketData interface methods + tests * linter * wip: token validation * update trace identifier validation in Token + tests * rm Printf * update with pr review * pr review * linter * rm unused function: linter * wip pr feedback * fix test * pr review * lintttttt * wip: backwards compatibility for transfer rpc * implement changes for ics20-v2 packet data for onRecvPacket, onAcknowledgePacket and onTimeoutPacket * fix callbacks tests * lint --------- Co-authored-by: chatton <github.qpeyb@simplelogin.fr> Co-authored-by: Charly <charly@interchain.berlin>
chatton
requested review from
AdityaSripal,
crodriguezvega,
damiannolan,
DimitrisJim,
gjermundgaraba and
bznein
as code owners
chatton
commented
Jul 8, 2024
| DockerClient *dockerclient.Client | ||
| network string | ||
| chains []ibc.Chain | ||
| relayerWallets relayer.Map |
There was a problem hiding this comment.
this name is clearer I think now that there is a pool of relayers which is actually relayers, this is specifically wallets to use in tests.
chatton
changed the title
chatton
commented
Jul 9, 2024
Comment on lines
+345
to
+346
| rlyAName := fmt.Sprintf("%s-%s", ChainARelayerName, testName) | ||
| rlyBName := fmt.Sprintf("%s-%s", ChainBRelayerName, testName) |
There was a problem hiding this comment.
there is a relayer wallet per chain per test
chatton
commented
Jul 9, 2024
| @@ -64,7 +64,7 @@ func (s *E2ETestSuite) BroadcastMessages(ctx context.Context, chain ibc.Chain, u | |||
| broadcastFunc := func() (sdk.TxResponse, error) { | |||
| return cosmos.BroadcastTx(ctx, broadcaster, user, msgs...) | |||
| } | |||
| if s.relayers.ContainsRelayer(s.T().Name(), user) { | |||
| if s.relayerWallets.ContainsRelayer(s.T().Name(), user) { | |||
There was a problem hiding this comment.
just noticing that s.T().Name() is no longer the correct key for this, changing this will make the diff super ugly so I might do this in a follow up.
damiannolan
approved these changes
Jul 9, 2024
colin-axner
approved these changes
Jul 10, 2024
| @@ -53,6 +53,7 @@ func (suite *AuthzTransferTestSuite) TestAuthz_MsgTransfer_Succeeds() { | |||
| ctx := context.TODO() | |||
|
|
|||
| testName := t.Name() | |||
| t.Parallel() | |||
There was a problem hiding this comment.
Is there a rule of thumb when to add this?
There was a problem hiding this comment.
Ideally we can just add this any time there are tests that do not make chain-wide modifications such as param changes / chain halts / config modifications etc.
In some cases it works out the box (like with authz and forwarding E2Es) and some will take some re-working or tweaking (looking at ICA )
There was a problem hiding this comment.
Will make sure to add this to the updated README for E2Es once this block of work is done!
|
gjermundgaraba
approved these changes
Jul 10, 2024
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.
Description
depends on #6721This PR enables the following tests to run as a full test suite in parallel
ref: #6764
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.
docs/).godoccomments.Files changedin the GitHub PR explorer.SonarCloud Reportin the comment section below once CI passes.