Skip to content

Conversation

@dsarno
Copy link
Owner

@dsarno dsarno commented Sep 6, 2025

Summary

  • ensure canonicalization script always respects fragment file IDs
  • tighten NL-3 regex to avoid cross-matching titles like "End-of-Class Helper"

Testing

  • pytest -q

https://chatgpt.com/codex/tasks/task_e_68bbc9dea89c8327ae1610674fb9549d

@coderabbitai
Copy link

coderabbitai bot commented Sep 6, 2025

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch codex/fix-testcase-naming-to-respect-file-id

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 and usage tips.

@dsarno dsarno merged commit 40463d1 into nl-CI-workflow-fixes Sep 6, 2025
1 check passed
@dsarno dsarno deleted the codex/fix-testcase-naming-to-respect-file-id branch September 6, 2025 05:46
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Greptile Summary

This PR fixes a critical test identification issue in the Claude NL suite workflow by refining the test canonicalization logic. The changes address a problem where the NL-3 regex pattern was too broad, causing test cases with titles like "End-of-Class Helper" to be incorrectly matched and renamed, leading to T-D tests being misclassified as NL-3 tests.

The fix involves two key modifications to the GitHub workflow file:

  1. Tightened NL-3 regex pattern: The pattern now requires more specific matching criteria - either a "Content" suffix or specific "test [ABC]" patterns instead of the generic "Class" matching that was causing false positives.

  2. Enhanced canonicalization priority: The logic now always respects filename-derived test IDs when present, ensuring that file-based identification takes precedence over pattern-based renaming.

These changes integrate into the existing CI/CD pipeline that manages test execution and reporting for the Unity MCP project. The workflow handles test discovery, execution, and canonicalization of test names to maintain consistency across different test suites. By fixing the naming collision issue, this ensures that test results are accurately categorized and reported, which is crucial for maintaining test integrity in the automated testing environment.

Important Files Changed

Changed Files
Filename Score Overview
.github/workflows/claude-nl-suite.yml 4/5 Fixed test canonicalization logic to prevent naming collisions and tightened regex patterns for more accurate test identification

Confidence score: 4/5

  • This PR is safe to merge with minimal risk as it addresses a specific naming issue without affecting core functionality
  • Score reflects targeted fixes to well-defined regex and canonicalization logic that improve test accuracy
  • Pay close attention to the workflow file to ensure the regex changes don't inadvertently exclude valid test cases

Sequence Diagram

sequenceDiagram
    participant User
    participant GitHub Actions
    participant Docker
    participant Unity
    participant MCP Server
    participant Claude AI
    participant Test Reports

    User->>GitHub Actions: "Trigger workflow_dispatch"
    GitHub Actions->>GitHub Actions: "Check secrets (Unity license, Anthropic API key)"
    GitHub Actions->>Docker: "Pull Unity image"
    GitHub Actions->>GitHub Actions: "Setup Python/uv environment"
    GitHub Actions->>GitHub Actions: "Install MCP server dependencies"
    
    alt Unity License Available
        GitHub Actions->>Docker: "Activate Unity license (EBL/ULF)"
        Docker->>Unity: "Start Unity with license"
        Unity->>Docker: "License activation complete"
    end
    
    GitHub Actions->>Docker: "Start Unity container (persistent bridge)"
    Docker->>Unity: "Launch Unity with MCP bridge"
    Unity->>Unity: "Start AutoConnect for MCP"
    GitHub Actions->>GitHub Actions: "Wait for Unity bridge ready signal"
    Unity->>GitHub Actions: "Bridge listening on port"
    
    GitHub Actions->>GitHub Actions: "Write MCP config (.claude/mcp.json)"
    GitHub Actions->>GitHub Actions: "Create report skeletons and helper scripts"
    GitHub Actions->>GitHub Actions: "Snapshot baseline files"
    
    GitHub Actions->>Claude AI: "Run Claude NL suite with MCP tools"
    Claude AI->>MCP Server: "Connect via stdio transport"
    MCP Server->>Unity: "Execute Unity commands"
    Unity->>MCP Server: "Return Unity state/results"
    MCP Server->>Claude AI: "Provide Unity data"
    Claude AI->>Claude AI: "Generate test edits and validations"
    Claude AI->>GitHub Actions: "Output test results fragments"
    
    GitHub Actions->>GitHub Actions: "Canonicalize testcase names"
    GitHub Actions->>GitHub Actions: "Backfill missing NL/T tests"
    GitHub Actions->>GitHub Actions: "Merge fragments into JUnit XML"
    GitHub Actions->>GitHub Actions: "Build markdown summary"
    GitHub Actions->>Test Reports: "Publish JUnit report"
    GitHub Actions->>Test Reports: "Upload artifacts"
    
    GitHub Actions->>Docker: "Stop Unity container"
    Docker->>Unity: "Shutdown Unity instance"
Loading

1 file reviewed, no comments

Edit Code Review Bot Settings | Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants