-
Notifications
You must be signed in to change notification settings - Fork 27
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 refactor separate suites #592
Conversation
scripts/local/e2e_test.sh
Outdated
printUsage | ||
} | ||
|
||
valid_components="teleporter governance validator-manager" |
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.
If we just test for the existence of ./tests/local//$component/$component.test
, then we don't have to maintain this list.
} | ||
|
||
// Define the Teleporter before and after suite functions. | ||
var _ = ginkgo.BeforeSuite(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.
Is this setup the same for all the suites? Can we extract this function out somewhere common?
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.
The only difference is that governance
and validator-manager
suites omit setting up Teleporter. I'm open to suggestions, but IMO the set of network setup functions are sufficiently modular to cover our test cases.
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.
Sorry - got my wires crossed. This WIP branch makes the change I described. With it, the governance
and validator-manager
suites have no dependency on the Teleporter contracts
scripts/local/e2e_test.sh
Outdated
valid_components="teleporter governance validator-manager" | ||
|
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.
We still reference valid_components
on line 21. We should probably just ls ./tests/local/
to give a list of valid components.
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 just removed the variable containing valid components altogether to simplify things.
scripts/local/e2e_test.sh
Outdated
function printUsage() { | ||
cat << EOF | ||
Arguments: | ||
--component <component> Component test suite to run. Must be a valid directory in tests/local/ |
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.
Love the change to be able to only test the component that changed but should we still have the ability to run all of them easily?
I'm thinking it might be worthwhile locally and maybe on merge to master which would be a good sanity check and be rarer than each PR commit push
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.
For CI, we should run the suites in parallel on separate runners in all scenarios for expediancy and isolation. The local use case makes sense though. I've modified the script to run all components by default if no argument is provided.
Why this should be merged
Following #591, updates the remaining validator manager e2e tests to use the full validator registration flow with the P-Chain.
How this works
teleporter
,governance
, andvalidator-manager
. Each of these suites run in their own CI jobs.validator-manager
suite test since they modify the underlying network topologyvalidator-manager
tests to reduce the overall e2e running timeerc20_delegation.go
removed; delegation logic moved toerc20_token_staking.go
native_delegation.go
removed; delegation logic moved tonative_token_staking.go
poa_validator_manager.go
removed; PoA owner checks moved topoa_to_pos.go
How this was tested
CI
How is this documented
N/A