Require passing sandbox integration tests#112
Conversation
teesofttech
added
area: ci/cd
📝 WalkthroughWalkthroughAdds a Python integration-test gate, wires provider preflight and TRX validation into GitHub Actions, documents sandbox testing behavior, and adds unit tests covering workflow wiring, provider selection, result parsing, and gating rules. ChangesSandbox integration test gating
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/integration-tests.yml (1)
71-95: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove
if: always()to prevent cascading step failures.Because the
Run integration testsstep usescontinue-on-error: true, its failure will not prevent subsequent steps from running (the step conclusion is treated as success).However, using
if: always()on these post-test steps forces them to run even if an earlier critical step likepreflightorBuild solutionfails. In those scenarios, no.trxfiles are generated, causing these steps to fail noisily and pollute the workflow logs with "TRX file not found" errors. Removingif: always()allows them to run naturally after test completion (whether tests passed or failed) but correctly skips them if the build or preflight crashed.♻️ Proposed refactor
- name: Validate and summarize integration results - if: always() run: | python3 scripts/integration_test_gate.py summarize \ --provider "${{ steps.preflight.outputs.provider }}" \ --trx ./TestResults/integration-test-results.trx \ --github-summary "$GITHUB_STEP_SUMMARY" - name: Upload integration test results uses: actions/upload-artifact@v4 - if: always() with: name: integration-test-results path: ./TestResults/*.trx if-no-files-found: error - name: Publish test results uses: dorny/test-reporter@v1 - if: always() with: name: Integration Test Results path: ./TestResults/*.trx reporter: dotnet-trx fail-on-error: false # the tested result gate above owns job failure🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/integration-tests.yml around lines 71 - 95, Remove the if: always() conditions from the “Validate and summarize integration results,” “Upload integration test results,” and “Publish test results” steps in the workflow. Preserve their normal execution after the integration test step, while allowing them to be skipped when earlier preflight or build steps fail and no TRX files exist.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/workflows/integration-tests.yml:
- Around line 71-95: Remove the if: always() conditions from the “Validate and
summarize integration results,” “Upload integration test results,” and “Publish
test results” steps in the workflow. Preserve their normal execution after the
integration test step, while allowing them to be skipped when earlier preflight
or build steps fail and no TRX files exist.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ad9f1642-ffd6-4acb-8c43-6b00ac2fa4aa
📒 Files selected for processing (5)
.github/workflows/ci-tests.yml.github/workflows/integration-tests.ymldocs/integration-testing.mdscripts/integration_test_gate.pyscripts/tests/test_integration_test_gate.py
There was a problem hiding this comment.
Pull request overview
This PR hardens the scheduled sandbox integration workflow so it cannot report a false “success” when sandbox credentials are missing and all integration tests are skipped, by adding a Python-based preflight + TRX results gate and wiring it into GitHub Actions.
Changes:
- Added a Python “integration test gate” to (1) validate provider selection and required secrets, and (2) parse TRX results, publish a provider summary, and fail when zero tests pass.
- Updated the integration workflow to require explicit gateway selection, apply a provider-specific
dotnet testfilter, upload TRX artifacts reliably, and let the gate decide pass/fail. - Added unit tests for the gate and documented integration-environment setup; reduced scheduled frequency from daily to weekly until credentials are stable.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/tests/test_integration_test_gate.py | Adds unit tests covering workflow wiring, preflight validation, TRX parsing, and failure conditions. |
| scripts/integration_test_gate.py | Implements provider planning, TRX parsing, job summary generation, and pass/fail gating logic. |
| docs/integration-testing.md | Documents provider suites, required environment secrets, and how to run the gate locally. |
| .github/workflows/integration-tests.yml | Wires preflight + result gate into the scheduled/manual integration workflow and tightens dispatch input/options. |
| .github/workflows/ci-tests.yml | Runs the Python gate unit tests in PR CI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| passed = int(counters.get("passed", "0")) | ||
| failed = int(counters.get("failed", "0")) |
| Manual runs expose only `all` and providers with implemented sandbox tests. | ||
| Selecting one provider applies an exact fully-qualified-name test filter. The | ||
| scheduled `all` run executes configured provider suites and fails during | ||
| preflight when none are configured. |
Summary
Root cause
The integration tests use runtime skips when credentials are absent.
dotnet testtherefore returned success for two skipped Peach Payments tests, and the reporting action was explicitly configured not to fail the workflow. The manual gateway input was also never applied to the test filter.TDD
Tests were written and observed failing before implementation. The resulting 11-test suite covers unsupported selections, missing secrets,
allselection, exact filtering, workflow wiring, TRX counts, xUnit'sNotExecutedskip representation, failed tests, and zero-pass runs.Validation
git diff --check: cleanRepository configuration still required
The GitHub
integrationenvironment currently contains no secrets. AddPEACH_ENTITY_IDandPEACH_ACCESS_TOKEN, then manually run thepeachpaymentsworkflow and confirm a real create/verify flow before closing #58. Until then, scheduled runs will correctly fail rather than report a false success.Refs #58
Summary by CodeRabbit
New Features
Documentation
Tests