diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 95ea254fc..ec6a8a1f4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -164,17 +164,18 @@ jobs: # D-0275: the default run above is v1.4 only, so a test that breaks under # the v1.5 strict host-key and pipeline paths stayed unseen until the - # P7-SHIP flip. One Linux leg re-runs the suite with v1.5 on. cmd/commands - # still has tests that assume v1.4 (see D-0275 in hq/quality/debt.yaml), so - # that package runs only the pinned Hasura remote test here; widen the -run as - # the rest are pinned or fixed, until the whole package runs. + # P7-SHIP flip. One Linux leg re-runs the whole suite with v1.5 on. The + # skipped names pass alone under NSELF_V15=1 but fail after tests that load + # a project .env and leave its keys in the process environment (the v1.5 + # remote deploy then refuses them as process-only keys, D-0260). The first + # three are internal/reconcile tests. D-0275 in hq/quality/debt.yaml tracks + # isolating them so this list can go. - name: Test (v1.5 mode, NSELF_V15=1) if: matrix.os == 'ubuntu-latest' env: NSELF_V15: '1' run: | - go test -mod=vendor -timeout 10m -skip '^(TestShadowInvocationFiles|TestContainerImpactBrokenCompose|TestStateFaultInjection)$' $(go list -mod=vendor ./... | grep -v '/cmd/commands$') - go test -mod=vendor -timeout 10m -run '^TestDBHasuraRemotePartialFailure$' ./cmd/commands + go test -mod=vendor -timeout 20m -skip '^(TestShadowInvocationFiles|TestContainerImpactBrokenCompose|TestStateFaultInjection|TestWriteResolvedDeployEnv_LaterFilesOverrideEarlier|TestWriteResolvedDeployEnv_CleanupRemovesFile|TestEnvCascadePerEnv|TestDeployCascadeMatchesBuild|TestSnapshotMatchesConfigLoad|TestRemoteDeployNeverReadsEnvLocal|TestSnapshotLiteralDollarStaysLiteral|TestScratchStageShipsGeneratedAndAllowlisted|TestFrontedScratchStagesFront|TestFrontedLayoutMismatchIsRefusedBeforeStaging|TestScratchReportsStaleGeneratedOutputTheBuildDidNotRegenerate|TestScratchAllowlistedMarkedFileIsCopiedAndShipped|TestScratchReportsSymlinksUnderNginx)$' ./... - name: Test (Windows — pure-Go packages; Docker integration tests skipped) if: matrix.skip_docker_tests == 'true' diff --git a/cmd/commands/access_targets_test.go b/cmd/commands/access_targets_test.go index 01817f34f..410fc3aa0 100644 --- a/cmd/commands/access_targets_test.go +++ b/cmd/commands/access_targets_test.go @@ -12,6 +12,7 @@ import ( "github.com/nself-org/cli/internal/access" "github.com/nself-org/cli/internal/admin" + "github.com/nself-org/cli/internal/compat/compattest" "github.com/nself-org/cli/internal/controlplane" "github.com/nself-org/cli/internal/errs" "github.com/nself-org/cli/internal/version" @@ -181,6 +182,7 @@ func TestAccessProdConfirm(t *testing.T) { } func TestAccessListHostGolden(t *testing.T) { + compattest.Set(t, false) // D-0275: asserts v1.4 behaviour; v1.5 mode is not asserted by this test withFixtureTransport(t) defer resetFlags(accessListCmd) accessListCmd.SetContext(context.Background()) diff --git a/cmd/commands/config_set_test.go b/cmd/commands/config_set_test.go index 4f963c0c2..2be50b1c2 100644 --- a/cmd/commands/config_set_test.go +++ b/cmd/commands/config_set_test.go @@ -23,6 +23,8 @@ import ( "path/filepath" "strings" "testing" + + "github.com/nself-org/cli/internal/sysexec" ) // TestConfigSet_MonorepoRedirectIsVisible proves the T16 fix: running @@ -36,6 +38,7 @@ import ( // from — this test fails against that code because the output contains no // path separator at all, let alone the backend directory's full path. func TestConfigSet_MonorepoRedirectIsVisible(t *testing.T) { + t.Cleanup(sysexec.Set(&sysexec.Fake{})) // D-0275: v1.5 config set builds; keep it off the host mkcert monorepoRoot := t.TempDir() backendDir := filepath.Join(monorepoRoot, ".backend") nestedDir := filepath.Join(monorepoRoot, "apps", "foo") diff --git a/cmd/commands/config_test.go b/cmd/commands/config_test.go index e926b56a1..3d8292090 100644 --- a/cmd/commands/config_test.go +++ b/cmd/commands/config_test.go @@ -9,6 +9,7 @@ import ( "testing" "github.com/nself-org/cli/internal/config" + "github.com/nself-org/cli/internal/sysexec" "github.com/spf13/cobra" ) @@ -565,6 +566,7 @@ func TestConfigGet_UnknownKeyReturnsError(t *testing.T) { // TestConfigSet_UpdatesExistingKey verifies that 'config set KEY VALUE' // updates an existing key in-place in the .env file. func TestConfigSet_UpdatesExistingKey(t *testing.T) { + t.Cleanup(sysexec.Set(&sysexec.Fake{})) // D-0275: v1.5 config set builds; keep it off the host mkcert dir := makeNSelfProject(t, "BASE_DOMAIN=old.example.com\n") root := newConfigCmd() @@ -591,6 +593,7 @@ func TestConfigSet_UpdatesExistingKey(t *testing.T) { // TestConfigSet_AppendsNewKey verifies that 'config set KEY VALUE' appends a // new key=value line to the .env file when the key does not already exist. func TestConfigSet_AppendsNewKey(t *testing.T) { + t.Cleanup(sysexec.Set(&sysexec.Fake{})) // D-0275: v1.5 config set builds; keep it off the host mkcert dir := makeNSelfProject(t, "PROJECT_NAME=myapp\n") root := newConfigCmd() @@ -617,6 +620,7 @@ func TestConfigSet_AppendsNewKey(t *testing.T) { // TestConfigSet_EnvFlagWritesToNamedFile verifies that --env dev causes // 'config set' to write to .env.dev rather than the default .env. func TestConfigSet_EnvFlagWritesToNamedFile(t *testing.T) { + t.Cleanup(sysexec.Set(&sysexec.Fake{})) // D-0275: v1.5 config set builds; keep it off the host mkcert dir := makeNSelfProject(t, "PROJECT_NAME=myapp\n") root := newConfigCmd() diff --git a/cmd/commands/db_envelope.go b/cmd/commands/db_envelope.go index e5e4d88b8..741da44b5 100644 --- a/cmd/commands/db_envelope.go +++ b/cmd/commands/db_envelope.go @@ -72,7 +72,7 @@ func dataEnv(run func(*cobra.Command, []string) error) func(*cobra.Command, []st // the registry is not consulted (a plain run never builds it). func jsonEnvelopeOn(cmd *cobra.Command) bool { // compat.V15(P7-SURF-14): --json on the data commands is ignored or the pre-contract form (NSELF_JSON_LEGACY=1 then prints an empty list as bare []) -> v1 envelope - if !compat.V15() { + if !compat.V15() || cmd == nil { // a nil command (runBundleList accepts one) has no flags to read return false } if _, ok := jsonDataTypes[invokedKey(cmd)]; !ok { diff --git a/cmd/commands/db_envelope_nil_test.go b/cmd/commands/db_envelope_nil_test.go new file mode 100644 index 000000000..53f8e973e --- /dev/null +++ b/cmd/commands/db_envelope_nil_test.go @@ -0,0 +1,18 @@ +package commands + +import ( + "testing" + + "github.com/nself-org/cli/internal/compat/compattest" +) + +// TestJSONEnvelopeOnNilCommand: runBundleList accepts a nil command, so the +// envelope check must answer "off" for one instead of dereferencing it in v1.5 +// (D-0275: the bundle list tests panicked under NSELF_V15=1). +func TestJSONEnvelopeOnNilCommand(t *testing.T) { + compattest.Both(t, func(t *testing.T) { + if jsonEnvelopeOn(nil) { + t.Error("jsonEnvelopeOn(nil) = true, want false") + } + }) +} diff --git a/cmd/commands/deploy_env_scope_test.go b/cmd/commands/deploy_env_scope_test.go index 4b438d81b..29324eabd 100644 --- a/cmd/commands/deploy_env_scope_test.go +++ b/cmd/commands/deploy_env_scope_test.go @@ -717,6 +717,7 @@ func TestBlueGreenRemoteRefused(t *testing.T) { // pipeline deploy fail, naming it. The ssh and rsync on PATH are fakes that // exit 1 (and PATH holds nothing else), so no host is contacted. func TestPipelineFailedServerExitsNonZero(t *testing.T) { + compattest.Set(t, false) // D-0275: asserts v1.4 behaviour; v1.5 mode is not asserted by this test dir, _ := scopeFixture(t, false) bin := t.TempDir() for _, n := range []string{"ssh", "rsync"} { @@ -1067,6 +1068,7 @@ func TestPipelineBuildsRemoteAndShipsSnapshot(t *testing.T) { // snapshot fails, nothing is probed or shipped, so a stale local compose can // never reach a host. func TestPipelineBuildOrSnapshotFailureStopsBeforeProbe(t *testing.T) { + compattest.Set(t, false) // D-0275: asserts v1.4 behaviour; v1.5 mode is not asserted by this test for _, mode := range []string{"build", "snapshot"} { dir, p := scopeFixture(t, false) log, _ := fakeShipBin(t) diff --git a/cmd/commands/deploy_stream_test.go b/cmd/commands/deploy_stream_test.go index 223125072..17f32ef31 100644 --- a/cmd/commands/deploy_stream_test.go +++ b/cmd/commands/deploy_stream_test.go @@ -585,6 +585,7 @@ const goldenLocalDryRun = `{ // bytes as before this feature (goldens captured from the base code) and nothing // goes to the stream writer. func TestDeployJSONUnchanged(t *testing.T) { + compattest.Set(t, false) // D-0275: asserts v1.4 behaviour; v1.5 mode is not asserted by this test for _, tc := range []struct{ name, target, golden string }{ {"pipeline dry run", "qa", goldenPipelineDryRun}, {"legacy local dry run", "local", goldenLocalDryRun}, diff --git a/cmd/commands/legacy_spellings_test.go b/cmd/commands/legacy_spellings_test.go index 43ca655b3..b4ad8ac7b 100644 --- a/cmd/commands/legacy_spellings_test.go +++ b/cmd/commands/legacy_spellings_test.go @@ -34,6 +34,8 @@ import ( "os" "strings" "testing" + + "github.com/nself-org/cli/internal/compat/compattest" ) // TestRewriteLegacyInvocation_ReleaseStatusSplicesArgs locks the argv-splice @@ -89,6 +91,7 @@ func TestRewriteLegacyInvocation_ReleaseStatusSplicesArgs(t *testing.T) { // status` against a project directory with no `release` plugin installed, // and asserts both reach the same final failure. func TestReleaseStatusLegacyAndDirectSpellingsFailIdentically(t *testing.T) { + compattest.Set(t, false) // D-0275: Execute() under v1.5 reshapes the shared root command and breaks every registration test after it runFromScratchDir(t) t.Setenv("HOME", t.TempDir()) // ensure no release plugin is "installed" via a real ~/.nself diff --git a/cmd/commands/secrets_core_logmask_test.go b/cmd/commands/secrets_core_logmask_test.go index dcb9c3087..9c3a85374 100644 --- a/cmd/commands/secrets_core_logmask_test.go +++ b/cmd/commands/secrets_core_logmask_test.go @@ -11,6 +11,7 @@ import ( "slices" "testing" + "github.com/nself-org/cli/internal/compat/compattest" "github.com/nself-org/cli/internal/errs" "github.com/nself-org/cli/internal/invoke" "github.com/spf13/cobra" @@ -28,6 +29,7 @@ func findCmdForMask(t *testing.T, path ...string) *cobra.Command { } func TestCommandSecretValues_Positionals(t *testing.T) { + compattest.Set(t, false) // D-0275: asserts v1.4 behaviour; v1.5 mode is not asserted by this test cases := []struct { name string path []string diff --git a/cmd/commands/ssl_acme_add_test.go b/cmd/commands/ssl_acme_add_test.go index 8bd20a6c4..26c37a7b8 100644 --- a/cmd/commands/ssl_acme_add_test.go +++ b/cmd/commands/ssl_acme_add_test.go @@ -17,6 +17,7 @@ import ( "testing" "time" + "github.com/nself-org/cli/internal/compat/compattest" "github.com/nself-org/cli/internal/ssl" "github.com/nself-org/cli/internal/ssl/acme" ) @@ -128,6 +129,7 @@ func TestSSLAddACME(t *testing.T) { } func TestSSLAddACMERefusals(t *testing.T) { + compattest.Set(t, false) // D-0275: asserts v1.4 behaviour; v1.5 mode is not asserted by this test f, _ := addFix(t, http.ErrServerClosed) if _, err := f.runAdd("app.example.org", "--acme", "--agree-tos"); err == nil || !strings.Contains(err.Error(), "does not answer the HTTP-01 challenge location") || len(f.runs) != 0 { diff --git a/cmd/commands/ssl_acme_test.go b/cmd/commands/ssl_acme_test.go index 1dead59d3..cee668930 100644 --- a/cmd/commands/ssl_acme_test.go +++ b/cmd/commands/ssl_acme_test.go @@ -30,6 +30,7 @@ import ( "github.com/spf13/cobra" "github.com/spf13/pflag" + "github.com/nself-org/cli/internal/compat/compattest" "github.com/nself-org/cli/internal/docker" "github.com/nself-org/cli/internal/ssl" "github.com/nself-org/cli/internal/ssl/acme" @@ -389,6 +390,7 @@ func acmeServed(t *testing.T, f *acmeFix) []byte { } func TestACMEFlagsRequireACME(t *testing.T) { + compattest.Set(t, false) // D-0275: asserts v1.4 behaviour; v1.5 mode is not asserted by this test f := newACMEFix(t) for _, fl := range []string{"--dry-run", "--agree-tos", "--nginx-container=x", "--adopt-certbot", "--lineage=x", "--dns-credential-file=x"} { if _, err := f.run(sslSetupCmd, fl); err == nil || !strings.Contains(err.Error(), "only works with --acme") { @@ -531,6 +533,7 @@ func TestSSLCertbotUnchanged(t *testing.T) { } func TestACMENginxFailureRollsBack(t *testing.T) { + compattest.Set(t, false) // D-0275: asserts v1.4 behaviour; v1.5 mode is not asserted by this test f := newACMEFix(t) if _, err := f.run(sslSetupCmd, "--acme", "--agree-tos"); err != nil { t.Fatal(err) diff --git a/cmd/commands/ssl_acme_traefik_test.go b/cmd/commands/ssl_acme_traefik_test.go index 226e81bb5..b421502ad 100644 --- a/cmd/commands/ssl_acme_traefik_test.go +++ b/cmd/commands/ssl_acme_traefik_test.go @@ -17,6 +17,7 @@ import ( "time" "github.com/nself-org/cli/internal/build" + "github.com/nself-org/cli/internal/compat/compattest" "github.com/nself-org/cli/internal/config" "github.com/nself-org/cli/internal/docker" "github.com/nself-org/cli/internal/ssl" @@ -105,6 +106,7 @@ func TestAcmeReloaderSelect(t *testing.T) { } func TestAcmeTraefikReloadRollback(t *testing.T) { + compattest.Set(t, false) // D-0275: asserts v1.4 behaviour; v1.5 mode is not asserted by this test f := newACMEFix(t) renders := f.useTraefik(2) // the 1st render is the setup install, the 2nd is the renewal's reload if _, err := f.run(sslSetupCmd, "--acme", "--agree-tos"); err != nil { diff --git a/cmd/commands/ssl_add_test.go b/cmd/commands/ssl_add_test.go index 3d81f1e88..14e1dbf8c 100644 --- a/cmd/commands/ssl_add_test.go +++ b/cmd/commands/ssl_add_test.go @@ -23,6 +23,8 @@ import ( "runtime" "strings" "testing" + + "github.com/nself-org/cli/internal/compat/compattest" ) // TestRunSSLAdd_NoProject_Errors verifies runSSLAdd refuses to proceed @@ -46,6 +48,7 @@ func TestRunSSLAdd_NoProject_Errors(t *testing.T) { // a project misconfigured in ANY way should fail on the most actionable // error, and "install certbot" must not be masked by a later check. func TestRunSSLAdd_NoCertbot_Errors(t *testing.T) { + compattest.Set(t, false) // D-0275: asserts v1.4 behaviour; v1.5 mode is not asserted by this test withProjectRoot(t, func(root string) { if err := os.WriteFile(filepath.Join(root, ".env"), []byte("ADMIN_EMAIL=ops@example.com\n"), 0o644); err != nil { t.Fatalf("write .env: %v", err) @@ -100,6 +103,7 @@ func stubBinDir(t *testing.T, name string) string { // one would either fail deep inside certbot with a confusing error or // register the account under no identity at all. func TestRunSSLAdd_MissingAdminEmail_Errors(t *testing.T) { + compattest.Set(t, false) // D-0275: asserts v1.4 behaviour; v1.5 mode is not asserted by this test withProjectRoot(t, func(root string) { if err := os.WriteFile(filepath.Join(root, ".env"), []byte("BASE_DOMAIN=example.com\n"), 0o644); err != nil { t.Fatalf("write .env: %v", err) diff --git a/cmd/commands/ssl_setup_test.go b/cmd/commands/ssl_setup_test.go index 8a413dddd..966fd5d10 100644 --- a/cmd/commands/ssl_setup_test.go +++ b/cmd/commands/ssl_setup_test.go @@ -21,6 +21,8 @@ import ( "path/filepath" "strings" "testing" + + "github.com/nself-org/cli/internal/compat/compattest" ) // resetSSLSetupFlags restores sslSetupCmd's flags to their registered @@ -46,6 +48,7 @@ func TestRunSSLSetup_NoProject_Errors(t *testing.T) { } func TestRunSSLSetup_NoCertbot_Errors(t *testing.T) { + compattest.Set(t, false) // D-0275: asserts v1.4 behaviour; v1.5 mode is not asserted by this test withProjectRoot(t, func(root string) { resetSSLSetupFlags(t) if err := os.WriteFile(filepath.Join(root, ".env"), []byte("BASE_DOMAIN=example.com\nADMIN_EMAIL=ops@example.com\n"), 0o644); err != nil { @@ -65,6 +68,7 @@ func TestRunSSLSetup_NoCertbot_Errors(t *testing.T) { // regression here would attempt (and fail deep inside certbot, or worse, // hang on an ACME challenge) issuance for a domain that can never validate. func TestRunSSLSetup_LocalhostDomain_Rejected(t *testing.T) { + compattest.Set(t, false) // D-0275: asserts v1.4 behaviour; v1.5 mode is not asserted by this test withProjectRoot(t, func(root string) { resetSSLSetupFlags(t) if err := os.WriteFile(filepath.Join(root, ".env"), []byte("BASE_DOMAIN=localhost\nADMIN_EMAIL=ops@example.com\n"), 0o644); err != nil { @@ -87,6 +91,7 @@ func TestRunSSLSetup_LocalhostDomain_Rejected(t *testing.T) { // checked one would silently register with an empty identity when the // other source was used. func TestRunSSLSetup_MissingEmail_FlagAndEnvBothEmpty_Errors(t *testing.T) { + compattest.Set(t, false) // D-0275: asserts v1.4 behaviour; v1.5 mode is not asserted by this test withProjectRoot(t, func(root string) { resetSSLSetupFlags(t) if err := os.WriteFile(filepath.Join(root, ".env"), []byte("BASE_DOMAIN=example.com\n"), 0o644); err != nil { @@ -108,6 +113,7 @@ func TestRunSSLSetup_MissingEmail_FlagAndEnvBothEmpty_Errors(t *testing.T) { // deep inside the certbot invocation with a confusing error, instead of // failing fast with a clear message naming the supported providers. func TestRunSSLSetup_UnsupportedProvider_Rejected(t *testing.T) { + compattest.Set(t, false) // D-0275: asserts v1.4 behaviour; v1.5 mode is not asserted by this test withProjectRoot(t, func(root string) { resetSSLSetupFlags(t) defer resetSSLSetupFlags(t) diff --git a/internal/repoqa/compat_ci_test.go b/internal/repoqa/compat_ci_test.go index 1b3700575..66e7e6603 100644 --- a/internal/repoqa/compat_ci_test.go +++ b/internal/repoqa/compat_ci_test.go @@ -10,7 +10,8 @@ import ( ) // TestV15SuiteInCI prevents the default compatibility mode from hiding v1.5 -// regressions. A required job must run the suite (cmd/commands narrowed, D-0275) with NSELF_V15=1. +// regressions. A required job must run the whole module, cmd/commands included +// (D-0275: named skips only), with NSELF_V15=1. func TestV15SuiteInCI(t *testing.T) { data, err := os.ReadFile(filepath.Join(repoRoot(t), ".github/workflows/ci.yml")) if err != nil { @@ -43,13 +44,36 @@ func TestV15SuiteInCI(t *testing.T) { if mode != "1" || step.ContinueOnError { continue } - // The step must test the module (go list ./...) and keep the Hasura - // remote tests of cmd/commands under v1.5. - if strings.Contains(step.Run, "go test") && strings.Contains(step.Run, "go list -mod=vendor ./...") && - strings.Contains(step.Run, "-run '^TestDBHasuraRemotePartialFailure$' ./cmd/commands") { + if runsV15Suite(step.Run) { return } } } - t.Fatal("CI must run the v1.5 suite (go list ./... plus cmd/commands Hasura tests) in a required job with NSELF_V15=1") + t.Fatal("CI must run the whole v1.5 suite, cmd/commands included, in a required job with NSELF_V15=1") +} + +// runsV15Suite reports whether a step's script tests the whole module: go test +// over ./... with nothing filtered out of the package list. Skipping named +// tests (-skip) is allowed; dropping a package is not. +func runsV15Suite(run string) bool { + return strings.Contains(run, "go test") && strings.Contains(run, " ./...") && + !strings.Contains(run, "grep -v") && !strings.Contains(run, "-run '") +} + +// TestRunsV15SuiteControls is the positive control: the old narrowed shapes are +// refused and the whole-module form is accepted. +func TestRunsV15SuiteControls(t *testing.T) { + for _, tc := range []struct { + name, run string + want bool + }{ + {"whole module with skips", "go test -mod=vendor -timeout 20m -skip '^(TestX)$' ./...", true}, + {"cmd/commands filtered out", "go test -mod=vendor $(go list -mod=vendor ./... | grep -v '/cmd/commands$')", false}, + {"only a -run selection", "go test -mod=vendor -run '^TestX$' ./cmd/commands", false}, + {"no go test", "echo ./...", false}, + } { + if got := runsV15Suite(tc.run); got != tc.want { + t.Errorf("%s: runsV15Suite = %v, want %v", tc.name, got, tc.want) + } + } }