Review agent's upgrade tests and change used versions if necessary

[rdner]

All the preliminary research was done here #4296 (comment)

We should review all our current upgrade tests and decide whether using the versions they use actually makes sense.

The goal is to minimize the amount of different versions used for testing, we currently have:

• define.Version() => define.Version()
• define.Version() => define.Version()-SNAPSHOT
• define.Version() => PreviousMinor
• PreviousMinor => define.Version()
• define.Version() => define.Version()-SNAPSHOT+<lastBuildID>
• define.Version() => define.Version()-SNAPSHOT+<lastBuildID~1>
• GetUpgradableVersions(ctx, define.Version(), 2, 1) => define.Version()

FYI: versions returned from GetUpgradableVersions() and PreviousMinor() are stable/deterministic and configured by a file.

[rdner]

I noticed that the following test cases request a build version:

TestStandaloneUpgradeRetryDownload

elastic-agent/testing/integration/upgrade_standalone_retry_test.go

Lines 51 to 53 in ddd832b

	// Upgrade to an older snapshot build of same version but with a different commit hash
	aac := tools.NewArtifactAPIClient(tools.WithLogFunc(t.Logf))
	buildInfo, err := aac.FindBuild(ctx, startVersion.VersionWithPrerelease(), startVersionInfo.Binary.Commit, 0)

TestStandaloneUpgradeUninstallKillWatcher

elastic-agent/testing/integration/upgrade_uninstall_test.go

Lines 51 to 55 in ddd832b

	// Start on a snapshot build, we want this test to upgrade to our
	// build to ensure that the uninstall will kill the watcher.
	// We need a snapshot with a non-matching commit hash to perform the upgrade
	aac := tools.NewArtifactAPIClient(tools.WithLogFunc(t.Logf))
	buildInfo, err := aac.FindBuild(ctx, endVersion.VersionWithPrerelease(), endVersionInfo.Binary.Commit, 0)

From the name of the tests I don't think they should use build versions, what do you think
@pchila ?

We're trying to minimize the use of the artifact API due to #4268 so I'd just use the current snapshot and skip the test when commit hashes match.

[pchila]

I noticed that the following test cases request a build version:

TestStandaloneUpgradeRetryDownload

elastic-agent/testing/integration/upgrade_standalone_retry_test.go

Lines 51 to 53 in ddd832b

	// Upgrade to an older snapshot build of same version but with a different commit hash
	aac := tools.NewArtifactAPIClient(tools.WithLogFunc(t.Logf))
	buildInfo, err := aac.FindBuild(ctx, startVersion.VersionWithPrerelease(), startVersionInfo.Binary.Commit, 0)

TestStandaloneUpgradeUninstallKillWatcher

elastic-agent/testing/integration/upgrade_uninstall_test.go

Lines 51 to 55 in ddd832b

	// Start on a snapshot build, we want this test to upgrade to our
	// build to ensure that the uninstall will kill the watcher.
	// We need a snapshot with a non-matching commit hash to perform the upgrade
	aac := tools.NewArtifactAPIClient(tools.WithLogFunc(t.Logf))
	buildInfo, err := aac.FindBuild(ctx, endVersion.VersionWithPrerelease(), endVersionInfo.Binary.Commit, 0)

From the name of the tests I don't think they should use build versions, what do you think @pchila ?

We're trying to minimize the use of the artifact API due to #4268 so I'd just use the current snapshot and skip the test when commit hashes match.

I think those test can be changed to use regular snapshots/released versions without any loss of functionality. If the build lookup is just to avoid hash collision we can change them after implementing #4380 and have those tests try to upgrade to latest snapshot for the given version

[rdner]

I think further adjustments are no longer necessary. Closing now.