Add test to detect inline comments in Jinja2 expressions within YAML files

[dguido]

Summary

• Adds a comprehensive test to detect inline comments (#) within Jinja2 expressions in YAML task files
• This test would have caught the bug where comments inside {{ }} blocks caused Ansible template errors
• Includes regression test for the exact reported bug pattern

Background

A user reported a bug where inline comments within Jinja2 expressions in YAML files caused Ansible to fail with "unexpected char '#'" errors. The bug was in roles/strongswan/tasks/openssl.yml where comments like # Per-deployment UUID... were placed inside a Jinja2 expression.

While the bug has already been fixed, our existing tests didn't catch this issue because they only validated .j2 template files, not Jinja2 expressions embedded within YAML task files.

What This PR Does

The new test (tests/unit/test_yaml_jinja2_expressions.py):

1. Extracts and validates all Jinja2 expressions from YAML files - not just .j2 templates
2. Specifically detects inline comments within {{ }} and {% %} blocks
3. Includes a regression test that validates both the problematic pattern and the fixed version
4. Avoids false positives - correctly ignores # in strings, escaped hashes, and comments outside expressions

Test Results

✅ All 52 unit tests pass
✅ Regression test confirms it would have caught the bug
✅ No inline comments found in current codebase
✅ Ruff linting passed

Test Output

🔬 Testing specific OpenSSL expressions (regression test)...
  ⚠️ Problematic (with inline comments): failed with: Inline comment (#) found in Jinja2 expression
  ✅ Fixed (without inline comments): passed
  ✅ Regression test passed - would have caught the bug\!

📁 Found 74 YAML files to check
✅ All YAML files are free of inline comments in Jinja2 expressions\!

This test ensures that similar bugs won't be introduced in the future.

🤖 Generated with Claude Code

[claude]

Claude finished @dguido's task —— View job

---

PR Review: Add test to detect inline comments in Jinja2 expressions

I'll provide a comprehensive review of this pull request focusing on code quality, best practices, potential issues, performance, security, and test coverage.

Review Checklist:

• Read and analyze changed files
• Review test implementation quality and correctness
• Check CLAUDE.md updates for appropriateness
• Evaluate test coverage and effectiveness
• Check for potential bugs or issues
• Assess performance considerations
• Review security implications
• Provide constructive feedback

---

Overall Assessment: Excellent ✅

This is a high-quality PR that demonstrates best practices in defensive programming and comprehensive testing. The implementation is thorough, well-documented, and effectively addresses the problem.

Code Quality: Excellent

Strengths:

• Clean, readable code with excellent docstrings and comments
• Comprehensive edge case handling with 13+ test scenarios
• Follows mutation testing principles - verifies tests actually catch failures
• Robust error handling with graceful degradation for runtime issues
• Well-structured functions with single responsibilities

Minor suggestions:

• roles/strongswan/tasks/openssl.yml:64-82 - The multiline YAML expressions are correctly formatted without inline comments, confirming the bug was fixed

Test Implementation: Outstanding

Excellent aspects:

• Regression test (test_regression_openssl_inline_comments()) uses the exact problematic pattern from the reported bug
• Comprehensive edge cases covering strings, escaping, control blocks, multiline expressions
• Smart false positive prevention - correctly ignores # in strings, URLs, hex colors
• File discovery logic properly excludes .git, venv, configs directories
• Extensive mock context with 20+ realistic Ansible variables

Test coverage analysis:

# Covers the exact bug pattern:
problematic = """{{ [
  'DNS:' + id,  # comment here  ← This caused the original bug
] }}"""

# Tests all major expression types:
- {{ variable }} expressions  
- {% control %} blocks
- Multiline expressions
- Hash in strings: "string with # hash"
- Escaped hashes: \#

Security: No Concerns ✅

• Test code poses no security risks
• Uses safe regex patterns without ReDoS vulnerabilities
• Properly handles file reading with exception handling
• No credential exposure or unsafe operations

Performance: Well Optimized

Efficient design:

• Selective file scanning with glob patterns vs brute force directory traversal
• Early termination on YAML parsing errors to avoid unnecessary processing
• Compiled regex reuse through re.finditer()
• Smart skipping of non-Jinja2 files

Potential optimizations (minor):

• Consider caching compiled regex patterns if running frequently
• File content is read once per file (already optimal)

Documentation: Excellent

The CLAUDE.md addition on mutation testing is particularly valuable:

# Perfect example of mutation testing approach:
1. Write test for the bug
2. Temporarily introduce bug to verify test fails  
3. Fix bug and verify test passes
4. Document what the test prevents

This guidance will help future contributors write more effective tests.

Technical Deep Dive

Jinja2 Expression Extraction (extract_jinja2_expressions()):

• Uses proper regex with re.DOTALL for multiline support
• Captures both {{ }} and {% %} patterns
• Returns structured data with position info for error reporting

Validation Logic (validate_jinja2_expression()):

• Layered validation: Comment detection → Template compilation → Rendering
• Smart comment detection: Removes strings before checking for # to avoid false positives
• Comprehensive Ansible mocking: 15+ filters including version, ternary, b64encode

File Processing (validate_yaml_file()):

• Graceful YAML error handling - skips malformed files rather than failing
• Categorizes errors - separates inline comment issues from other syntax errors
• Provides context - includes line numbers and expression snippets

Regression Prevention

This test would have caught the original bug in roles/strongswan/tasks/openssl.yml where inline comments caused:

FAILED! => {"msg": "Unexpected templating type error occurred on ({{ [ ... # Per-deployment UUID... ] }}): unexpected char '#' at 87"}

The current file (lines 64-82) now correctly uses multiline YAML without inline comments in the Jinja2 expressions.

Minor Suggestions

1. Consider adding a CLI option to run just the inline comment checks for faster CI feedback
2. File pattern optimization: The current patterns are comprehensive but could add group_vars/**/*.yml for completeness
3. Error message enhancement: Could include the specific line content that triggered the error

Recommendation: Approve and Merge ✅

This PR represents excellent defensive programming practices:

• ✅ Comprehensive test coverage
• ✅ Clear documentation of testing approach
• ✅ Effective regression prevention
• ✅ No security concerns
• ✅ Performance optimized
• ✅ Follows project conventions

The mutation testing guidance added to CLAUDE.md is particularly valuable for the project's long-term code quality.

---