Skip to content

Fix skills acceptance tests#13365

Merged
babakks merged 1 commit into
trunkfrom
wm-fix-skills-acceptance-tests
May 7, 2026
Merged

Fix skills acceptance tests#13365
babakks merged 1 commit into
trunkfrom
wm-fix-skills-acceptance-tests

Conversation

@williammartin

@williammartin williammartin commented May 7, 2026

Copy link
Copy Markdown
Member

Description

Two acceptance tests in TestSkills were failing due to code changes that weren't reflected in the test expectations:

skills-publish-dry-run: The nested skills discovery change (9a368f45e) made DiscoverLocalSkills walk deeper, so gh skill publish --dry-run $WORK was finding test-repo/skills/hello-world/SKILL.md inside $WORK instead of failing. Fixed by using an explicit empty directory for the "no skills" test case. Also removed a test case combining --fix and --dry-run which are now mutually exclusive flags.

skills-install-namespaced: PR #13266 intentionally flattened skill installs to use skill.Name instead of InstallName() because agent clients only discover immediate subdirectories. The test still expected namespaced directory layout (alice/deploy/). Updated to use distinct base names (alice-deploy, bob-deploy) and expect flat install paths.

Before

➜  wm-unhook-skills-telem git:(trunk) GH_ACCEPTANCE_HOST=github.com \
   GH_ACCEPTANCE_ORG=gh-acceptance-testing \
   GH_ACCEPTANCE_TOKEN=$(gh auth token) \
   go test -tags=acceptance -run ^TestSkills$ ./acceptance
--- FAIL: TestSkills (0.00s)
    --- FAIL: TestSkills/skills-publish-dry-run (0.70s)
        testscript.go:584: # Publish dry-run from a directory with no skills/ should fail gracefully (0.668s)
            > ! exec gh skill publish --dry-run $WORK
            [stdout]
            warning	hello-world	recommended field missing: license
            warning		not a git repository. Initialize with: git init && gh repo create
            [stderr]

            Dry run complete. Use without --dry-run to publish.
            FAIL: testdata/skills/skills-publish-dry-run.txtar:2: unexpected command success

    --- FAIL: TestSkills/skills-search-page (1.00s)
        testscript.go:584: # Pagination returns results on page 2 (0.976s)
            > exec gh skill search --owner github copilot --page 2
            [stderr]
            GitHub API rate limit exceeded. Please wait a minute and try again.
            [exit status 1]
            FAIL: testdata/skills/skills-search-page.txtar:2: unexpected command failure

    --- FAIL: TestSkills/skills-install-namespaced (15.19s)
        testscript.go:584: # Two namespaced skills with the same base name in the same repo should
            # be independently installable using path-based disambiguation.
            # Use gh as a credential helper (0.105s)
            # Create a repo with two namespaced skills that share the name "deploy" (5.369s)
            # Publish so the skills are discoverable (4.300s)
            # Install alice's deploy skill using the full path to disambiguate (2.179s)
            # Install bob's deploy skill using the full path (2.417s)
            # Verify both were installed to separate directories (0.701s)
            > exists $HOME/.copilot/skills/alice/deploy/SKILL.md
            FAIL: testdata/skills/skills-install-namespaced.txtar:35: $WORK/.copilot/skills/alice/deploy/SKILL.md does not exist

FAIL
FAIL	github.com/cli/cli/v2/acceptance	22.645s
FAIL

After

➜ GH_ACCEPTANCE_HOST=github.com \
   GH_ACCEPTANCE_ORG=gh-acceptance-testing \
   GH_ACCEPTANCE_TOKEN=$(gh auth token) \
   go test -tags=acceptance -run ^TestSkills$ ./acceptance
ok  	github.com/cli/cli/v2/acceptance	21.062s

@williammartin

Copy link
Copy Markdown
Member Author

CC @SamMorrowDrums and @tommaso-moro for manual review until we get you added back as reviewers.

@williammartin williammartin force-pushed the wm-fix-skills-acceptance-tests branch from 9f79bd3 to f47e459 Compare May 7, 2026 18:47
@williammartin williammartin marked this pull request as ready for review May 7, 2026 18:47
@williammartin williammartin requested a review from a team as a code owner May 7, 2026 18:47
@williammartin williammartin requested review from babakks and Copilot and removed request for Copilot May 7, 2026 18:47

@SamMorrowDrums SamMorrowDrums left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot approve, but this is tantamount to an approval

@babakks babakks merged commit b77d79c into trunk May 7, 2026
15 checks passed
@babakks babakks deleted the wm-fix-skills-acceptance-tests branch May 7, 2026 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants