RSDK-13711 Specify test version#239
Conversation
viamrobotics#232) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
danielbotros
left a comment
There was a problem hiding this comment.
Much needed improvement, thanks Evan! Have not run tests yet but left some initial comments
|
gonna remove myself as a reviewer |
jmatth
left a comment
There was a problem hiding this comment.
LGTM. The few comments I left are nitpicks and don't need to be addressed to merge.
| @@ -17,6 +17,9 @@ if [ "$OS" = "Darwin" ]; then | |||
| BINARY_OS_PREFIX="darwin-" | |||
| fi | |||
| URL="https://storage.googleapis.com/packages.viam.com/apps/viam-agent/viam-agent-stable-${BINARY_OS_PREFIX}${ARCH}" | |||
There was a problem hiding this comment.
If you want to be cool and don't care about readability you can do this conditional assignment in one line:
| URL="https://storage.googleapis.com/packages.viam.com/apps/viam-agent/viam-agent-stable-${BINARY_OS_PREFIX}${ARCH}" | |
| URL="${AGENT_CUSTOM_URL}:-https://storage.googleapis.com/packages.viam.com/apps/viam-agent/viam-agent-stable-${BINARY_OS_PREFIX}${ARCH}}" |
|
|
||
| // latestStableRelease returns the most recently uploaded stable viam-agent | ||
| // release version (bare semver, e.g. "0.27.3"). | ||
| func latestStableRelease(ctx context.Context) (string, error) { |
There was a problem hiding this comment.
It might be possible to remove the need to specify the stable + dev versions in the config and just replace them whith these helpers.
| // | ||
| // Only viam-agent specifiers are supported today; viam-server has different | ||
| // upload paths and would need its own resolver. | ||
| func resolveVersionSpec(ctx context.Context, spec string) (string, error) { |
There was a problem hiding this comment.
This gets called by installAgent, which in turn is called multiple times across the test suite. It would be nice to cache the resolved version to cut down on unnecessary GCS queries but not essential.
danielbotros
left a comment
There was a problem hiding this comment.
LGTM, congrats on finishing agent testing 😄!!
This PR re-engineers the test suite to enable testing against a specified version of
viam-agentinstead ofstable.Interface/infra changes
viam_agent_testto specify the version under testinstallAgentnow uses the localinstall.sh(just likeuninstallAgent) instead of pulling the install script from gcp.installAgentnow checks that viam-agent is running at the specified version, instead of just running at all. If the version isn't correct, it installs the specified version.serialclient.EnsureOnlinewas re-added to the pre-scenario hook (this fixes some instability caused by the provisioning tests leaving the network adapter in a weird state)The agent version under test can be specified in a number of ways:
pr.xxxwill test a PR by number, if it has thedev-releaselabel (so that artifacts are built) e.g."pr.227""dev"will test the tip of main"stable"will test the latest stable release"x.y.z"will test any release version e.g."0.27.3""file:///path/to/agent"will test an arbitrary binary (that you manually copy onto the pi)Each feature test now prints the versions under test before running:
Test changes
Tests have been modified to account for the new syntax requirements to specify versions (like
stable,the version under test,an old version):viam-agent is installed->viam-agent is installed at the version under testthe viam-agent systemd unit is running->the viam-agent systemd unit is running with the version under testThe upgrade and downgrade tests have been changed so they make sense with this new flow:
upgrade.featureupgrades from stable to the version under testdowngrade.featuredowngrades from the version under test to stableChanges beyond tests
This PR makes a small modification to
install.sh- it adds$AGENT_CUSTOM_URL, which will be used instead of$URLas the download target for the agent binary if it$AGENT_CUSTOM_URLis passed in. This should be totally transparent if not used, but it enables the test to install any version (instead of relying on agent to update itself via pinning, which isn't really the same thing).