-
Notifications
You must be signed in to change notification settings - Fork 108
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
test: Stateful upgrade #1135
test: Stateful upgrade #1135
Conversation
@@ -295,8 +303,9 @@ func LocalSmokeTest(_ *cobra.Command, _ []string) { | |||
smokeTest.TestMessagePassingRevertSuccess() | |||
smokeTest.CheckZRC20ReserveAndSupply() | |||
|
|||
smokeTest.TestPauseZRC20() | |||
smokeTest.CheckZRC20ReserveAndSupply() | |||
//DISABLED temporarily since older binaries might not work with this due to admin account |
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 remember Bertrant created these two tests and he made them working on develop? No blocking.
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 TestPauseZRC20
and TestUpdateBytcode
can be commented out since it only concerns isolated actions and don't need to be checked everytime in the CI.
But I added in the genesis generation of smoke tests the account for foreign coin admin, I'm not sure what would cause it to not work then
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 your change is not included in older versions. This particular test path would run the smoketest on an older version, do an upgrade then run the smoketest again. It isn't guaranteed that the account will be present in the older versions.
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 should comment smokeTest.TestUpdateBytecode()
for the same reason, also use the admin account.
I think we could keep those disabled since it represents tests for administration, we might want to have a specific smoketest for it.
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.
looks good.
Is it still WIP @kevinssgh ? |
Description
Added new target in makefile that will run the smoketest twice and perform a binary update between. The old and new versions to be tested can be passed into the makefile target as environment variables.
ex.
stateful-upgrade:
@echo "--> Starting stateful smoketest"
$(DOCKER) build --build-arg old_version=v9.0.0 --build-arg new_version=v9.0.6-test -t zetanode -f ./Dockerfile-versioned .
$(DOCKER) build -t orchestrator -f contrib/localnet/orchestrator/Dockerfile.fastbuild .
cd contrib/localnet/ && $(DOCKER) compose -f docker-compose-stateful.yml up -d
Closes: #1117
TODO: Need to create script that restarts zetaclients after upgrade.
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Include instructions and any relevant details so others can reproduce.