Add GitHub-issue-driven test framework (+ src/ library carve-out)#67
Open
gaurav wants to merge 132 commits into
Open
Add GitHub-issue-driven test framework (+ src/ library carve-out)#67gaurav wants to merge 132 commits into
gaurav wants to merge 132 commits into
Conversation
Each assertion type (Resolves, DoesNotResolve, ResolvesWith, ResolvesWithType, SearchByName, Needed) is now a self-contained class in src/babel_validation/assertions/, grouped by which service it targets (nodenorm.py, nameres.py, common.py). A central ASSERTION_HANDLERS registry in __init__.py maps lowercase assertion names to handler instances, and NodeNormAssertion / NameResAssertion marker base classes allow isinstance() checks for applicability. GitHubIssueTest.test_with_nodenorm() and test_with_nameres() are now 3-line dispatchers. A README.md documents all supported assertion types with examples in both wiki and YAML syntax. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…pendently. Bumps pytest to >=9.0 (which includes built-in subtests support). Each TestResult is now evaluated in its own subtest block, so a failure no longer short-circuits the rest. Adds post-loop state-consistency subtests: a closed issue with failing tests fails with a "consider reopening" message, and an open issue where all tests pass emits an xfail "consider closing" hint. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Previously, get_github_issue_id() and GitHubIssueTest.__str__() both resolved the org/repo name via github_issue.repository.organization.name, which triggers lazy PyGitHub API calls. Parse org/repo from html_url instead (always present in the issue JSON, no extra round-trip needed). Also resolves the TODO comment in get_test_issues_from_issue(). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The GitHub issue tests always read Repositories from [DEFAULT]; per-environment overrides are not implemented. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment on lines
+100
to
+118
| def pytest_generate_tests(metafunc): | ||
| if "github_issue_id" not in metafunc.fixturenames: | ||
| return | ||
| issue_id_filter = metafunc.config.getoption("issue", default=[]) | ||
| if issue_id_filter: | ||
| try: | ||
| issues = _get_github_issues_test_cases().get_issues_by_ids(issue_id_filter) | ||
| except GithubException as e: | ||
| if e.status == 401: | ||
| _record_auth_error(e) | ||
| metafunc.parametrize("github_issue_id", [_AUTH_ERROR_ID], ids=[_AUTH_ERROR_ID]) | ||
| return | ||
| raise | ||
| ids = [_issue_id(i) for i in issues] | ||
| for issue, id_ in zip(issues, ids): | ||
| _fetched_issues_cache[id_] = issue | ||
| else: | ||
| ids = _get_all_test_issue_ids() | ||
| metafunc.parametrize("github_issue_id", ids, ids=ids) |
Comment on lines
+178
to
+186
| # Parse string as YAML. | ||
| yaml_dict = yaml.safe_load(match.removeprefix("```yaml").removesuffix("```")) | ||
|
|
||
| babel_tests = yaml_dict.get('babel_tests') if isinstance(yaml_dict, dict) else None | ||
| if babel_tests is None: | ||
| raise ValueError( | ||
| f"YAML block in issue {github_issue_id} matched the detection pattern " | ||
| f"but contains no 'babel_tests' top-level key: {match!r}" | ||
| ) |
Comment on lines
+12
to
+13
| def test_with_nodenorm(self, param_sets, nodenorm, label=""): | ||
| yield self.failed("Test needed for issue") |
pytest_generate_tests for test_github_issue fetches issues from GitHub at collection time. pytest applies -m marker deselection only *after* generation, so running 'pytest -m unit' (as CI does) would still hit the GitHub search API across every configured repo before deselecting the test. Evaluate the marker expression ourselves against the test's own markers and, when it would deselect this test, parametrize an empty set instead of fetching. This keeps unit-only runs offline. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Both assertions document 'Two or more CURIEs per param_set' but never checked. A single-CURIE ResolvesWith passed vacuously (canonical id == itself) while a single-CURIE DoesNotResolveWith always failed (one canonical id) — both silent authoring traps. Fail loudly with a clear message instead, matching HasLabel and SearchByName. Adds unit coverage for the too-few-params case. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
normalize_curie indexed the result dict directly while every other code path uses .get(). NodeNorm normally echoes every requested CURIE (null when unresolvable), but switch to .get() so an omitted key returns None rather than raising. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The NameRes 'found in top N' threshold was hardcoded to the handler default of 5, ignoring NameResXFailIfInTop in targets.ini. Read it from target_info so the threshold is configurable per environment. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
gaurav
changed the title
Move the marker-expression check out of the github_issues conftest into tests/_pytest_helpers.py so the Google Sheet test modules can reuse it to defer their own network-backed parametrization. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The gsheet and blocklist test modules instantiated GoogleSheetTestCases / load_blocklist_from_gsheet at import time and parametrized via a module-level @pytest.mark.parametrize, so every collection — including 'pytest -m unit' — downloaded the sheets even though those tests aren't unit tests. Move parametrization into pytest_generate_tests with a lazy, cached fetch, and skip the fetch entirely when a -m filter would deselect the test. Row IDs (row=N) and xfail marks are unchanged, so -k 'row=42' still works. With this, 'pytest -m unit' runs fully offline: ~0.3s and no network, versus ~2.5s and a GitHub + Google Sheet round trip before. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A `babel_tests:` value that is a list/scalar (not a mapping) previously crashed with an opaque AttributeError on `.items()`, and a non-string assertion key (e.g. `123:`) blew up later when `.lower()` was called. Both now raise a clear ValueError up front, with unit coverage. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Resolved-node message formatting indexed result['type'][0] in five
places, which would raise IndexError if NodeNorm ever returned a node
with an empty type list. Route all sites through a NodeNormTest.first_type()
helper and use .get('label', '') for the same robustness.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Importing the tool registered the _FlowList representer on the global yaml.SafeDumper, changing YAML output process-wide. Move it onto a local _BabelTestDumper subclass; emitter output is unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Jun 24, 2026
Collaborator
Author
|
Recommended merge order: #67 → #95 → #96. These are stacked PRs, each based on the previous branch (not
Merging in this order keeps each diff small and reviewable. Once #67 merges, GitHub will retarget #95 onto |
This was referenced Jun 26, 2026
Collaborator
Author
|
To make this reviewable, I've split it into a stack of smaller PRs (review/merge top-down):
Each was reconstructed from this branch's final tree (path-based for 1–3; #104 cherry-picks the CSV commits with authorship intact) and verified offline. Plan is to merge these into |
Record the non-obvious things: `-m unit` is the offline CI suite and network-backed modules defer fetches via deselected_by_markexpr; src/ is importable thanks to the hatch wheel packaging; black isn't strictly enforced; and commits sign through 1Password's SSH agent. Also fix the `-k "row=42"` example, which pytest's expression parser rejects. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
gaurav
added a commit
that referenced
this pull request
Jun 26, 2026
The repo's `.gitignore` predates the Python rewrite — it only covered Scala/Giter8/IntelliJ artifacts, so `__pycache__/` and `*.pyc` showed up as untracked noise throughout the tree and were easy to `git add` by accident. Appends the standard [GitHub `Python.gitignore`](https://github.com/github/gitignore/blob/main/Python.gitignore) template (bytecode, `build/`/`dist/`, `.pytest_cache/`, `.venv`/`.env`, mypy/coverage caches, …) below the existing entries, plus a `.DS_Store` line for macOS. Existing entries are kept at the top, so this is purely additive. Independent of the #67 split stack (#101–#104) and based on `main`, so it can merge immediately. No files are currently tracked that the new patterns would retroactively ignore — `git ls-tree` shows no `.pyc`/`__pycache__` in the tree — so nothing needs `git rm --cached`. ### Verify ``` git check-ignore -v src/__pycache__/x.pyc .venv .env # all matched ``` 🤖 Generated with [Claude Code](https://claude.com/claude-code)
gaurav
added a commit
that referenced
this pull request
Jun 26, 2026
Pure-ish refactor, no service-behavior change. Moves the code under
`tests/common/` into an importable `src/babel_validation` package and
updates the Google Sheet test modules to match.
### What changes
- Split the monolithic `tests/common/google_sheet_test_cases.py` into:
- `core/testrow.py` — `TestRow` / `TestStatus` / `TestResult`
- `services/{nodenorm,nameres}.py` — `CachedNodeNorm` / `CachedNameRes`
- `sources/google_sheets/{google_sheet_test_cases,blocklist}.py`
- Gsheet test modules now import from the new locations and parametrize
**lazily** in `pytest_generate_tests` via
`tests/_pytest_helpers.deselected_by_markexpr`, so marker-deselected
runs never hit the network.
- Migrate `tests/pytest.ini` into `[tool.pytest.ini_options]`; add a
hatchling build that packages `src/`; add `filelock` (gsheet disk
cache).
- Move `test_env.py` → `tests/test_environment/`.
Two incidental one-liners ride along in the final test files (NameRes
`biolink_type` passed as a list; description identifiers as lists not
sets) — kept as-is so #67 rebases cleanly.
### Verify
- `uv run pytest -m unit --collect-only -q` — full suite
imports/collects offline (no network).
- `uv run pytest tests/test_environment/test_env.py` — live Google Sheet
download+parse through the moved code.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Introduces a GitHub-issue-driven test framework for babel-validation, alongside a library carve-out and supporting tooling.
Major pieces
tests/common/to an installablesrc/babel_validation/package (core,services,sources,assertions,tools). The project now builds as a hatchling wheel; existingfrom src.babel_validation.X import Yimports keep working.AssertionHandlertypes (Resolves,DoesNotResolve,ResolvesWith,DoesNotResolveWith,HasLabel,ResolvesWithType,SearchByName,Needed) with an auto-generatedassertions/README.mdand a drift test that fails CI if it goes stale.{{BabelTest|...}}wiki lines and```yaml babel_tests:blocks from issue bodies. Open issues are expected to fail (strict xfail, so XPASS flags a closeable issue); closed issues are expected to pass.csv-to-babeltests) for bulk-authoring assertion blocks, reusing the same handlers and YAML schema as the issue parser.pytest -m uniton PRs.Review fixes (this PR)
Addressing the code review on the original revision:
pytest -m unitnow runs fully offline. Both the GitHub issue tests and the Google Sheet / blocklist tests previously fetched from the network at collection time, because their parametrization is built before pytest applies-mdeselection. The marker expression is now evaluated up front (sharedtests/_pytest_helpers.deselected_by_markexpr), and the GitHub fetch and Google Sheet downloads are skipped when the test would be deselected. Net effect:pytest -m unitdrops from ~2.5s + network to ~0.3s offline. Row IDs (row=N) and xfail marks are unchanged, so-k 'row=42'still works.ResolvesWith/DoesNotResolveWithnow enforce their documented 2+ CURIE arity. A single-CURIE param_set previously passed (or failed) vacuously; it now fails loudly with a clear message, matchingHasLabel/SearchByName. Adds unit coverage.SearchByNametop-N is read fromtargets.ini(NameResXFailIfInTop) instead of being hardcoded to 5, so it's configurable per environment.CachedNodeNorm.normalize_curieuses.get()to avoid aKeyErrorif NodeNorm ever omits a requested CURIE, consistent with the rest of the code.Unrelated changes riding along in this PR
These are intentional and correct, but not part of the GitHub-issue test framework itself:
pytestbumped 8.4.2 → 9.0.2 (plus new deps:pygithub,pytest-xdist[psutil],pytest-subtests,click,pyyaml,tqdm,filelock,python-dotenv).tests/pytest.iniremoved; pytest config (testpaths,timeout,markers) moves intopyproject.toml[tool.pytest.ini_options].tests/targets.ini[ci]/[ci-es]restructure: both CI targets now point NodeNorm atnodenorm-es.ci.transltr.io(the oldbiothings.ci.transltr.io/nodenorm/URL is gone);[ci]pairs the ES NodeNorm with the non-ES NameRes,[ci-es]with the ES NameRes. ARepositorieslist (scanned for embedded assertions) is added to[DEFAULT].tests/nameres/test_nameres_from_gsheet.py: thebiolink_typerequest parameter is now sent as a list ([biolink_class]) instead of a bare string, matching the current NameRes API.Follow-up: library carve-out (stacked PRs)
This PR is being kept as the base for two smaller, stacked follow-up PRs that make the GitHub-issue test framework usable as an installable library (so the Babel pipeline and Babel Explorer can consume it). They branch off this one rather than
main, since the library code only exists here:add-github-issue-tests): rename the import packagesrc.babel_validation→babel_validation, shipsrc/babel_validationas the top-level wheel package, and split runtime vs dev dependencies (PyGitHub stays core). No behavior change.library-packaging, stacked on Library packaging: import as babel_validation + split runtime/dev deps #95): turnGitHubIssueTestinto a plainAssertionobject, and add a pytest-independentrun_issue_tests()reporting API (IssueReport/ResultRecord) with the pytest suite rewired as a thin adapter.Recommended review/merge order: this PR (#67) → #95 → #96. Out-of-scope library-usability improvements surfaced along the way are tracked in #97, #98, and #99.