Skip to content

feat: enhance E2E test coverage for all tool x feature combinations#1449

Merged
dyoshikawa merged 4 commits intomainfrom
enhance-e2e
Apr 8, 2026
Merged

feat: enhance E2E test coverage for all tool x feature combinations#1449
dyoshikawa merged 4 commits intomainfrom
enhance-e2e

Conversation

@dyoshikawa
Copy link
Copy Markdown
Owner

Summary

  • Expand E2E tests from ~160 to 294 test cases, covering all normal (happy-path) cases from the README's tool × feature matrix (25 tools × 7 features)
  • Add generate, orphan check (for isDeletable=true tools), and import tests for all supported tool × feature combinations including project, global, and simulated modes
  • Fix a bug in generate.ts where --delete --check mode with orphan files was bypassed when totalGenerated === 0 (early return skipped hasDiff validation)

Changes

  • src/cli/commands/generate.ts: Fix early return bug — remove return when totalGenerated === 0 so --check mode still validates hasDiff from orphan detection
  • src/e2e/e2e-helper.ts: Add simulateCommands and simulateSkills parameters to runGenerate
  • src/e2e/e2e-rules.spec.ts: Add 18 new tools to generate tests (root + non-root + global), expand import tests to 23 tools
  • src/e2e/e2e-ignore.spec.ts: Add 11 new tools to generate tests, add zed JSON format support, expand import tests to 11 tools
  • src/e2e/e2e-commands.spec.ts: Add 8 local + 2 simulated + 5 global generate tests, expand import tests to 10 tools
  • src/e2e/e2e-subagents.spec.ts: Add 5 local + 3 simulated + 1 global generate tests, expand import tests to 9 tools (including kiro JSON format)
  • src/e2e/e2e-skills.spec.ts: Add 13 local + 2 simulated + 7 global generate tests, expand import tests to 15 tools
  • src/e2e/e2e-mcp.spec.ts: Add 10 local + 8 global generate tests, fix orphan tests with proper source files, expand import tests to 9 tools
  • src/e2e/e2e-hooks.spec.ts: Add 4 local + 6 global generate tests, expand import tests to 5 tools

Path corrections

  • replit skills: .replit/.agents/
  • codexcli subagents: .md.toml
  • kiro subagents: .md.json
  • junie MCP: .junie/settings/.junie/mcp/
  • opencode/kilo MCP: .json.jsonc
  • antigravity skills global: .agent/.gemini/antigravity/

Test plan

  • pnpm test:e2e — 294 tests passing
  • pnpm cicheck — all checks passing (format, lint, typecheck, unit tests, spell check, secret lint)

🤖 Generated with Claude Code

Expand E2E tests to cover all normal (happy-path) cases from the
README's tool x feature matrix. This includes generate, orphan check,
and import tests for rules, ignore, MCP, commands, subagents, skills,
and hooks features across all 25 supported tools.

Key changes:
- Add generate tests for all tool x feature combinations (project,
  global, and simulated modes)
- Add orphan check tests for deletable tool files, excluding
  isDeletable=false tools (merged settings files)
- Add import tests for all tools per feature
- Fix --delete --check bug where orphan detection was bypassed when
  totalGenerated was 0 (early return skipped hasDiff check)
- Add --simulate-commands and --simulate-skills flags to e2e-helper
- Fix incorrect output paths: replit skills (.agents/), codexcli
  subagents (.toml), kiro subagents (.json), junie MCP, opencode/kilo
  MCP (.jsonc), antigravity skills global

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 6, 2026 14:22
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Expands Rulesync’s E2E suite to cover more tool × feature combinations (including simulated + global modes) and adjusts generate behavior so --delete --check correctly fails when orphan files are detected even if no files are written.

Changes:

  • Fixed rulesync generate control flow so --check validates hasDiff even when totalGenerated === 0.
  • Extended E2E helpers to support additional simulation flags (--simulate-commands, --simulate-skills).
  • Significantly expanded E2E coverage across rules/ignore/mcp/commands/subagents/skills/hooks, including additional targets and global-mode cases.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/cli/commands/generate.ts Removes early return so --check can fail on orphan diffs even when no files are generated.
src/e2e/e2e-helper.ts Adds helper support for simulate flags when invoking rulesync generate.
src/e2e/e2e-rules.spec.ts Broadens rules generate/import/global-mode coverage for many more targets.
src/e2e/e2e-ignore.spec.ts Adds ignore generation/import/orphan tests for additional tools and JSON-format support for Zed.
src/e2e/e2e-mcp.spec.ts Expands MCP generation/import/global coverage and improves orphan test setup with valid source config.
src/e2e/e2e-commands.spec.ts Adds many more command generation/import/global tests plus simulated command coverage.
src/e2e/e2e-subagents.spec.ts Adds more subagent generation/import/global tests plus simulated subagent coverage and format-specific paths.
src/e2e/e2e-skills.spec.ts Adds more skills generation/import/global tests plus simulated skills coverage.
src/e2e/e2e-hooks.spec.ts Adds more hooks targets and introduces global-mode hooks generation tests.

@@ -122,22 +122,23 @@ export async function generateCommand(logger: Logger, options: GenerateOptions):
if (totalGenerated === 0) {
Copy link

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

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

When totalGenerated === 0 you log "✓ All files are up to date" unconditionally, but result.hasDiff can still be true (e.g., --delete finds orphan files without writing new files). This produces incorrect/contradictory output (especially in --check mode where it will then throw). Consider only printing the up-to-date message when !result.hasDiff, and letting the --check block handle messaging, or otherwise basing this branch on result.hasDiff rather than totalGenerated.

Suggested change
if (totalGenerated === 0) {
if (!result.hasDiff) {

Copilot uses AI. Check for mistakes.
await writeFileContent(join(testDir, ".rulesync", ".gitkeep"), "");
await writeFileContent(join(testDir, orphanPath), "# orphan\n");
await writeFileContent(
join(testDir, ".rulesync", "mcp.json"),
Copy link

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

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

This test hardcodes the Rulesync source path as .rulesync/mcp.json instead of using the existing RULESYNC_MCP_RELATIVE_FILE_PATH constant. Using the constant keeps tests aligned with path changes and avoids duplicating Rulesync internal paths.

Suggested change
join(testDir, ".rulesync", "mcp.json"),
join(testDir, RULESYNC_MCP_RELATIVE_FILE_PATH),

Copilot uses AI. Check for mistakes.
// Verify that the imported rule file was created
// Note: The imported file keeps the original filename (CLAUDE.md), not overview.md
const importedRulePath = join(testDir, ".rulesync", "rules", "CLAUDE.md");
const importedRulePath = join(testDir, ".rulesync", "rules", importedFileName);
Copy link

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

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

This test constructs the imported Rulesync path via join(testDir, ".rulesync", "rules", ...) instead of using RULESYNC_RULES_RELATIVE_DIR_PATH. Using the constant avoids hardcoding Rulesync internal directories and makes future path refactors less error-prone.

Suggested change
const importedRulePath = join(testDir, ".rulesync", "rules", importedFileName);
const importedRulePath = join(
testDir,
RULESYNC_RULES_RELATIVE_DIR_PATH,
importedFileName,
);

Copilot uses AI. Check for mistakes.
dyoshikawa and others added 2 commits April 7, 2026 03:07
- Add unit tests covering the generate.ts --check mode bug where
  totalGenerated === 0 + hasDiff === true (orphan-only diffs).
- Strengthen e2e-hooks.spec assertions: previously the catch-all branch
  only checked that 'hooks' key existed; now verifies the configured
  command paths are preserved (event-name casing varies per tool, e.g.
  geminicli maps stop to AfterAgent).
- Fix JunieRule.fromFile to read from .junie/guidelines.md (it was
  incorrectly reading from baseDir/guidelines.md, breaking rules import
  for junie). Update junie-rule unit tests accordingly.
- Re-enable junie in the e2e rules import test now that the path bug is
  fixed.
- Replace the copilot hooks excluded comment with a proper import test
  that uses copilot's flat { type, bash } entry schema instead of the
  canonical { matcher, hooks: [...] } shape.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- junie-rule.ts: extract isRootRelativeFilePath helper to remove the
  duplicated literal check, and reuse getSettablePaths() values instead
  of hardcoding '.junie' in fromFile so the path resolution stays
  consistent with getSettablePaths.
- generate.test.ts: strengthen the orphan-only check-mode regression
  test by capturing loadToolFiles/removeOrphanAiFiles mocks and
  asserting they were invoked, and assert on ErrorCodes.GENERATION_FAILED
  instead of the human-readable error message. Split the no-diff
  success test into delete-enabled and delete-disabled cases so each
  variant exercises a distinct code path.
- e2e-hooks.spec.ts: extract assertHookCommandsPreserved helper to
  centralize the command-path assertion rationale and remove duplicated
  JSON.stringify checks. Document why copilot intentionally cannot
  assert audit.sh (copilot does not support the stop hook event).
Copilot AI review requested due to automatic review settings April 7, 2026 11:59
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

@@ -122,22 +122,23 @@ export async function generateCommand(logger: Logger, options: GenerateOptions):
if (totalGenerated === 0) {
const enabledFeatures = features.join(", ");
logger.info(`✓ All files are up to date (${enabledFeatures})`);
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

When totalGenerated === 0 you always log "✓ All files are up to date (...)" even if result.hasDiff is true (e.g., only orphan deletions detected with --delete --check). This produces contradictory output ("up to date" followed by a failing check) and can mislead CI logs. Consider conditioning this message on !result.hasDiff (or emitting a different message when diffs are only from orphan deletions).

Suggested change
logger.info(`✓ All files are up to date (${enabledFeatures})`);
if (result.hasDiff) {
logger.info(`${modePrefix} Changes detected in generated files (${enabledFeatures})`);
} else {
logger.info(`✓ All files are up to date (${enabledFeatures})`);
}

Copilot uses AI. Check for mistakes.
@dyoshikawa dyoshikawa merged commit 1d52e8f into main Apr 8, 2026
9 checks passed
@dyoshikawa
Copy link
Copy Markdown
Owner Author

@dyoshikawa Thank you!

@dyoshikawa dyoshikawa deleted the enhance-e2e branch April 8, 2026 01:43
@github-actions github-actions bot mentioned this pull request Apr 8, 2026
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.

2 participants