Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upGitHub is where the world builds software
Millions of developers and companies build, ship, and maintain their software on GitHub — the largest and most advanced development platform in the world.
Remove loop of testing against older versions #615
Conversation
| @@ -6,7 +6,7 @@ set -e | |||
| # place with this version at the beginning of each test and many commands have | |||
| # conditional logic based on the remote version. Running the suite against | |||
| # different major versions ensures we're covering these conditional paths. | |||
| REMOTE_VERSIONS="2.19.0 2.21.0" | |||
| REMOTE_VERSIONS="2.19.0 2.20.0 2.21.0" | |||
ipmsteven
Jun 9, 2020
Contributor
hmm, it seems we need also add 2.19 here since that is our current supported version of ghes
hmm, it seems we need also add 2.19 here since that is our current supported version of ghes
cainejette
Jun 9, 2020
Author
Contributor
What do you mean? 2.19 is the first one listed
What do you mean? 2.19 is the first one listed
lildude
Jun 9, 2020
Member
This is mostly useless these days as I don't think there are any more version-specific code paths so is really just acting as a version verifier that unnecessarily runs the entire test suite twice. Adding a third version just adds another unnecessary delay.
This is mostly useless these days as I don't think there are any more version-specific code paths so is really just acting as a version verifier that unnecessarily runs the entire test suite twice. Adding a third version just adds another unnecessary delay.
cainejette
Jun 9, 2020
Author
Contributor
Ah interesting, that is a good point. Perhaps we should just drop this altogether and only run tests once...
Ah interesting, that is a good point. Perhaps we should just drop this altogether and only run tests once...
ipmsteven
Jun 9, 2020
Contributor
hmm, I feel there are three version constraints here:
- The absolute versions of GHES we support, right now it is: 2.18, 2.19, 2.20, and 2.21
- The absolute versions of backup-util we support: do we have it now?
- The relative versions constraint between
backup-util and GHES, which are:
Backup Utilities major.minor(or feature).patch can be used to backup and restore for GHES with version from major.minor-2.patch to major.minor.patch
My question is: do we need to have version validation for ALL version constraints mentioned above in backup-util layer?
hmm, I feel there are three version constraints here:
- The absolute versions of GHES we support, right now it is: 2.18, 2.19, 2.20, and 2.21
- The absolute versions of backup-util we support: do we have it now?
- The relative versions constraint between
backup-utilandGHES, which are:
Backup Utilitiesmajor.minor(or feature).patchcan be used to backup and restore for GHES with version frommajor.minor-2.patchtomajor.minor.patch
My question is: do we need to have version validation for ALL version constraints mentioned above in backup-util layer?
cainejette
Jun 24, 2020
Author
Contributor
I don't think we need to but it would certainly give us more confidence if we did!
I don't think we need to but it would certainly give us more confidence if we did!
|
I am wondering why we are removing the test for old version. |
It does make sense. That test is testing to ensure we prevent restoring a backup of GHES 2.21.x to GHES 2.20.x. |
Co-authored-by: Yunlei Liu <ipmsteven@github.com>
|
We can We can also Lines 214 to 216 in 71cd8f6 ... as that env var won't exist to be updated. |
Co-authored-by: Colin Seymour <colin@github.com>
|
LGTM |
There used to be different code paths executed per GHES version, so we'd run the tests against each simulated version. We no longer have different code paths, so we're running the tests twice for no reason.