Skip to content

chore: Fix the skill-target layout test that pointed at a removed directory - #2022

Merged
hatayama merged 2 commits into
feature/cli-discoverability-integrationfrom
chore/tool-skill-synchronizer-stale-target-test
Jul 27, 2026
Merged

chore: Fix the skill-target layout test that pointed at a removed directory#2022
hatayama merged 2 commits into
feature/cli-discoverability-integrationfrom
chore/tool-skill-synchronizer-stale-target-test

Conversation

@hatayama

@hatayama hatayama commented Jul 27, 2026

Copy link
Copy Markdown
Owner

Summary

  • Fixes the one failing EditMode test in ToolSkillSynchronizerTests, which was failing for a reason unrelated to what it verifies.

User Impact

None — test-only. Nothing about skill detection or installation changes.

Changes

DetectTargets_WhenGroupedLayoutRequested_DetectsEmptyFlatManagedDirectories built its fixture under .cursor. That directory stopped being a skill target when the target list was trimmed to Claude Code, Codex CLI, Common and Antigravity, so detection skipped it and returned no targets at all. The test then failed on Assert.That(detectedTargets.Length, Is.EqualTo(1)) before reaching the layout assertions it exists for.

The fixture now uses .claude, the directory every other single-target test in this file uses, so the test exercises the grouped-versus-flat layout detection it was written to cover.

SkillInstallLayoutTests named the same removed directory in one fixture. It passed either way because that API takes the directory name directly, but it is the same staleness, so it now uses .codex like the case beside it. No .cursor reference is left in the tests.

Triage result: the failure was in the test, not in the product. SkillTargetDetector correctly ignores directories that are not skill targets.

Verification

  • uloop run-tests (EditMode, ToolSkillSynchronizerTests|SkillInstallLayoutTests): 70/70 passed. Before this change ToolSkillSynchronizerTests was 51/52 with Expected: 1 But was: 0 at line 907.

The test built its fixture under .cursor, which stopped being a skill target in
#1983. Detection skipped the directory entirely, so the test asserted on an empty
result and failed on the count before it could check the layout state it exists to
verify. .claude is the directory every other single-target test in this fixture
uses.
@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The grouped-layout detection test now points its empty flat managed directory fixture at .claude instead of .cursor; all other setup and assertions remain unchanged.

Changes

Tool skill layout tests

Layer / File(s) Summary
Update empty flat directory fixture
Assets/Tests/Editor/ToolSkillSynchronizerTests.cs
The test’s targetRoot uses the .claude directory while preserving the existing setup and assertions.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly describes the test fix and mentions the removed directory causing the issue.
Description check ✅ Passed The description is directly related to the test-only change and matches the updated fixture and verification.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/tool-skill-synchronizer-stale-target-test

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

AreSkillsInstalled_WhenLegacyManagedDirectoryIsEmpty_StillDetectsFlatLayout still
named .cursor. It passes either way because the API takes the directory name
directly, but keeping a removed target in a fixture invites the same confusion the
sibling test just caused. .codex is what the neighbouring case uses.
@hatayama
hatayama merged commit 71161ca into feature/cli-discoverability-integration Jul 27, 2026
2 checks passed
@hatayama
hatayama deleted the chore/tool-skill-synchronizer-stale-target-test branch July 27, 2026 02:52
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.

1 participant