Skip to content

test(windows): run reviewer provider process tests instead of skipping them #3687

Description

@Denver2828

Pre-flight Checklist

  • I have searched existing issues and this is not a duplicate
  • I understand that PRs will be rejected if the linked issue does not have status:approved

📝 Bug Description

Five reviewer-provider process tests are skipped on Windows with the reason the helper process uses POSIX argument handling.

The current helpers do not invoke a POSIX shell. They execute the Go test binary directly through exec.CommandContext.

I verified on current main that removing only the five Windows skip guards and the three now-unused runtime imports makes all five affected tests—and the complete internal/reviewerprovider package—pass on Windows.

The proposed correction is test-only: 0 additions and 18 deletions across three files.

🔄 Steps to Reproduce

  1. Check out current main at 26d7ceed225e16d45378a78333a5e4068d8287e7.

  2. On Windows, run:

    go test ./internal/reviewerprovider -count=1 -v
  3. Observe that these five tests are skipped:

    • TestClaudeAdapterUsesStdinAndReturnsUntouchedRawOutput
    • TestCodexAdapterUsesStdinAndReturnsUntouchedRawOutput
    • TestCodexAdapterConfiguresApprovedLoopbackProvider
    • TestPiAdapterUsesStdinLockedDownArgumentsAndReturnsUntouchedRawOutput
    • TestPiAdapterFailsClosedOnProcessFailureAndEmptyOutput
  4. Remove only their Windows skip guards and the three now-unused runtime imports.

  5. Run the same package test again.

All five affected tests and the complete package pass on Windows.

✅ Expected Behavior

The five reviewer-provider process tests should execute and pass on Windows, covering stdin transport, locked-down arguments, untouched raw output, approved loopback configuration, and fail-closed Pi behavior.

❌ Actual Behavior

All five tests are skipped on Windows because of a stale assumption that their helper processes require POSIX argument handling, leaving these reviewer-provider transport contracts untested on Windows.

Gentle AI Version

Current main at commit 26d7ceed225e16d45378a78333a5e4068d8287e7.

Operating System

Windows

AI Agent / Client

Other

📋 Affected Area

CLI (commands, flags)

💡 Logs / Error Output

SKIP: TestClaudeAdapterUsesStdinAndReturnsUntouchedRawOutput
SKIP: TestCodexAdapterUsesStdinAndReturnsUntouchedRawOutput
SKIP: TestCodexAdapterConfiguresApprovedLoopbackProvider
SKIP: TestPiAdapterUsesStdinLockedDownArgumentsAndReturnsUntouchedRawOutput
SKIP: TestPiAdapterFailsClosedOnProcessFailureAndEmptyOutput

Reason: the helper process uses POSIX argument handling

After removing only the skip guards:

PASS
ok github.com/gentleman-programming/gentle-ai/v2/internal/reviewerprovider 0.641s

Additional Context

Affected files:

  • internal/reviewerprovider/claude_adapter_test.go
  • internal/reviewerprovider/codex_adapter_test.go
  • internal/reviewerprovider/pi_adapter_test.go

The verified correction removes 18 lines and adds none. It changes no production code.

Searches using the skip reason and all five exact test names found no equivalent open or closed issue or pull request.

Related precedent: PR #3227 replaced other POSIX-only provider test fakes with cross-platform helpers, but it did not cover these reviewer-provider package tests.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workinggood first issueGood for newcomerspriority:lowLow prioritystatus:approvedApproved for implementation — PRs can now be openedtype:bugBug fixup-for-grabsScoped, approved and ready for a community contributor to pick up

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions