Expand test coverage for glob.rs brace validation before refactoring

[coderabbitai]

Context

The brace validation logic in src/manifest/glob.rs is complex with significant cyclomatic complexity (~47 conditional/control flow statements), but currently has only 3 test functions covering this code.

Problem

Before undertaking refactoring work proposed in #171 and #172, we need comprehensive test coverage to:

• Ensure current behaviour is well-documented through tests
• Provide a safety net during refactoring
• Prevent regressions
• Validate edge cases in brace matching, escape sequences, and state transitions

Requirements

Expand test coverage for:

• CharContext and BraceValidationState structures
• BraceValidator implementation including all state transitions
• Edge cases in brace matching (nested braces, escaped braces, malformed patterns)
• Platform-specific logic (Windows vs Unix path handling)
• validate_brace_matching function
• process_escape_sequence function

Acceptance Criteria

• Test coverage includes happy paths for all major code paths
• Edge cases and error conditions are tested
• Platform-specific behaviour is validated
• Tests are well-documented explaining what behaviour they validate

Blocked Issues

This issue blocks:

• Refactor glob pattern handling to use globwalk crate and remove custom parsing logic #171 - Consider using a dedicated crate for glob functionality
• Refactor glob.rs brace validation complexity #172 - Consider simplifying brace validation logic

References

• Original analysis: Refactor glob.rs brace validation complexity #172 (comment)
• Related file: src/manifest/glob.rs (525 lines)

[traycerai]

Plan

Observations

The src/manifest/glob.rs file contains complex brace validation logic with ~47 conditional/control flow statements but only 3 unit tests in the module's test section. While integration tests exist in tests/manifest_glob_tests.rs, they don't directly test the internal structures (CharContext, BraceValidationState, BraceValidator) or provide comprehensive coverage of edge cases and state transitions.

Approach

Expand the unit test section in src/manifest/glob.rs to comprehensively test all structures, methods, and edge cases. Tests will be organized by component (structures, validator methods, edge cases, platform-specific behavior) with clear documentation explaining the behavior being validated. This approach ensures the refactoring safety net is in place before undertaking work in issues #171 and #172.

Reasoning

Listed the repository structure, read the main implementation file src/manifest/glob.rs and the existing test file tests/manifest_glob_tests.rs to understand the current code complexity and identify gaps in test coverage for the brace validation logic.

Proposed File Changes

📄 src/manifest/glob.rs ^(MODIFY) 🔗

Expand the #[cfg(test)] mod tests section (currently lines 463-525) with comprehensive test coverage:

1. CharContext Tests:

• Add test for creating CharContext with various character types (alphanumeric, special chars, braces)
• Add test for CharContext with escaped flag set
• Add test for CharContext inside character class
• Add test for CharContext at different positions

2. BraceValidationState Tests:

• Add test for initial state (depth=0, in_class=false, last_open_pos=None)
• Add test for state with nested braces (depth > 1)
• Add test for state tracking last_open_pos correctly
• Add test for state with in_class flag set
• Add test for escape_active flag on Unix vs Windows

3. BraceValidator Tests:

Constructor:

• Add test verifying BraceValidator::new() creates correct initial state
• Add test verifying escape_active is true on Unix, false on Windows

handle_escape_sequence():

• Add test for backslash followed by brace on Unix (should escape)
• Add test for backslash followed by brace on Windows (should not escape)
• Add test for backslash at end of pattern
• Add test for double backslash
• Add test for backslash followed by non-special character
• Add test for escaped flag reset after processing escaped character

handle_character_class():

• Add test for entering character class with '['
• Add test for exiting character class with ']'
• Add test for nested brackets (e.g., "[[")
• Add test for brackets with braces inside (e.g., "[{}]")
• Add test that braces inside classes don't affect in_class state

handle_braces():

• Add test for opening brace incrementing depth
• Add test for closing brace decrementing depth
• Add test for closing brace when depth is 0 (should error)
• Add test for braces inside character class (should be ignored)
• Add test for last_open_pos tracking with multiple opening braces
• Add test for escaped braces not affecting depth (Unix only)

validate_final_state():

• Add test for valid final state (depth=0)
• Add test for invalid final state (depth > 0)
• Add test for invalid final state (depth < 0, though this shouldn't happen)
• Add test that error includes correct position from last_open_pos

process_character() integration:

• Add test for complete character processing flow
• Add test for character processing with escape sequences
• Add test for character processing inside character class
• Add test for character processing with nested braces

4. validate_brace_matching() Tests:

Happy paths:

• Add test for empty pattern
• Add test for pattern without braces
• Add test for single pair of balanced braces
• Add test for multiple separate balanced brace pairs
• Add test for nested balanced braces (2 levels)
• Add test for deeply nested balanced braces (4+ levels)
• Add test for braces with comma-separated alternatives

Error conditions:

• Add test for single opening brace
• Add test for single closing brace
• Add test for multiple unmatched opening braces
• Add test for multiple unmatched closing braces
• Add test for mismatched nested braces (e.g., "{a{b}")
• Add test for closing brace before opening
• Add test that error message includes correct position
• Add test that error message includes correct character

Edge cases with escapes (Unix):

• Add test for escaped opening brace (should not count)
• Add test for escaped closing brace (should not count)
• Add test for backslash before opening brace
• Add test for backslash before closing brace
• Add test for double backslash before brace
• Add test for escaped brace inside balanced braces
• Add test for pattern ending with backslash

Edge cases with character classes:

• Add test for braces inside character class (should be ignored)
• Add test for opening brace in class, closing outside
• Add test for closing brace in class, opening outside
• Add test for nested character classes with braces
• Add test for unclosed character class with braces

Complex patterns:

• Add test for mixed escaped and unescaped braces
• Add test for braces with wildcards (*, ?, **)
• Add test for braces in path separators context
• Add test for very long pattern with many braces
• Add test for pattern with all special characters

5. process_escape_sequence() Tests (Unix only):

• Add test for escaping asterisk (should become "[*]")
• Add test for escaping question mark (should become "[?]")
• Add test for escaping opening bracket (should become "[[]")
• Add test for escaping closing bracket (should become "[]]")
• Add test for escaping opening brace (should become "[{]")
• Add test for escaping closing brace (should become "[}]")
• Add test for backslash followed by non-special char (should keep backslash)
• Add test for backslash at end of string (should keep backslash)
• Add test for multiple consecutive escape sequences

6. Platform-specific Tests:

Unix-specific:

• Add test verifying escape_active is true
• Add test for backslash escape behavior
• Add test for force_literal_escapes() integration
• Add test for normalize_separators() with backslashes

Windows-specific:

• Add test verifying escape_active is false
• Add test that backslash doesn't escape braces
• Add test for backslash as path separator
• Add test for normalize_separators() with forward slashes

7. State Transition Tests:

• Add test for state transitions through complete pattern
• Add test for state reset after error
• Add test for state with multiple character classes
• Add test for state with alternating escapes and braces
• Add test for state tracking through nested structures

8. Error Message Tests:

• Add test verifying error includes pattern
• Add test verifying error includes position
• Add test verifying error includes character
• Add test verifying error type is correct
• Add test for error message format consistency

Each test should include a doc comment explaining what behavior it validates and why it's important for the refactoring safety net. Group related tests using nested modules or clear naming conventions (e.g., test_char_context_*, test_brace_validator_*, etc.).

Import In IDE

🤖 Prompt for AI Agents

I have created the following plan after thorough exploration and analysis of the codebase. Follow the below plan verbatim. Trust the files and references. Do not re-verify what's written in the plan. Explore only when absolutely necessary. First implement all the proposed file changes and then I'll review all the changes together at the end.

### Observations

The `src/manifest/glob.rs` file contains complex brace validation logic with ~47 conditional/control flow statements but only 3 unit tests in the module's test section. While integration tests exist in `tests/manifest_glob_tests.rs`, they don't directly test the internal structures (`CharContext`, `BraceValidationState`, `BraceValidator`) or provide comprehensive coverage of edge cases and state transitions.


### Approach

Expand the unit test section in `src/manifest/glob.rs` to comprehensively test all structures, methods, and edge cases. Tests will be organized by component (structures, validator methods, edge cases, platform-specific behavior) with clear documentation explaining the behavior being validated. This approach ensures the refactoring safety net is in place before undertaking work in issues #171 and #172.


### Reasoning

Listed the repository structure, read the main implementation file `src/manifest/glob.rs` and the existing test file `tests/manifest_glob_tests.rs` to understand the current code complexity and identify gaps in test coverage for the brace validation logic.


## Proposed File Changes

### src/manifest/glob.rs(MODIFY)

Expand the `#[cfg(test)] mod tests` section (currently lines 463-525) with comprehensive test coverage:

**1. CharContext Tests:**
- Add test for creating CharContext with various character types (alphanumeric, special chars, braces)
- Add test for CharContext with escaped flag set
- Add test for CharContext inside character class
- Add test for CharContext at different positions

**2. BraceValidationState Tests:**
- Add test for initial state (depth=0, in_class=false, last_open_pos=None)
- Add test for state with nested braces (depth > 1)
- Add test for state tracking last_open_pos correctly
- Add test for state with in_class flag set
- Add test for escape_active flag on Unix vs Windows

**3. BraceValidator Tests:**

**Constructor:**
- Add test verifying BraceValidator::new() creates correct initial state
- Add test verifying escape_active is true on Unix, false on Windows

**handle_escape_sequence():**
- Add test for backslash followed by brace on Unix (should escape)
- Add test for backslash followed by brace on Windows (should not escape)
- Add test for backslash at end of pattern
- Add test for double backslash
- Add test for backslash followed by non-special character
- Add test for escaped flag reset after processing escaped character

**handle_character_class():**
- Add test for entering character class with '['
- Add test for exiting character class with ']'
- Add test for nested brackets (e.g., "[[")
- Add test for brackets with braces inside (e.g., "[{}]")
- Add test that braces inside classes don't affect in_class state

**handle_braces():**
- Add test for opening brace incrementing depth
- Add test for closing brace decrementing depth
- Add test for closing brace when depth is 0 (should error)
- Add test for braces inside character class (should be ignored)
- Add test for last_open_pos tracking with multiple opening braces
- Add test for escaped braces not affecting depth (Unix only)

**validate_final_state():**
- Add test for valid final state (depth=0)
- Add test for invalid final state (depth > 0)
- Add test for invalid final state (depth < 0, though this shouldn't happen)
- Add test that error includes correct position from last_open_pos

**process_character() integration:**
- Add test for complete character processing flow
- Add test for character processing with escape sequences
- Add test for character processing inside character class
- Add test for character processing with nested braces

**4. validate_brace_matching() Tests:**

**Happy paths:**
- Add test for empty pattern
- Add test for pattern without braces
- Add test for single pair of balanced braces
- Add test for multiple separate balanced brace pairs
- Add test for nested balanced braces (2 levels)
- Add test for deeply nested balanced braces (4+ levels)
- Add test for braces with comma-separated alternatives

**Error conditions:**
- Add test for single opening brace
- Add test for single closing brace
- Add test for multiple unmatched opening braces
- Add test for multiple unmatched closing braces
- Add test for mismatched nested braces (e.g., "{a{b}")
- Add test for closing brace before opening
- Add test that error message includes correct position
- Add test that error message includes correct character

**Edge cases with escapes (Unix):**
- Add test for escaped opening brace (should not count)
- Add test for escaped closing brace (should not count)
- Add test for backslash before opening brace
- Add test for backslash before closing brace
- Add test for double backslash before brace
- Add test for escaped brace inside balanced braces
- Add test for pattern ending with backslash

**Edge cases with character classes:**
- Add test for braces inside character class (should be ignored)
- Add test for opening brace in class, closing outside
- Add test for closing brace in class, opening outside
- Add test for nested character classes with braces
- Add test for unclosed character class with braces

**Complex patterns:**
- Add test for mixed escaped and unescaped braces
- Add test for braces with wildcards (*, ?, **)
- Add test for braces in path separators context
- Add test for very long pattern with many braces
- Add test for pattern with all special characters

**5. process_escape_sequence() Tests (Unix only):**
- Add test for escaping asterisk (should become "[*]")
- Add test for escaping question mark (should become "[?]")
- Add test for escaping opening bracket (should become "[[]")
- Add test for escaping closing bracket (should become "[]]")
- Add test for escaping opening brace (should become "[{]")
- Add test for escaping closing brace (should become "[}]")
- Add test for backslash followed by non-special char (should keep backslash)
- Add test for backslash at end of string (should keep backslash)
- Add test for multiple consecutive escape sequences

**6. Platform-specific Tests:**

**Unix-specific:**
- Add test verifying escape_active is true
- Add test for backslash escape behavior
- Add test for force_literal_escapes() integration
- Add test for normalize_separators() with backslashes

**Windows-specific:**
- Add test verifying escape_active is false
- Add test that backslash doesn't escape braces
- Add test for backslash as path separator
- Add test for normalize_separators() with forward slashes

**7. State Transition Tests:**
- Add test for state transitions through complete pattern
- Add test for state reset after error
- Add test for state with multiple character classes
- Add test for state with alternating escapes and braces
- Add test for state tracking through nested structures

**8. Error Message Tests:**
- Add test verifying error includes pattern
- Add test verifying error includes position
- Add test verifying error includes character
- Add test verifying error type is correct
- Add test for error message format consistency

Each test should include a doc comment explaining what behavior it validates and why it's important for the refactoring safety net. Group related tests using nested modules or clear naming conventions (e.g., `test_char_context_*`, `test_brace_validator_*`, etc.).

---

Developer Humor

🧪 Testing, testing, one-two-three,\n> Braces nested deep as can be!\n> Escape sequences dance,\n> State transitions prance,\n> Now refactoring can flow worry-free! 🎭\n>\n> \n> { { { tests } } }\n> \\o/ \\o/ \\o/\n>

---

Execution Information

Branch: main
Commit: 7a272c4

---

💡 Tips

Supported Commands (Inside Comments)

• Use @traycerai generate to iterate on the previous version of the implementation plan.

Supported Commands (Inside Description)

• Add @traycerai ignore anywhere in the ticket description to prevent this ticket from being processed.
• Add @traycerai branch:<branch-name> anywhere in the ticket description to specify the target branch for the implementation plan.

Community

• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.