docs(test): add explicit parametrize IDs and README testing instructions#37
docs(test): add explicit parametrize IDs and README testing instructions#37
Conversation
Rewrite test_integration.py with a proper fixture-based structure: - session-scoped github_env and date_windows fixtures for shared setup - module-scoped fetched_workspace fixture that runs fetch once and reuses the database across all classify tests - date windows computed dynamically relative to today rather than hardcoded values - four parametrised fetch variants (default, with-dates, reset-db, config) - dedicated classify tests: table output, stricter threshold, JSON output, and --exclude-primary-merged Expand the README Development section with instructions for running unit tests only, integration tests only, and each individual integration test by its pytest node ID. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 55150e70af
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "--start", | ||
| date_windows.classify_start, | ||
| "--end", | ||
| date_windows.classify_end, |
There was a problem hiding this comment.
These classify invocations rely on the CLI default --min-samples 30 while the baseline window is fixed to roughly today-182d through today-30d; when expressjs/express has fewer than 30 merged PRs in that window, classify returns an insufficient-data error and _run_cli fails the test. This makes the new integration suite time-dependent and can break without code changes, so the tests should either pass a smaller explicit --min-samples or widen the baseline window.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
yes I think that is a valid point. Can we modify the number of samples to 10 for integration tests.
ghinks
changed the title
1 participant
What
idsto the@pytest.mark.parametrizedecorator intests/test_integration.pyso each fetch variant has a stable, human-readable node ID (fetch-default,fetch-with-dates,fetch-reset-db,fetch-config) instead of auto-generated ones based on lambda string representations.Why
Without explicit IDs, pytest generates test IDs like
test_fetch_examples_integration[<lambda>-Successfully saved0]. These are fragile — adding or reordering a parametrize variant silently shifts the numeric suffixes and breaks any command that targets a specific test by ID. Named IDs are stable regardless of ordering.Changes
tests/test_integration.pyids=["fetch-default", "fetch-with-dates", "fetch-reset-db", "fetch-config"]to@pytest.mark.parametrizeREADME.mduv run pytest -m "not integration"for unit tests onlyuv run pytest -m integrationfor integration tests onlyTesting
Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com