-
Notifications
You must be signed in to change notification settings - Fork 675
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
Refactor e2e tests for P-chain tests #3295
Conversation
// PChainWorkflow is an integration test for normal P-Chain operations | ||
// - Issues an Add Validator and an Add Delegator using the funding address | ||
// - Exports AVAX from the P-Chain funding address to the X-Chain created address | ||
// - Exports AVAX from the X-Chain created address to the P-Chain created address | ||
// - Checks the expected value of the funding address | ||
// - Issues an AddPermissionlessValidatorTx | ||
// - Issues an AddPermissionlessDelegatorTx | ||
// - Issues an ExportTx on the P-chain and verifies the expected balances | ||
// - Issues an ImportTx on the X-chain and verifies the expected balances |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This description was previously misleading... The functionality in this test isn't changed.
…s/avalanchego into dynamic-fees-e2e-testing-cleanup
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be preferable to separate the stylistic non-functional changes into a separate PR to ensure that the functional changes are able to get the attention they deserve.
@@ -229,84 +225,104 @@ var _ = ginkgo.Describe("[Staking Rewards]", func() { | |||
require.NoError(err) | |||
}) | |||
|
|||
tc.By("stopping beta node to prevent it and its delegator from receiving a validation reward") | |||
require.NoError(betaNode.Stop(tc.DefaultContext())) | |||
tc.By("stopping beta node to prevent it and its delegator from receiving a validation reward", func() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(No action required) What's the point of changing this to be wrapped by the By statement?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was under the impression that the reason that By
took in a function like this was to allow for that function to specify the logic that the By
text was describing.
Is that not the case? If that isn't the case, what is the purpose of the function? If it's just as a scope delimiter, why wouldn't we just explicitly use scope delimiters ({
and }
)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
afaict there is a different stack trace but otherwise no utility that I can see:
tc.By("checking Maru's patience", func() {
require.False(true, "Maru is not patient")
})
STEP: checking Maru's patience @ 08/14/24 22:23:18.839
[FAILED] in [It] - /home/me/go/pkg/mod/github.com/stretchr/[email protected]/assert/assertions.go:788 @ 08/14/24 22:23:18.84
• [FAILED] [0.011 seconds]
[Banff] [It] can send custom assets X->P and P->X
/home/me/src/avalanchego/master/tests/e2e/banff/suites.go:25
[FAILED]
Error Trace: /home/me/src/avalanchego/master/tests/e2e/banff/suites.go:47
/home/me/go/pkg/mod/github.com/onsi/ginkgo/[email protected]/internal/suite.go:323
/home/me/go/pkg/mod/github.com/onsi/ginkgo/[email protected]/core_dsl.go:591
/home/me/src/avalanchego/master/tests/fixture/e2e/ginkgo_test_context.go:47
/home/me/src/avalanchego/master/tests/e2e/banff/suites.go:46
/home/me/go/pkg/mod/github.com/onsi/ginkgo/[email protected]/internal/node.go:463
/home/me/go/pkg/mod/github.com/onsi/ginkgo/[email protected]/internal/suite.go:889
/home/me/go/pkg/mod/golang.org/[email protected]/src/runtime/asm_arm64.s:1197
Error: Should be false
Messages: Maru is not patient
In [It] at: /home/me/go/pkg/mod/github.com/stretchr/[email protected]/assert/assertions.go:788 @ 08/14/24 22:23:18.84
tc.By("checking Maru's patience")
require.False(true, "Maru is not patient")
STEP: checking Maru's patience @ 08/14/24 22:23:50.297
[FAILED] in [It] - /home/me/go/pkg/mod/github.com/stretchr/[email protected]/assert/assertions.go:788 @ 08/14/24 22:23:50.297
• [FAILED] [0.005 seconds]
[Banff] [It] can send custom assets X->P and P->X
/home/me/src/avalanchego/master/tests/e2e/banff/suites.go:25
[FAILED]
Error Trace: /home/me/src/avalanchego/master/tests/e2e/banff/suites.go:47
/home/me/go/pkg/mod/github.com/onsi/ginkgo/[email protected]/internal/node.go:463
/home/me/go/pkg/mod/github.com/onsi/ginkgo/[email protected]/internal/suite.go:889
/home/me/go/pkg/mod/golang.org/[email protected]/src/runtime/asm_arm64.s:1197
Error: Should be false
Messages: Maru is not patient
In [It] at: /home/me/go/pkg/mod/github.com/stretchr/[email protected]/assert/assertions.go:788 @ 08/14/24 22:23:50.297
Why this should be merged
How this works
e2e.NewPrivateKey
to avoid annoying error checks.How this was tested