Replace zap logger to testing for the integration tests. #508
Conversation
dzmitryhil
requested review from
ysv,
miladz68,
wojtek-coreum and
vertex451
as code owners
There was a problem hiding this comment.
Reviewed 23 of 24 files at r1, 1 of 1 files at r2, 2 of 2 files at r3, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @dzmitryhil, @silverspase, @wojtek-coreum, and @ysv)
a discussion (no related file):
Running this, I still get logs from helper functions. But I think the point of having t.Helper was that it will not print the logs for that function ?
go clean -testcache && go test -v -timeout 30s -tags integrationtests -run ^TestAssetFTIssue$ github.com/CoreumFoundation/coreum/integration-tests/modules
=== RUN TestAssetFTIssue
=== PAUSE TestAssetFTIssue
=== CONT TestAssetFTIssue
assetft_test.go:46: Funding accounts for tests, it might take a while...
assetft_test.go:46: Test accounts funded
--- PASS: TestAssetFTIssue (3.62s)
PASS
ok github.com/CoreumFoundation/coreum/integration-tests/modules 3.663s
.github/workflows/ci.yml line 38 at r2 (raw file):
docker-cache: false - ci_step: "integration tests coreum-modules" command: "crust build/integration-tests/coreum/modules images --log-format=yaml && crust znet test --test-groups=coreum-modules --log-format=yaml"
we agreed to move this change into another PR.
There was a problem hiding this comment.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @miladz68, @silverspase, @wojtek-coreum, and @ysv)
a discussion (no related file):
Previously, miladz68 (milad) wrote…
Running this, I still get logs from helper functions. But I think the point of having t.Helper was that it will not print the logs for that function ?
go clean -testcache && go test -v -timeout 30s -tags integrationtests -run ^TestAssetFTIssue$ github.com/CoreumFoundation/coreum/integration-tests/modules === RUN TestAssetFTIssue === PAUSE TestAssetFTIssue === CONT TestAssetFTIssue assetft_test.go:46: Funding accounts for tests, it might take a while... assetft_test.go:46: Test accounts funded --- PASS: TestAssetFTIssue (3.62s) PASS ok github.com/CoreumFoundation/coreum/integration-tests/modules 3.663s
The log line is the test line, not the helper function line.
.github/workflows/ci.yml line 38 at r2 (raw file):
Previously, miladz68 (milad) wrote…
we agreed to move this change into another PR.
Here the changes for the tests only.
There was a problem hiding this comment.
Reviewed 21 of 24 files at r1, 1 of 1 files at r2, 2 of 2 files at r3, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @dzmitryhil, @miladz68, @silverspase, and @wojtek-coreum)
integration-tests/ibc/asset_ft_test.go line 169 at r3 (raw file):
}) requireT := require.New(t)
minor: requireT initialization should be moved to the top of the func. Because error assertion should be done right after coreumBankClient.Balance
integration-tests/modules/auth_test.go line 190 at r3 (raw file):
result, err := client.BroadcastRawTx(ctx, clientCtx, encodedTx) requireT.NoError(err) t.Logf("Fully signed tx executed, txHash:%s", result.TxHash)
I can see that you don't add space after : in most places when you print some variable value
Is it done in purpose ?
There was a problem hiding this comment.
Reviewed 21 of 24 files at r1, 1 of 1 files at r2, 2 of 2 files at r3, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @dzmitryhil, @miladz68, and @silverspase)
There was a problem hiding this comment.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @dzmitryhil and @silverspase)
There was a problem hiding this comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @silverspase and @ysv)
integration-tests/ibc/asset_ft_test.go line 169 at r3 (raw file):
Previously, ysv (Yaroslav Savchuk) wrote…
minor: requireT initialization should be moved to the top of the func. Because error assertion should be done right after
coreumBankClient.Balance
Done
integration-tests/modules/auth_test.go line 190 at r3 (raw file):
Previously, ysv (Yaroslav Savchuk) wrote…
I can see that you don't add space after
:in most places when you print some variable valueIs it done in purpose ?
It's just a style. IMO since we have : we don't need additional delimiter such a space.
There was a problem hiding this comment.
Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @miladz68, @silverspase, and @ysv)
There was a problem hiding this comment.
Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @silverspase and @ysv)
There was a problem hiding this comment.
Reviewed 1 of 1 files at r4, 1 of 1 files at r5, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @silverspase)
Now we don't pass
test.vflag that's why you don't see it in the CI tests now. But once the crust supports it, the output will be like in the following snippet.Example of the test output:
This change is