Remove loop of testing against older versions

[cainejette]

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.

[ipmsteven]

hmm, it seems we need also add 2.19 here since that is our current supported version of ghes

backup-utils/test/bin/ghe-negotiate-version

Line 6 in 5e551d2

echo "GitHub Enterprise Server version $GHE_TEST_REMOTE_VERSION"

[cainejette]

What do you mean? 2.19 is the first one listed

[lildude]

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]

Ah interesting, that is a good point. Perhaps we should just drop this altogether and only run tests once...

[ipmsteven]

hmm, I feel there are three version constraints here:

1. The absolute versions of GHES we support, right now it is: 2.18, 2.19, 2.20, and 2.21
2. The absolute versions of backup-util we support: do we have it now?
3. 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?

[cainejette]

I don't think we need to but it would certainly give us more confidence if we did!

[ipmsteven]

I am wondering why we are removing the test for old version.
if we removes it, then this existing test seems not make sense anymore?

begin_test "ghe-host-check blocks restore to old release"
(
  set -e
  
  mkdir -p "$GHE_DATA_DIR/current/"
  echo "$GHE_TEST_REMOTE_VERSION" > "$GHE_DATA_DIR/current/version"

  # shellcheck disable=SC2046 # Word splitting is required to populate the variables
  read -r bu_version_major bu_version_minor bu_version_patch <<<$(ghe_parse_version $GHE_TEST_REMOTE_VERSION)
  ! GHE_TEST_REMOTE_VERSION=$bu_version_major.$((bu_version_minor-1)).$bu_version_patch ghe-restore -v
)

[lildude]

if we removes it, then this existing test seems not make sense anymore?

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.

[lildude]

We can 🔥 the entire REMOTE_VERSIONS.

We can also 🔥 this:

backup-utils/script/release

Lines 214 to 216 in 71cd8f6

	content = File.read('script/cibuild')
	new_content = content.gsub(/^REMOTE_VERSIONS=.*$/, "REMOTE_VERSIONS=\"#{min_version} #{new_version}\"")
	File.open('script/cibuild', 'w') {|file| file.puts new_content }

... as that env var won't exist to be updated.

[lildude]

LGTM