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

e2e tests folder restructure #14

Merged
merged 13 commits into from
Jun 20, 2024
Merged

Conversation

magiodev
Copy link
Collaborator

@magiodev magiodev commented Jun 16, 2024

This PR reorganizes the e2e tests for better structure and reuse of helper functions.

Changes

  1. New test_suite Package:
    • Moved all helpers, types, and non-test files to a new test_suite folder.
    • Split suite_helpers.go into several context-specific files in test_suite.
  2. Transfer Test Update:
    • Now runs three times to cover each version of the CW721 contract.
  3. Test File Refactor:
    • Replaced full_test.go with basic_test.go, using the new test suite structure.

@magiodev magiodev changed the title Feat/class id fix test e2e tests folder restructure Jun 16, 2024
@magiodev magiodev marked this pull request as ready for review June 16, 2024 15:50
@magiodev magiodev self-assigned this Jun 16, 2024
@magiodev magiodev requested a review from taitruong June 16, 2024 15:50
e2e/callback_test.go Outdated Show resolved Hide resolved
require.Error(suite.T(), err)
}

func callbackMemo(srcCallback, srcReceiver, dstCallback, dstReceiver string) string {
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer using same namings as it is used for props. like srcCallback -> ack_callback_data, etc.

}

func sendIcsFromChainAToB(suite *CallbackTestSuite, nft, tokenId, memo string, relay bool) {
msg := fmt.Sprintf(`{ "send_nft": {"cw721": "%s", "ics721": "%s", "token_id": "%s", "recipient":"%s", "channel_id":"%s", "memo":"%s"}}`, nft, suite.bridgeA.String(), tokenId, suite.testerB.String(), suite.pathAB.EndpointA.ChannelID, memo)
Copy link
Member

Choose a reason for hiding this comment

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

What happens if memo is empty? Memo is an optional prop.

return b64.StdEncoding.EncodeToString([]byte(memo))
}

func nftSentCb() string {
Copy link
Member

Choose a reason for hiding this comment

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

better rename to create_ack_callback_data

return b64.StdEncoding.EncodeToString([]byte(`{ "nft_sent": {}}`))
}

func nftReceivedCb() string {
Copy link
Member

Choose a reason for hiding this comment

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

rename: create_receive_callback_data

return r
}

func GetCw721SendIbcAwayMessage(path *wasmibctesting.Path, coordinator *wasmibctesting.Coordinator, tokenId string, bridge, receiver sdk.AccAddress, timeout int64, memo string) string {
Copy link
Member

Choose a reason for hiding this comment

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

GetCw721SendIbcAwayMessage -> CreateSendNftWithIbcOutgoingMsg

Comment on lines +25 to +26
ibcAway := fmt.Sprintf(`{ "receiver": "%s", "channel_id": "%s", "timeout": { "timestamp": "%d" }, "memo": %s }`, receiver.String(), path.EndpointA.ChannelID, timeout, memo)
ibcAwayEncoded := b64.StdEncoding.EncodeToString([]byte(ibcAway))
Copy link
Member

Choose a reason for hiding this comment

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

ibcAway -> ibcOutgoingMsg
ibcAwayEncoded -> ibcOutgoingMsgEncoded

require.NoError(t, err)
}

func Ics721TransferNft(t *testing.T, chain *wasmibctesting.TestChain, path *wasmibctesting.Path, coordinator *wasmibctesting.Coordinator, nft string, tokenId string, bridge, sender, receiver sdk.AccAddress, memo string) *sdk.Result { // Send the NFT away.
Copy link
Member

Choose a reason for hiding this comment

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

Ics721TransferNft -> Ics721SendNft

memo := callbackMemo(nftSentCb(), "", nftReceivedCb(), "")
// A -> B token_id 2
Copy link
Member

Choose a reason for hiding this comment

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

What's the difference between sendIcsFromChainAToB and test_suite.Ics721TransferNft? Seems to lookalike

require.Equal(suite.T(), testerDataOwnerB, suite.testerB.String())
}

func (suite *CbTestSuite) TestSuccessfulTransferWithReceivers() {
func (suite *CallbackTestSuite) TestSuccessfulTransferWithReceivers() {
Copy link
Member

Choose a reason for hiding this comment

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

This test has same result as for TestSuccessfulTransfer. By its name, I wouldnt know what it actually tests. As far as I see only difference is, by memo, where addresses are provided in one and not provided by other test:

TestSuccessfulTransferWithReceivers:
memo := callbackMemo(nftSentCb(), suite.testerA.String(), nftReceivedCb(), suite.testerB.String())

TestSuccessfulTransfer:
memo := callbackMemo(nftSentCb(), "", nftReceivedCb(), "")

require.Equal(suite.T(), chainBOwner, suite.testerB.String())

// We query the data we have on the tester contract
// This ensures that the callbacks are called after all the messages was completed
// and the transfer was successful
testerDataOwnerA := queryTesterSent(suite.T(), suite.chainA, suite.testerA.String())
testerDataOwnerA := test_suite.QueryTesterSent(suite.T(), suite.chainA, suite.testerA.String())
Copy link
Member

@taitruong taitruong Jun 16, 2024

Choose a reason for hiding this comment

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

Not clear to me, how below 3 assertions work. In case ack and receive addr is empty in callback, then ack callback is called on sender on chain A, and receive callback is called on recipient. So some kind of result data is check and verified on sender and recipient (contract?)?

@taitruong taitruong merged commit 811f567 into class_id_fix Jun 20, 2024
4 checks passed
@taitruong
Copy link
Member

@magiodev great job! thx a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants