test: add unit tests for version-check comparison and skip logic - #1339
test: add unit tests for version-check comparison and skip logic#1339Montana wants to merge 1 commit into
Conversation
Sorry, something went wrong.
Sorry, something went wrong.
|
Hi @burmudar, You're right. I'd added the comparison and skip helpers but never invoked them, so the check was dead code. Fixed by calling maybeWarnVersion(os.Args[1:]) from main() before maybeRunMigratedCommand(). It reuses the existing getRecommendedVersion against .api/src-cli/version instead of duplicating the request. Behavior matches the PR description: fail-open on any error, 3s timeout, stderr only so --json is untouched, and the skip rules (dev build via version.BuildTag == version.DefaultBuildTag, SRC_SKIP_VERSION_CHECK, and version/help/no-arg) are all applied at the call site. Comparison is major/minor/patch only, so prereleases don't warn. I've folded your #1339 tests in here so the comparison and skip logic are covered. One thing to flag, because this runs before flag parsing, -endpoint/-config passed as flags aren't picked up by the early config read — it resolves the endpoint from SRC_ENDPOINT, the config file, or the default. Happy to move the call after config resolution if you'd rather flags be honored; I kept it simple to preserve the fail-open guarantees, but your call. I've reopened another 2 PR's that should address this, you can fold them together if you want. Cheers, |
Sorry, something went wrong.
Before running a command, src does a best-effort check of the running version against the version recommended by the configured instance (/.api/src-cli/version) and prints a single stderr warning when behind. Fail-open, 3s timeout, stderr only so --json is unaffected. Skips version/help/no-arg, dev builds, and SRC_SKIP_VERSION_CHECK. Compares major/minor/patch only. Adds unit tests. Closes sourcegraph#930. Supersedes sourcegraph#1339, sourcegraph#1343.
Hey Sourcegraph,
This change adds cmd/src/version_check_test.go, covering both pieces of thenew logic with table-driven unit tests. The functions under test are pure and have no network or filesystem dependencies, so the suite runs fast and
deterministically.
isOutdated
Verifies the version comparison that decides whether a warning should fire:
4.3.1), older minor (4.3.0 vs 4.4.0), and older major (3.43.0 vs
4.0.0) are each reported as outdated, while equal (4.4.0 vs 4.4.0) and
newer (4.5.0 vs 4.4.0) versions are not.
compares the same as its bare form, so the result doesn't depend on how the
version string is formatted.
components: a prerelease on the same base (4.4.0-rc.1 vs 4.4.0) is not
flagged as outdated, while a prerelease on an older base (4.3.0-rc.1 vs
4.4.0) still is. This guards against spurious warnings for users running
release candidates.
than guessing) when the current version can't be parsed (dev) or the
recommended version is empty, so callers know to stay silent instead of
warning on bad data.
shouldSkipVersionCheck
Verifies the guard that decides when to run the check at all:
an empty subcommand, and flag-only invocations like -h / --help, where a
warning would be noise (and where version already reports this information
itself).
real commands (search, batch, api), so the feature isn't accidentally
disabled.
variable suppresses the check, covering the documented escape hatch for CI
and scripts.
default dev value, so local and development builds don't warn on every
invocation.
Because shouldSkipVersionCheck reads the package-level version.BuildTag, the test temporarily overrides it to a real release value (4.3.0) and restores it via t.Cleanup, keeping the cases isolated from the build-time version.