Split unit and integration test jobs#789
Conversation
Separate the single 'tests' job into two independent jobs: - unit-tests (required gate): runs tests excluding integration/, browser/, chapter/, examples/, and mcp/ directories. These are fast, deterministic tests that don't depend on external services. - integration-tests (non-blocking): runs tests/integration, tests/browser, and tests/chapter with continue-on-error: true and --reruns 3 for flaky test resilience. Failures here no longer block merges. Also adds 'unit' and 'integration' pytest markers to pyproject.toml for future use in test classification. This addresses the ~85% failure rate on main caused by integration tests hitting live external services (staging API, e-commerce sites) and browser OOM crashes.
| - name: Checkout code (PR or push) | ||
| if: github.event_name != 'workflow_dispatch' | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| ref: ${{ github.event.pull_request.head.sha }} | ||
|
|
||
| - id: 'auth' |
|
| Filename | Overview |
|---|---|
| .github/workflows/test-cicd.yml | Splits tests into unit-tests (required gate) and integration-tests (non-blocking, continue-on-error); integration test results are not uploaded as artifacts. |
| pyproject.toml | Adds unit and integration pytest marker definitions to suppress PytestUnknownMarkWarning; no other changes. |
Prompt To Fix All With AI
This is a comment left during a code review.
Path: .github/workflows/test-cicd.yml
Line: 216-225
Comment:
**No artifact upload for integration test results**
`pytest-integration.xml` and `pytest-integration.txt` are generated but never uploaded. Since integration tests are expected to fail regularly and `continue-on-error: true` hides the failure from the status check UI, there's no easy way to inspect results without digging through raw job logs. Consider adding an artifact upload step so failures remain accessible.
```yaml
- name: Upload integration test results
if: always()
uses: actions/upload-artifact@v4
with:
name: integration-test-results
path: |
pytest-integration.xml
pytest-integration.txt
retention-days: 7
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (2): Last reviewed commit: "ci: add tests/mcp and tests/examples to ..." | Re-trigger Greptile
| uv run pytest -n logical \ | ||
| tests/integration tests/browser tests/chapter \ | ||
| --ignore=tests/integration/test_webvoyager_resolution.py \ | ||
| --ignore=tests/integration/test_e2e.py \ | ||
| --reruns 3 --reruns-delay 5 \ | ||
| --durations=10 --junitxml=pytest-integration.xml \ | ||
| | tee pytest-integration.txt |
There was a problem hiding this comment.
tests/mcp and tests/examples silently dropped from CI
unit-tests excludes tests/mcp and tests/examples, but integration-tests only runs tests/integration tests/browser tests/chapter. Neither job will execute tests/mcp or tests/examples, so any regressions in those areas will go completely undetected. The original tests job previously ran tests/mcp (it was not in the ignore list), so this is a regression in coverage.
Add the missing directories to the integration-tests run command (or add them back to unit-tests if they have no external dependencies):
| uv run pytest -n logical \ | |
| tests/integration tests/browser tests/chapter \ | |
| --ignore=tests/integration/test_webvoyager_resolution.py \ | |
| --ignore=tests/integration/test_e2e.py \ | |
| --reruns 3 --reruns-delay 5 \ | |
| --durations=10 --junitxml=pytest-integration.xml \ | |
| | tee pytest-integration.txt | |
| uv run pytest -n logical \ | |
| tests/integration tests/browser tests/chapter tests/mcp tests/examples \ | |
| --ignore=tests/integration/test_webvoyager_resolution.py \ | |
| --ignore=tests/integration/test_e2e.py \ | |
| --reruns 3 --reruns-delay 5 \ | |
| --durations=10 --junitxml=pytest-integration.xml \ | |
| | tee pytest-integration.txt |
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/test-cicd.yml
Line: 219-225
Comment:
**`tests/mcp` and `tests/examples` silently dropped from CI**
`unit-tests` excludes `tests/mcp` and `tests/examples`, but `integration-tests` only runs `tests/integration tests/browser tests/chapter`. Neither job will execute `tests/mcp` or `tests/examples`, so any regressions in those areas will go completely undetected. The original `tests` job previously ran `tests/mcp` (it was not in the ignore list), so this is a regression in coverage.
Add the missing directories to the integration-tests run command (or add them back to unit-tests if they have no external dependencies):
```suggestion
uv run pytest -n logical \
tests/integration tests/browser tests/chapter tests/mcp tests/examples \
--ignore=tests/integration/test_webvoyager_resolution.py \
--ignore=tests/integration/test_e2e.py \
--reruns 3 --reruns-delay 5 \
--durations=10 --junitxml=pytest-integration.xml \
| tee pytest-integration.txt
```
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Good catch — both tests/mcp and tests/examples require external dependencies (MCP server subprocess, browser automation, live API calls, example script execution), so they belong in the integration-tests job. Fixed in 7a6ac8b by adding them to the integration-tests pytest run command.
Address review feedback: these directories were excluded from unit-tests but not included in integration-tests, silently dropping them from CI. Both contain tests requiring external dependencies (MCP server, browser automation, live API calls) so they belong in the integration-tests job.
Summary
testsjob intounit-tests(required gate) andintegration-tests(non-blocking) to stop integration test flakiness from blocking merges on main. Thetestsjob was failing ~85% of pushes due to integration tests hitting live external services (staging API regressions, e-commerce page changes, browser OOM crashes).unit-testsruns all tests excepttests/integration/,tests/browser/,tests/chapter/,tests/examples/, andtests/mcp/— these are fast, deterministic tests with no external dependencies. Pre-commit checks and coverage reporting remain here.integration-testsrunstests/integration,tests/browser, andtests/chapterwithcontinue-on-error: trueand--reruns 3for flaky test resilience. Failures no longer block merges.Important
Manual Steps Required Before Merging
testsjob is currently a required status check, replace it withunit-tests(the new required gate job name)Context
Addresses insight: tests job on main failing ~85% of pushes. Three failure modes were identified:
test_personas.pytests)None of these affect unit tests — splitting the jobs isolates the required CI gate from external service instability.
Changes
.github/workflows/test-cicd.ymltests→unit-tests+integration-testsjobspyproject.tomlunitandintegrationpytest markersNote
Created by Mendral. Tag @mendral-app with feedback or questions.