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

feat (tests): enable multiple upgrades for automated upgrade tests #1283

Merged
merged 32 commits into from
Jan 27, 2023

Conversation

MalteHerrmann
Copy link
Contributor

@MalteHerrmann MalteHerrmann commented Jan 25, 2023

Description

This PR does the following

  • introduces the option to have multi-step upgrades during the automated upgrade tests
  • enables to also include versions in testing that don't have the submit-legacy-proposal command (Evmos < v10.0.0)
  • use custom pruning settings for the upgraded nodes

In order to run a sequence of version A to version B to version C, the syntax is as follows:

make test-upgrade INITIAL_VERSION=A/B TARGET_VERSION=C

The following commands can be tested (checkbox indicates if they work as expected):

  • make test-upgrade INITIAL_VERSION=v10.0.1
  • make test-upgrade INITIAL_VERSION=v10.0.1 TARGET_VERSION=v11.0.0-rc3
  • make test-upgrade INITIAL_VERSION=v10.0.1/v11.0.0-rc1 TARGET_VERSION=v11.0.0-rc3

Closes ENG-1388

@linear
Copy link

linear bot commented Jan 25, 2023

ENG-1388 Adjust Upgrade Test Suite to allow for upgrading a series of more than two upgrades

Currently only one-step upgrades are possible (e.g. v10.0.1 > v11.0.0-rc1).

In order to test certain cases regarding testnet upgrades it would be necessary to replicate multi-step upgrades.

@MalteHerrmann MalteHerrmann marked this pull request as ready for review January 25, 2023 23:31
@MalteHerrmann MalteHerrmann requested a review from a team as a code owner January 25, 2023 23:31
@MalteHerrmann MalteHerrmann requested review from Vvaradinov and GAtom22 and removed request for a team January 25, 2023 23:31
tests/e2e/e2e_suite_test.go Outdated Show resolved Hide resolved
tests/e2e/e2e_suite_test.go Outdated Show resolved Hide resolved
tests/e2e/upgrade/utils.go Outdated Show resolved Hide resolved
tests/e2e/upgrade/manager.go Outdated Show resolved Hide resolved
tests/e2e/e2e_utils_test.go Show resolved Hide resolved
tests/e2e/e2e_suite_test.go Outdated Show resolved Hide resolved
tests/e2e/e2e_suite_test.go Show resolved Hide resolved
tests/e2e/e2e_utils_test.go Outdated Show resolved Hide resolved
Copy link
Contributor

@Vvaradinov Vvaradinov left a comment

Choose a reason for hiding this comment

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

tACK. Great work on this @MalteHerrmann this will really simplify upgrade testing!

tests/e2e/e2e_utils_test.go Show resolved Hide resolved
Copy link
Contributor

@GAtom22 GAtom22 left a comment

Choose a reason for hiding this comment

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

tACK! Great work!! Left a small typo fix.
Also, I noted that running the make test-upgrade command runs all the tests on the e2e package. Not sure if this is intended, but refactoring the test-upgrade instruction on the Makefile to this (only last line changes) only runs the automated upgrade test and shows the logs of the upgrade on real time (at least for me, using Ubuntu)

test-upgrade:
	mkdir -p ./build
	rm -rf build/.evmosd
	INITIAL_VERSION=$(INITIAL_VERSION) TARGET_VERSION=$(TARGET_VERSION) \
	E2E_SKIP_CLEANUP=$(E2E_SKIP_CLEANUP) MOUNT_PATH=$(MOUNT_PATH) CHAIN_ID=$(CHAIN_ID) \
	go test ./tests/e2e -v -run ^TestIntegrationTestSuite$

Another nice-to-have would be to extend this functionality to any number of upgrades. For example, I would replace INITIAL_VERSION and TARGET_VERSION with VERSION_SEQUENCE.

VERSION_SEQUENCE=<initial_version>/<1st_upgrade_version>/<2nd_upgrade_version>/<target_version>

But this is not a priority right now, and I understand the refactor would take some time. Just mentioning as a nice-to-have

tests/e2e/README.md Outdated Show resolved Hide resolved
Co-authored-by: Tomas Guerra <54514587+GAtom22@users.noreply.github.com>
@MalteHerrmann
Copy link
Contributor Author

MalteHerrmann commented Jan 26, 2023

Another nice-to-have would be to extend this functionality to any number of upgrades. For example, I would replace INITIAL_VERSION and TARGET_VERSION with VERSION_SEQUENCE.

@GAtom22 it is already possible to include any number of upgrades, they just have to be separated by a forward slash - so e.g. make test-upgrade INITIAL_VERSION=v9.1.0/v10.0.1/v11.0.0-rc1 TARGET_VERSION=v11.0.0-rc3 works

I also thought about refactoring the variables to an upgrade sequence but saw the advantage with keeping the target version separate. The advantage is that users can just leave it empty and know the local codebase is used and don’t have to remember the exact syntax e.g. to put latest at the end of the sequence: v9.1.0/v10.0.1/latest.

What do you think?

@codecov
Copy link

codecov bot commented Jan 26, 2023

Codecov Report

Merging #1283 (4adb8e0) into main (efe9cc7) will decrease coverage by 2.81%.
The diff coverage is 12.50%.

❗ Current head 4adb8e0 differs from pull request most recent head 80f3b50. Consider uploading reports for the commit 80f3b50 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1283      +/-   ##
==========================================
- Coverage   76.28%   73.48%   -2.81%     
==========================================
  Files         149      153       +4     
  Lines        8304     8639     +335     
==========================================
+ Hits         6335     6348      +13     
- Misses       1775     2095     +320     
- Partials      194      196       +2     
Impacted Files Coverage Δ
tests/e2e/upgrade/govexec.go 0.00% <0.00%> (ø)
tests/e2e/upgrade/manager.go 0.00% <0.00%> (ø)
tests/e2e/upgrade/utils.go 24.52% <57.14%> (ø)
tests/e2e/upgrade/node.go 0.00% <0.00%> (ø)

@MalteHerrmann MalteHerrmann enabled auto-merge (squash) January 27, 2023 16:38
@MalteHerrmann MalteHerrmann merged commit e92ecb6 into main Jan 27, 2023
@MalteHerrmann MalteHerrmann deleted the malte/multiple-upgrades-in-tests branch January 27, 2023 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants