Skip to content

internal/commands is order-dependent: TestCheckAPIMatchRefusesAPlatformCommandOnAnInstanceProfile fails under -shuffle #370

Description

@ktn-jamf

What happens

go test ./internal/commands/ -shuffle=N fails on seeds 2, 7 and 10 (and 1234, recorded earlier):

--- FAIL: TestCheckAPIMatchRefusesAPlatformCommandOnAnInstanceProfile

CI does not shuffle, so this is invisible there. It has surfaced repeatedly during review of #359 and #360 and is not caused by either — both were checked across ten seeds and every test they add passes.

Why it matters more than a flaky test

The package keeps its command-line state in package-level vars that cobra parses into: outputFmt, noColor, wide, outFile, selectFields, compact, fieldName, quiet, noHints, plus the auth ones. Any test that drives the root command leaves them set for whatever runs next.

That is not hypothetical. It produced a real false result during review:

  • go test -shuffle=4 failed a test in fix(output): print every command through the shared output formatter #359 with fieldName still "command" from an earlier runRoot, which sent printRows down the --field branch instead of rendering the table the test asserted on. Reproduced with no source change at all — the test was correct and the ordering was the bug.
  • Separately, TestBackupListResources_HonoursSelect leaked --select resource, and running it before the export test turned that test's buffer into [] instead of the expected YAML.

Both are fixed in #359 by a restoreOutputFlags(t) helper that saves and restores all nine output flag vars. The auth and profile vars have no equivalent, which is what the remaining failure is about.

Suggested direction

Two options, in increasing order of effort:

  1. A shared t.Cleanup helper for the auth vars, matching restoreOutputFlags in output_flags_test.go, applied to every test that constructs a root command. Cheap, and it makes the package shuffle-clean.
  2. Stop keeping flag state in package vars. They exist because generated commands reach them through CLIContext, so this is a wider change; option 1 is the pragmatic fix.

Either way, adding -shuffle=on to CI is what stops it coming back — but only after the existing failures are fixed, or CI becomes flaky.

Also present

The review that surfaced this reports two further order-dependent failures in the package at seeds 2 and 5, unrelated to the output flags. Worth running a sweep of seeds once the above lands rather than fixing one at a time.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions