From c219743b2ea557ac6de01aaa8bd03dca59a19c4e Mon Sep 17 00:00:00 2001 From: William Martin Date: Fri, 7 Aug 2026 13:06:49 +0200 Subject: [PATCH 01/39] Add acceptance coverage for gist gist had no acceptance scripts, so its commands were never exercised against a real host. Cover create, view and delete in one script, and edit, rename and list in another. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- acceptance/acceptance_test.go | 9 +++ .../gist/gist-create-view-delete.txtar | 40 +++++++++++++ .../testdata/gist/gist-edit-rename-list.txtar | 60 +++++++++++++++++++ 3 files changed, 109 insertions(+) create mode 100644 acceptance/testdata/gist/gist-create-view-delete.txtar create mode 100644 acceptance/testdata/gist/gist-edit-rename-list.txtar diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 9030e050611..c8ae750a047 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -85,6 +85,15 @@ func TestAuth(t *testing.T) { testscript.Run(t, testScriptParamsFor(tsEnv, "auth")) } +func TestGists(t *testing.T) { + var tsEnv testScriptEnv + if err := tsEnv.fromEnv(); err != nil { + t.Fatal(err) + } + + testscript.Run(t, testScriptParamsFor(tsEnv, "gist")) +} + func TestGPGKeys(t *testing.T) { var tsEnv testScriptEnv if err := tsEnv.fromEnv(); err != nil { diff --git a/acceptance/testdata/gist/gist-create-view-delete.txtar b/acceptance/testdata/gist/gist-create-view-delete.txtar new file mode 100644 index 00000000000..86ed08f53fb --- /dev/null +++ b/acceptance/testdata/gist/gist-create-view-delete.txtar @@ -0,0 +1,40 @@ +# Gists are owned by the authenticated user rather than an org, so unlike most +# other acceptance scripts there is no repository to create or clean up. +# +# Not covered here: gists whose files exceed the API's 1MB inline limit come +# back truncated, and the full content is then fetched from a raw URL on +# gist.githubusercontent.com rather than the API host. Triggering that needs a +# file too large to keep in a txtar, so it is left to unit tests. + +# Setup useful env vars +env GIST_DESC=${SCRIPT_NAME}-${RANDOM_STRING} + +# Create a gist from a file, capturing its URL +exec gh gist create gist-file.txt --desc ${GIST_DESC} +stdout 'https://gist.github.com/' +stdout2env GIST_URL + +# View the gist and check the description and content are both rendered +exec gh gist view ${GIST_URL} +stdout ${GIST_DESC} +stdout 'hello from the acceptance tests' + +# List the file names in the gist +exec gh gist view ${GIST_URL} --files +stdout 'gist-file.txt' + +# View a single file raw, which must be the content we uploaded and nothing else +exec gh gist view ${GIST_URL} --filename gist-file.txt --raw +stdout 'hello from the acceptance tests' +! stdout ${GIST_DESC} + +# Delete the gist. This is deliberately not deferred because deletion is what +# the script is testing, and a deferred delete would fail on the second attempt. +exec gh gist delete --yes ${GIST_URL} + +# Check the gist is gone +! exec gh gist view ${GIST_URL} +stderr 'not found' + +-- gist-file.txt -- +hello from the acceptance tests diff --git a/acceptance/testdata/gist/gist-edit-rename-list.txtar b/acceptance/testdata/gist/gist-edit-rename-list.txtar new file mode 100644 index 00000000000..537e63b9295 --- /dev/null +++ b/acceptance/testdata/gist/gist-edit-rename-list.txtar @@ -0,0 +1,60 @@ +# Gists are owned by the authenticated user rather than an org, so unlike most +# other acceptance scripts there is no repository to create or clean up. +# +# Only --add is exercised here. Editing a file's content, and editing the +# description on its own, both fall through to an interactive editor, so they +# cannot be driven from a script. --remove is left out deliberately, because it +# issues the same update request as --add and so covers no further ground. +# +# Every update to an existing gist is followed by a sleep. A read that comes +# straight after an update serves the previous state around twice in ten +# attempts, and it settles within about a second and a half. The sleeps matter +# most before the next gh command rather than before an assertion: rename reads +# the gist to build its update, so a stale read there renames nothing at all and +# still exits 0, leaving the gist permanently wrong rather than briefly behind. +# Creating a gist reads back consistently, so it needs no sleep. + +# Setup useful env vars +env GIST_DESC=${SCRIPT_NAME}-${RANDOM_STRING} + +# Create a gist from a file, capturing its URL +exec gh gist create first.txt --desc ${GIST_DESC} +stdout2env GIST_URL + +# Defer gist cleanup +defer gh gist delete --yes ${GIST_URL} + +# Add a second file to the gist +exec gh gist edit ${GIST_URL} --add second.txt +sleep 5 + +# Check both files are now in the gist +exec gh gist view ${GIST_URL} --files +stdout 'first.txt' +stdout 'second.txt' + +# Rename the second file. Checking that the new name is present is enough to +# prove the rename happened, and asserting the absence of the old name would +# only add another chance to read the gist before the write has settled. +exec gh gist rename ${GIST_URL} second.txt renamed.txt +sleep 5 + +# Check the rename took effect +exec gh gist view ${GIST_URL} --files +stdout 'first.txt' +stdout 'renamed.txt' + +# Check the renamed file kept its content +exec gh gist view ${GIST_URL} --filename renamed.txt --raw +stdout 'the second file' + +# Find the gist by description, which lists secret gists by default +exec gh gist list --filter ${GIST_DESC} +stdout ${GIST_DESC} +stdout '2 files' +stdout 'secret' + +-- first.txt -- +the first file +-- second.txt -- +the second file From bceaa3b3146c1908f8e5a58d75fd160167a4628a Mon Sep 17 00:00:00 2001 From: William Martin Date: Fri, 7 Aug 2026 13:06:57 +0200 Subject: [PATCH 02/39] Wait longer for the search index in the issues script search-issues flakes because gh search reads a separate index that lags issue creation, and five seconds was not reliably enough for it to catch up. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- acceptance/testdata/search/search-issues.txtar | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/acceptance/testdata/search/search-issues.txtar b/acceptance/testdata/search/search-issues.txtar index 82184f3f1a5..b44bb86eb1f 100644 --- a/acceptance/testdata/search/search-issues.txtar +++ b/acceptance/testdata/search/search-issues.txtar @@ -12,8 +12,10 @@ cd $SCRIPT_NAME-$RANDOM_STRING exec gh issue create --title 'Feature Request' --body $RANDOM_STRING -# It takes some time for the issue to be created and indexed -sleep 5 +# It takes some time for the issue to be created and indexed. Search reads a +# separate index rather than the issue itself, so this wait is longer than the +# five seconds used elsewhere, which was not enough to stop this flaking. +sleep 20 # Search for the issue exec gh search issues $RANDOM_STRING -R $ORG/$SCRIPT_NAME-$RANDOM_STRING From 0f3ab2203f8439b3707cb642842ab05dca66677c Mon Sep 17 00:00:00 2001 From: William Martin Date: Fri, 7 Aug 2026 13:07:41 +0200 Subject: [PATCH 03/39] Let GH_ACCEPTANCE_SCRIPT name several scripts The variable took a single script name, so running a chosen subset meant one go test invocation per script. Accept a comma separated list and select the ones belonging to the command directory under test. A filter that matches nothing in a directory now skips that directory rather than falling back to running all of it, so a mistyped script name reports as a skip instead of silently passing a full run. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- acceptance/acceptance_test.go | 65 +++++++------- acceptance/scriptfilter_test.go | 36 ++++++++ acceptance/scriptfilter_unit_test.go | 128 +++++++++++++++++++++++++++ 3 files changed, 199 insertions(+), 30 deletions(-) create mode 100644 acceptance/scriptfilter_test.go create mode 100644 acceptance/scriptfilter_unit_test.go diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index c8ae750a047..d92d37654cb 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -73,7 +73,7 @@ func TestAPI(t *testing.T) { t.Fatal(err) } - testscript.Run(t, testScriptParamsFor(tsEnv, "api")) + testscript.Run(t, testScriptParamsFor(t, tsEnv, "api")) } func TestAuth(t *testing.T) { @@ -82,7 +82,7 @@ func TestAuth(t *testing.T) { t.Fatal(err) } - testscript.Run(t, testScriptParamsFor(tsEnv, "auth")) + testscript.Run(t, testScriptParamsFor(t, tsEnv, "auth")) } func TestGists(t *testing.T) { @@ -91,7 +91,7 @@ func TestGists(t *testing.T) { t.Fatal(err) } - testscript.Run(t, testScriptParamsFor(tsEnv, "gist")) + testscript.Run(t, testScriptParamsFor(t, tsEnv, "gist")) } func TestGPGKeys(t *testing.T) { @@ -100,7 +100,7 @@ func TestGPGKeys(t *testing.T) { t.Fatal(err) } - testscript.Run(t, testScriptParamsFor(tsEnv, "gpg-key")) + testscript.Run(t, testScriptParamsFor(t, tsEnv, "gpg-key")) } func TestExtensions(t *testing.T) { @@ -109,7 +109,7 @@ func TestExtensions(t *testing.T) { t.Fatal(err) } - testscript.Run(t, testScriptParamsFor(tsEnv, "extension")) + testscript.Run(t, testScriptParamsFor(t, tsEnv, "extension")) } func TestIssues(t *testing.T) { @@ -118,7 +118,7 @@ func TestIssues(t *testing.T) { t.Fatal(err) } - testscript.Run(t, testScriptParamsFor(tsEnv, "issue")) + testscript.Run(t, testScriptParamsFor(t, tsEnv, "issue")) } func TestDiscussions(t *testing.T) { @@ -127,7 +127,7 @@ func TestDiscussions(t *testing.T) { t.Fatal(err) } - testscript.Run(t, testScriptParamsFor(tsEnv, "discussion")) + testscript.Run(t, testScriptParamsFor(t, tsEnv, "discussion")) } func TestIssues2_0(t *testing.T) { @@ -136,7 +136,7 @@ func TestIssues2_0(t *testing.T) { t.Fatal(err) } - testscript.Run(t, testScriptParamsFor(tsEnv, "issues-2.0")) + testscript.Run(t, testScriptParamsFor(t, tsEnv, "issues-2.0")) } func TestLabels(t *testing.T) { @@ -145,7 +145,7 @@ func TestLabels(t *testing.T) { t.Fatal(err) } - testscript.Run(t, testScriptParamsFor(tsEnv, "label")) + testscript.Run(t, testScriptParamsFor(t, tsEnv, "label")) } func TestOrg(t *testing.T) { @@ -154,7 +154,7 @@ func TestOrg(t *testing.T) { t.Fatal(err) } - testscript.Run(t, testScriptParamsFor(tsEnv, "org")) + testscript.Run(t, testScriptParamsFor(t, tsEnv, "org")) } func TestProject(t *testing.T) { @@ -163,7 +163,7 @@ func TestProject(t *testing.T) { t.Fatal(err) } - testscript.Run(t, testScriptParamsFor(tsEnv, "project")) + testscript.Run(t, testScriptParamsFor(t, tsEnv, "project")) } func TestPullRequests(t *testing.T) { @@ -172,7 +172,7 @@ func TestPullRequests(t *testing.T) { t.Fatal(err) } - testscript.Run(t, testScriptParamsFor(tsEnv, "pr")) + testscript.Run(t, testScriptParamsFor(t, tsEnv, "pr")) } func TestReleases(t *testing.T) { @@ -181,7 +181,7 @@ func TestReleases(t *testing.T) { t.Fatal(err) } - testscript.Run(t, testScriptParamsFor(tsEnv, "release")) + testscript.Run(t, testScriptParamsFor(t, tsEnv, "release")) } func TestRepo(t *testing.T) { @@ -190,7 +190,7 @@ func TestRepo(t *testing.T) { t.Fatal(err) } - testscript.Run(t, testScriptParamsFor(tsEnv, "repo")) + testscript.Run(t, testScriptParamsFor(t, tsEnv, "repo")) } func TestRulesets(t *testing.T) { @@ -199,7 +199,7 @@ func TestRulesets(t *testing.T) { t.Fatal(err) } - testscript.Run(t, testScriptParamsFor(tsEnv, "ruleset")) + testscript.Run(t, testScriptParamsFor(t, tsEnv, "ruleset")) } func TestSearches(t *testing.T) { @@ -208,7 +208,7 @@ func TestSearches(t *testing.T) { t.Fatal(err) } - testscript.Run(t, testScriptParamsFor(tsEnv, "search")) + testscript.Run(t, testScriptParamsFor(t, tsEnv, "search")) } func TestSecrets(t *testing.T) { @@ -217,7 +217,7 @@ func TestSecrets(t *testing.T) { t.Fatal(err) } - testscript.Run(t, testScriptParamsFor(tsEnv, "secret")) + testscript.Run(t, testScriptParamsFor(t, tsEnv, "secret")) } func TestSSHKeys(t *testing.T) { @@ -226,7 +226,7 @@ func TestSSHKeys(t *testing.T) { t.Fatal(err) } - testscript.Run(t, testScriptParamsFor(tsEnv, "ssh-key")) + testscript.Run(t, testScriptParamsFor(t, tsEnv, "ssh-key")) } func TestVariables(t *testing.T) { @@ -235,7 +235,7 @@ func TestVariables(t *testing.T) { t.Fatal(err) } - testscript.Run(t, testScriptParamsFor(tsEnv, "variable")) + testscript.Run(t, testScriptParamsFor(t, tsEnv, "variable")) } func TestWorkflows(t *testing.T) { @@ -244,7 +244,7 @@ func TestWorkflows(t *testing.T) { t.Fatal(err) } - testscript.Run(t, testScriptParamsFor(tsEnv, "workflow")) + testscript.Run(t, testScriptParamsFor(t, tsEnv, "workflow")) } func TestTelemetry(t *testing.T) { @@ -253,18 +253,21 @@ func TestTelemetry(t *testing.T) { t.Fatal(err) } - testscript.Run(t, testScriptParamsFor(tsEnv, "telemetry")) + testscript.Run(t, testScriptParamsFor(t, tsEnv, "telemetry")) } -func testScriptParamsFor(tsEnv testScriptEnv, command string) testscript.Params { - var files []string - if tsEnv.script != "" { - files = []string{path.Join("testdata", command, tsEnv.script)} - } +func testScriptParamsFor(t *testing.T, tsEnv testScriptEnv, command string) testscript.Params { + t.Helper() + files, filtered := selectScripts(command, tsEnv.scripts) var dir string - if len(files) == 0 { + if !filtered { + // No filter was set - run everything in the directory. dir = path.Join("testdata", command) + } else if len(files) == 0 { + // A filter was set but none of the selected scripts belong to this + // command directory, so skip rather than running the whole directory. + t.Skipf("testdata/%s: no selected script belongs to this command directory", command) } return testscript.Params{ @@ -570,7 +573,9 @@ type testScriptEnv struct { org string token string - script string + // scripts optionally narrows a run to named scripts within the command + // directory being run. Empty means run every script in the directory. + scripts []string skipDefer bool preserveWorkDir bool @@ -608,7 +613,7 @@ func (e *testScriptEnv) fromEnv() error { e.org = envMap["GH_ACCEPTANCE_ORG"] e.token = envMap["GH_ACCEPTANCE_TOKEN"] - e.script = os.Getenv("GH_ACCEPTANCE_SCRIPT") + e.scripts = parseScriptFilter(os.Getenv("GH_ACCEPTANCE_SCRIPT")) e.preserveWorkDir = os.Getenv("GH_ACCEPTANCE_PRESERVE_WORK_DIR") == "true" e.skipDefer = os.Getenv("GH_ACCEPTANCE_SKIP_DEFER") == "true" @@ -620,5 +625,5 @@ func TestSkills(t *testing.T) { if err := tsEnv.fromEnv(); err != nil { t.Fatal(err) } - testscript.Run(t, testScriptParamsFor(tsEnv, "skills")) + testscript.Run(t, testScriptParamsFor(t, tsEnv, "skills")) } diff --git a/acceptance/scriptfilter_test.go b/acceptance/scriptfilter_test.go new file mode 100644 index 00000000000..c76c56e95c2 --- /dev/null +++ b/acceptance/scriptfilter_test.go @@ -0,0 +1,36 @@ +package acceptance_test + +import ( + "os" + "path" + "strings" +) + +// parseScriptFilter splits a comma-separated GH_ACCEPTANCE_SCRIPT value into +// individual script names, trimming whitespace and ignoring empty entries. +func parseScriptFilter(raw string) []string { + var scripts []string + for _, s := range strings.Split(raw, ",") { + if s = strings.TrimSpace(s); s != "" { + scripts = append(scripts, s) + } + } + return scripts +} + +// selectScripts returns the script files under testdata/command that match the +// requested names, and reports whether a filter was applied (i.e. scripts is +// non-empty). A named script not found in the directory is silently ignored +// because it belongs to another command directory in the same run. +func selectScripts(command string, scripts []string) (files []string, filtered bool) { + if len(scripts) == 0 { + return nil, false + } + for _, script := range scripts { + p := path.Join("testdata", command, script) + if _, err := os.Stat(p); err == nil { + files = append(files, p) + } + } + return files, true +} diff --git a/acceptance/scriptfilter_unit_test.go b/acceptance/scriptfilter_unit_test.go new file mode 100644 index 00000000000..9de13bc3335 --- /dev/null +++ b/acceptance/scriptfilter_unit_test.go @@ -0,0 +1,128 @@ +package acceptance_test + +import ( + "os" + "path" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestParseScriptFilter(t *testing.T) { + tests := []struct { + name string + input string + want []string + }{ + { + name: "empty string returns nil", + input: "", + want: nil, + }, + { + name: "single name", + input: "repo-clone.txtar", + want: []string{"repo-clone.txtar"}, + }, + { + name: "two names", + input: "repo-clone.txtar,workflow-list.txtar", + want: []string{"repo-clone.txtar", "workflow-list.txtar"}, + }, + { + name: "whitespace around entries is trimmed", + input: " repo-clone.txtar , workflow-list.txtar ", + want: []string{"repo-clone.txtar", "workflow-list.txtar"}, + }, + { + name: "empty entries between commas are ignored", + input: "repo-clone.txtar,,workflow-list.txtar", + want: []string{"repo-clone.txtar", "workflow-list.txtar"}, + }, + { + name: "whitespace-only entries are ignored", + input: "repo-clone.txtar, ,workflow-list.txtar", + want: []string{"repo-clone.txtar", "workflow-list.txtar"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := parseScriptFilter(tt.input) + assert.Equal(t, tt.want, got) + }) + } +} + +// TestSelectScripts exercises the real selectScripts function, verifying that +// it correctly matches files in the command directory, skips names belonging to +// other directories, and reports whether a filter was applied. +func TestSelectScripts(t *testing.T) { + // Build a temporary testdata tree and change into it so selectScripts can + // resolve "testdata//