[test-improver] Improve tests for config/rules package

[github-actions]

File Analyzed

• Test File: internal/config/rules/rules_test.go
• Package: internal/config/rules
• Lines of Code: 731 → 810 (+79 lines net)

Improvements Made

1. Better Testing Patterns

• ✅ require.NotNil → require.Error: Replaced require.NotNil(t, err) with require.Error(t, err) in TestPortRange, TestTimeoutPositive, and TestMountFormat — more idiomatic testify for error-path assertions
• ✅ Raw t.Errorf → assert.Nil: Replaced manual if err != nil { t.Errorf(...) } patterns in TestNonEmptyString and TestAbsolutePath success paths with assert.Nil(t, err) — consistent use of testify throughout
• ✅ Descriptive test names: Disambiguated the single test case "unsupported server type" → "unsupported server type grpc" for clarity in failure output
• ✅ Added fmt import needed for the fmt.Sprintf path fix

2. Increased Coverage

• ✅ TestUnsupportedType: Added "unsupported server type websocket" and "unsupported empty type" test cases (1 → 3 cases)
• ✅ TestUndefinedVariable: Added PAT token (GITHUB_PERSONAL_ACCESS_TOKEN) and config dir (GITHUB_CONFIG_DIR) variable test cases (1 → 3 cases)
• ✅ TestUnsupportedField: Added "unsupported args field" and "unsupported field with empty suggestion" test cases (1 → 3 cases)
• ✅ TestNonEmptyString: Added "valid whitespace-only string is not empty" edge case — documents and verifies that NonEmptyString only rejects "", not " " (4 → 5 cases)

3. Cleaner & More Stable Tests

• ✅ Fixed fragile expectedPath computation in TestMountFormat: The original code had:

  expectedPath := "mcpServers.github.mounts[0]"
  if tt.index != 0 {
      expectedPath = "mcpServers.github.mounts[1]"
  }

  This was incorrect for any index > 1 (would return [1] instead of the actual index). Replaced with:

  assert.Equal(t, fmt.Sprintf("%s.mounts[%d]", tt.jsonPath, tt.index), err.JSONPath, ...)

  This correctly handles any index value and also uses tt.jsonPath rather than hardcoded "mcpServers.github".

Why These Changes?

internal/config/rules/rules_test.go was selected because it had a mix of testify patterns and inconsistent assertion styles. The file had raw t.Errorf patterns in success branches where testify assertions should be used, require.NotNil where require.Error is more descriptive for error values, and multiple single-case table tests that provided poor coverage signal. The fragile expectedPath computation was a latent bug — any test case with index > 1 would produce incorrect assertions.

---

Generated by Test Improver Workflow
Focuses on better patterns, increased coverage, and more stable tests

AI generated by Test Improver

[Copilot]

Pull request overview

This PR improves the test quality and coverage for the internal/config/rules package, focusing on better testing patterns, increased test coverage, and fixing a latent bug in the JSONPath assertion logic. The changes align with the codebase's established testify assertion patterns and add meaningful test cases that exercise edge cases and different code paths.

Changes:

• Replaced require.NotNil with require.Error and raw t.Errorf with assert.Nil for consistent testify usage
• Fixed a bug in TestMountFormat where the expected JSONPath computation was hardcoded and would fail for indices > 1
• Added 9 new test cases across 4 test functions to improve coverage of edge cases and different scenarios

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.