Add GitHub-issue-driven test framework, assertion handlers, and csv-to-babeltests CLI - #67
Open
gaurav wants to merge 136 commits into
Open
Add GitHub-issue-driven test framework, assertion handlers, and csv-to-babeltests CLI#67gaurav wants to merge 136 commits into
csv-to-babeltests CLI#67gaurav wants to merge 136 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>
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)
PyYAML resolves bare scalars by YAML 1.1 rules, but _to_list only validated the container type — so element types reached the handlers unchanged. A param_set like [CHEBI:16480, NO] parsed to [..., False] and killed HasLabelHandler with AttributeError, and a numeric identifier crashed _CURIE_RE.match with TypeError. "NO" (nitric oxide) and numeric labels are realistic Babel test content. _to_param() now coerces each scalar for all eight handlers at once. Numbers stringify cleanly; bare NO/YES/ON/OFF and nulls are rejected with a quote-it hint, since by the time PyYAML hands them over the original spelling is lost and str(False) would silently assert on the wrong label. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
NodeNorm may omit label or type from a node, and may omit a requested CURIE from its response entirely. Four places assumed otherwise: - SearchByNameHandler indexed result['id']['label'], raising KeyError on an unlabelled node — HasLabelHandler already guards this case. - ResolvesWithTypeHandler indexed node['type'], bypassing the first_type() guard added in add0848 for exactly this. - normalize_curies returned the raw response, so an omitted CURIE was absent from the mapping. DoesNotResolveWithHandler iterates that mapping, so omitted CURIEs vanished and the assertion passed vacuously. It is now seeded with every requested CURIE, making "one entry per input" an invariant rather than an accident of the pre-warming call order. - _compare_resolutions picked its canonical id from dict order, which depends on set iteration and cache state, so "but expected X" varied across runs for the same failing issue. It now walks params order. Also filters the csv-to-babeltests pre-warm through _CURIE_RE, matching what test_with_nodenorm has done since e322040 — an empty --equivalent-curie-column cell was being POSTed to NodeNorm. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
pytest.mark.xfail(strict=True) with no raises= treats any exception as the expected failure. The marker is added before the assertion loop and most tracked issues are open, so a NodeNorm outage, an HTTP 500, or a bug in our own issue parsing produced XFAIL and a fully green run across every open issue. raises=AssertionError restores the signal; the imperative pytest.xfail() calls at the end of the test are unaffected. Also widens _silent_unlink to catch OSError. The gsheet and issue caches use fixed names in the shared temp dir, so on a shared dev box or self-hosted runner a stale file owned by another user raised PermissionError out of pytest_configure and no test ran at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
src/babel_validation library, and csv-to-babeltests CLI
main's #101 landed a parallel library reorg, so the shared files conflicted. Resolved by taking main's structure and re-applying this branch's additions: - services/nameres.py, services/nodenorm.py: main's docstrings, Protocol interfaces, and clear_curie/delete_query → invalidate_curie/invalidate_query renames. Re-applied the normalize_curies result seeding from d1d1590 and documented the "one entry per requested CURIE" guarantee in the docstring. - sources/google_sheets/google_sheet_test_cases.py: main's cache_ttl_seconds constructor parameter and relative import. No callers referenced the removed CACHE_TTL_SECONDS class attribute. - tests/conftest.py: main's unlink_if_exists rename, keeping the OSError widening from bcbca58 plus the issue-cache cleanup and the selected_github_issues fixture. - CLAUDE.md, pyproject.toml: this branch is a superset of main's version. - uv.lock: regenerated with `uv lock`. pytest -m unit: 56 passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
src/babel_validation library, and csv-to-babeltests CLIcsv-to-babeltests CLI
gaurav
added a commit
that referenced
this pull request
Aug 19, 2026
Adds `src/babel_validation/assertions`: the engine that turns a named BabelTest assertion plus its parameters into a check evaluated against NodeNorm/NameRes. Nothing consumes it yet — the GitHub issue parser that produces the parameters arrives later in the stack — so this PR is the engine, its generated documentation, and its offline tests. **Stack 2 of 4** splitting #67. Base: `main` (#101 is merged). ### The assertion types `AssertionHandler` is the base class; `NodeNormTest` and `NameResTest` specialize it per service. Each concrete handler declares its parameters and yields `TestResult`s. Registered: `Resolves`, `DoesNotResolve`, `ResolvesWith`, `DoesNotResolveWith`, `HasLabel`, `ResolvesWithType`, `SearchByName`, and `Needed` (a placeholder that always fails, marking an issue as still needing a real test). Both wiki (`{{BabelTest|Resolves|CHEBI:15365}}`) and YAML syntax are supported, and an assertion can carry several independent **params lists**, each evaluated separately so one bad one doesn't sink the rest. ### Shared parameter handling `AssertionHandler.prepare_params_lists()` strips whitespace from every param, rejects params lists whose CURIEs are malformed, and warms the NodeNorm cache for the survivors in a single batched request. Both `test_with_nodenorm()` and `test_with_nameres()` route through it, so the NameRes path gets the same validation and the same one-request warming rather than a NodeNorm round-trip per params list. Two escape hatches keep that uniform treatment from being wrong for particular assertions: - `curie_params()` narrows which params are CURIEs — `HasLabel` to the first, `ResolvesWithType` to everything after the Biolink type, `SearchByName` to the expected CURIE only (its first param is a free-text query). - `VALIDATE_CURIES = False` opts an assertion out of format validation entirely. `DoesNotResolve` sets it: an identifier that isn't even a well-formed CURIE trivially doesn't resolve, which is exactly what that assertion exists to state, so rejecting it up front would leave the assertion unable to express its own purpose. ### A NodeNorm bulk-normalization fix `CachedNodeNorm.normalize_curies()` built its return value from `response.json()`, so a CURIE that NodeNorm silently omitted from its response was absent from the returned dict rather than present with a `None` value. A caller iterating the results would never see it and would report success for a CURIE it never tested. The warm-cache path happened to re-add the missing key, so the hole only opened on a cold lookup. It now builds the result from the requested CURIEs: exactly one entry per request, in request order. The ordering guarantee matters independently — "first CURIE that resolved" logic previously depended on the server's JSON ordering when cold and on set iteration order (per-process string hash randomization) when warm, so *which* CURIE got blamed in a failure message could vary between runs of the same test. ### Types and naming Parameters are named rather than left as nested `list[str]`: - `ParamsList` — one assertion invocation's parameters. Position is significant (`ResolvesWithType` takes its Biolink type first, `HasLabel` is `[curie, label]`), which is why this is a list. - `PreparedParamsList` — a frozen `(params, failure)` record. `prepare_params_lists()` returns a list of these rather than a `(stripped, failures_by_index)` tuple, so callers don't re-zip two structures by hand. Service parameters are annotated with the `NodeNormService` / `NameResService` Protocols that `services/` already defines for the purpose, rather than the concrete `CachedNodeNorm` / `CachedNameRes`, so a future drop-in replacement needs no changes here. ### Guardrails - **Registration.** `_register()` replaces the `{h.NAME: h for h in [...]}` comprehension and raises on a `NAME` that isn't lowercase or one that's already taken. Lowercase is load-bearing — the README promises users that assertion names are matched case-insensitively, which only holds if every registry key is lowercase — and a duplicate `NAME` would otherwise silently drop a handler. Both are mistakes only made while adding an assertion, so they fail loudly at import. - **Missing Biolink types.** When NodeNorm returns a node with no type, messages show `NO TYPE RETURNED`. The earlier placeholder, `unknown type`, had the shape of a real type — the older Biolink vocabulary was lowercase prose like `chemical entity` — so it could be misread as something Babel actually returned. ### Documentation `gen_docs.py` renders `assertions/README.md` from the handler class attributes, grouping handlers by the service they test rather than by their order in `ASSERTION_HANDLERS` — otherwise a handler registered in the wrong place lands under the wrong heading, and the sync test can't catch it because it regenerates the same wrong output. `tests/test_environment/test_assertions_docs.py` asserts the checked-in README stays in sync. The "Adding a New Assertion Type" instructions live in that generated README and nowhere else. There had been a second copy in the package docstring and the two had already drifted; the docstring now points at the README and describes the module layout instead. ### Tests `tests/test_environment/test_assertions.py` stubs `requests.post` rather than the service, so handlers run against the real `CachedNodeNorm` and exercise its contract instead of a fake restating it. The fixture DB drops one CURIE from the response entirely, reproducing what NodeNorm does for some unknown identifiers. 13 unit tests, all offline. Also registers the `unit` pytest marker, first used by these tests. ### Outcomes - The assertion vocabulary is fixed, documented, and enforced, so the issue parser in the next part of the stack has a stable target. - Every check here runs offline — no network, no GitHub API, no Google Sheet. - Deliberately not here: parsing assertions out of issue bodies, and any wiring into the existing pytest suites. ### Notes for review `gh pr diff` shows hunks in `.gitignore`, `services/nameres.py`, `sources/google_sheets/`, and `tests/conftest.py` that came from #101 and are already on `main` — they merge as no-ops. The net change against `main` is the `assertions/` package, the `normalize_curies()` fix, the two test files, and the pytest marker. `result: dict` is left untyped for NodeNorm response entries. A `TypedDict` would be more precise, but the response shape is Babel's to change and a wrong one is worse than an honest `dict`, so the docstrings say what the dict is instead. ### Verify - `uv run pytest -m unit -q` → 13 assertion tests pass offline. - `uv run python -m src.babel_validation.assertions.gen_docs` reproduces the committed README. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
gaurav
added a commit
that referenced
this pull request
Aug 26, 2026
) Test cases for Babel currently live in a Google Sheet, which is fine for bulk regression data and poor for anything tied to a specific bug: the sheet has no idea which issue a row came from, so nothing tells us when a fixed issue regresses or when an open one has quietly started working. This PR lets a test case live in the GitHub issue that motivated it, and runs those cases as pytest tests. An issue body carries assertions in either of two syntaxes — a wiki-style `{{BabelTest|Resolves|CHEBI:15365}}` marker, or a fenced YAML block whose top-level key is `babel_tests:` — and the harness turns each issue into one pytest item, with each assertion a subtest. Open issues are expected to fail, so an open issue whose assertions all pass is reported as a strict XPASS: the tool is telling you the issue looks closeable. Closed issues are expected to pass, so one that starts failing is telling you to reopen it. **Stack 3 of 4** splitting #67. #101 and #102 have merged, so this now bases on `main`; #104 is the remaining piece. This leaves #67 open, which stays open for #104. ## What's here **Parsing — `src/babel_validation/sources/github/`.** `GitHubIssuesTestCases` finds assertion blocks in issue bodies and resolves each to a handler from the assertions framework that landed in #102. Issue discovery uses the GitHub search API rather than paginating every issue, and reads `body` and `html_url` straight off the search result, so scanning the five configured repositories costs two search requests per repo and no core requests at all. **Harness — `tests/github_issues/`.** One pytest item per issue, parametrized by issue ID, across the repositories listed under `Repositories` in `targets.ini`'s `[DEFAULT]` section. `--issue` targets a specific issue by `org/repo#N`, `repo#N` or `N`. Issue IDs and hydrated issues are cached and shared across `pytest-xdist` workers behind a `FileLock`. **Input validation.** Issue bodies are untrusted: anyone with a GitHub account can write one, nothing reviews it, and we parse it and turn it into live NodeNorm and NameRes calls. That is tolerable while a human watches the run and can hit Ctrl-C; it is not tolerable for the unattended daily runs this is meant to enable. Two of the gaps were reproducible denial of service rather than theory: - The pattern that finds a `babel_tests` block ended with `\s+.*?\s+` before the closing fence. Those three nested backtracking quantifiers made matching cubic on a body that opens a block and never closes the fence — 6.8s at 4KB, 53s at 8KB, and hours at GitHub's 65536-character body limit. It runs during collection, which `pytest-timeout` does not cover, so a single such issue hung the whole run before any test started. Anchoring on the newline that follows `babel_tests:` in a real fenced block removes the ambiguity: 0.0008s at 65536 characters. - `yaml.safe_load` blocks code execution but still resolves aliases, and PyYAML shares the aliased nodes rather than copying them, so the load looks cheap and the cost lands on whatever formats the result afterwards. A 337-byte body of chained anchors reached 25MB in an error message. Aliases are refused outright, which covers merge keys too. On top of those: caps on body length, assertions, param sets and parameters per issue; per-parameter checks for empty, over-long and non-printable values; duplicate YAML keys rejected, because YAML keeps the last silently and the block a reviewer reads would not be the block that runs; issue text `repr()`'d into log lines so an ANSI escape or bidi override cannot reach an operator's terminal; and `--issue` resolving only within the configured repositories. Structural caps fail the whole issue — the fix is to split it across several issues — while a bad parameter fails only its own param set, so the rest of the issue still runs. The caps sit far above anything the configured repositories currently contain, so they are a no-op for real issues today, and they are documented in the generated `assertions/README.md` where issue authors will meet them. **Caches moved out of the shared temp directory.** Both the issue-ID cache and the Google Sheet CSV cache used fixed names in the world-writable temp directory. The issue cache holds the IDs a later run fetches and executes assertions from, so being able to write it was close to being able to choose what the run tests. They now live in `~/.cache/babel-validation`, created `0700`, overridable with `BABEL_VALIDATION_CACHE_DIR` — and failing to create that directory names the override, since a read-only home on a locked-down runner otherwise raises a `PermissionError` that gives no hint an escape hatch exists. The cache sweep in `pytest_configure` also stopped deleting the Google Sheet `.lock` files. 48b1c44 had already removed that for the GitHub issue lock — a concurrent pytest holding the lock keeps its now-unlinked inode while this run creates a fresh one, so two processes end up inside "the" lock — and the Google Sheet path had the same shape. `unlink_if_exists()` now also refuses any path outside the cache directory, because it deletes whatever it is handed and runs before anything else in the session. **Test layout.** The offline tests live in `tests/github_issues/unit/` — `test_syntax.py` (what the two syntaxes mean), `test_discovery.py` (finding issues, identifying them, resolving an ID) and `test_untrusted_input.py` (the guards on body content). A subdirectory rather than three siblings because of the fixture: `tests/github_issues/conftest.py` defines a session-scoped `github_issues_test_cases` that needs a real `GITHUB_TOKEN`, and these tests want a dummy-token parser instead. A `conftest.py` in the subdirectory scopes that override to exactly the files that want it, where putting it in the parent would replace the real fixture for the live `test_github_issues.py` as well. **Documentation.** `CLAUDE.md` gains an `Untrusted Input` section: which inputs are hostile and which (`targets.ini`) are trusted config, and each failure mode above written as the shape to look for rather than as the fix that was applied — so the next parser added here starts from them. The generated `assertions/README.md` carries the caps, where issue authors meet them. And it now says in as many words that a red `pytest tests/github_issues` is the tool working, not a defect to be fixed by editing the assertions, because the obvious reading of eighteen red tests is otherwise the wrong one. And it warns against writing a complete `{{BabelTest|...}}` marker into an issue: this repository is itself in the scanned `Repositories` list, so an issue that merely *describes* an assertion gets collected and runs it. ## What a run produces `pytest -m unit` is fully offline and needs no token: **97 passed**, in about a second. This is what CI runs, and it is green. `pytest tests/github_issues --target dev` currently reports **18 failing issues**, and that is the tool working rather than a defect in it: - **10 open issues XPASS** — every assertion now passes, so they look closeable. - **7 closed issues have failing assertions** — #406, #552, #584, #711, #714, #723, #906 — so they look like they should be reopened. - **1 issue uses an assertion type we have not written yet**: `ShouldNotHaveSynonym`, in NCATSTranslator/Babel#744. Unknown assertion names fail loudly by design rather than being skipped, so this stays a hard failure until #110 lands. It is unrelated to any NodeNorm behaviour. ## What it deliberately does not do - **No new CLI options for the limits.** They are module constants. `pytest --timeout=N` already exists and is the runtime knob for a slow issue. - **No guards around the NodeNorm/NameRes responses.** Those URLs come from `targets.ini`, which is trusted config, not from issue bodies. - **CI runs `-m unit` only**, so nothing here exercises the live GitHub path on a PR. Enabling it needs `issues: read` and a token; the recipe is in a comment in `tests.yaml`, and #114 covers building a tier of tests that need a token but not a full crawl. Worth knowing that without a token the issue tests **skip rather than fail**, so a run can go green having tested nothing. ## Follow-on work Nothing is blocking this merge. Two pieces are tracked separately: - #110 — implement the `ShouldNotHaveSynonym` assertion type, which is the one live failure above that is about this harness rather than about NodeNorm. - #114 — a tier of GitHub API tests that need a token but not a full issue crawl, so CI can cover the integration itself. - #115 — the per-issue size caps are checked after every `GitHubIssueTest` has been built, so an oversized body still constructs a few thousand objects before being rejected. Bounded by GitHub's own body limit and off the network path, so it matters only if the caps are ever tightened much further. 🤖 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
src/babel_validation/carve-out that landed onmainin Reorganize Babel Validation for more extensibility #101, addingsources/github/,assertions/, andtools/alongside the existingcore,services, andsources/google_sheets.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 — round 1
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.Review fixes — round 2
A second review pass found eight issues, all fixed here (
pytest -m unit: 49 → 56 passing).Failures that were being hidden
xfailno longer swallows infrastructure errors.xfail(strict=True)with noraises=treats any exception as the expected failure. Since the marker is applied before the assertion loop and most tracked issues are open, a NodeNorm outage, an HTTP 500, or a bug in our own parsing reported XFAIL — a fully green run across every open issue. Nowraises=AssertionError; the imperativepytest.xfail()calls are unaffected.DoesNotResolveWithno longer passes vacuously.normalize_curiesreturned NodeNorm's response as-is, so a CURIE omitted from it was simply absent from the mapping the handler iterates. The result dict is now seeded with every requested CURIE, so "one entry per input" is an invariant rather than an accident of the pre-warm call order.Crashes on realistic inputs
str._to_listvalidated the container type but not element types, while all eight handlers expectlist[str].HasLabel: - [CHEBI:16480, NO]parsed to[..., False](YAML 1.1 reads bareNOas boolean) →AttributeError; a numeric identifier →TypeErrorfrom_CURIE_RE.match. Numbers now stringify; bareNO/YES/ON/OFFand nulls are rejected with a quote-it hint, since the original spelling is unrecoverable by then.SearchByNamehandles a node with nolabel(.get('label', '')—HasLabelalready guarded this).ResolvesWithTypehandles a node with notype, using thefirst_type()guard added inadd0848.Smaller
ResolvesWithfailure messages are deterministic. The "but expected X" id came from dict order, which depends on set iteration and cache state, so the same failing issue reported different expectations across runs. It now follows param order.csv-to-babeltestsno longer POSTs unvalidated values. The batch pre-warm skipped the_CURIE_REfilter thattest_with_nodenormhas applied sincee322040, so a blank--equivalent-curie-columncell was sent to NodeNorm.unlink_if_existsafter the Reorganize Babel Validation for more extensibility #101 merge) caught onlyFileNotFoundError; on a shared box or self-hosted runner,PermissionErrorescapedpytest_configureand no test ran.New offline coverage:
tests/test_assertion_robustness.py(sparse nodes, omitted CURIEs, message determinism) and three YAML-scalar cases intests/github_issues/test_system.py.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/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].Merged
main(#101)mainhas since landed its own library reorg in #101, which touched the same sharedfiles. Merged in and resolved by taking main's structure and re-applying this branch's
work on top:
services/nameres.py,services/nodenorm.py— main's docstrings,Protocolinterfaces, and the
clear_curie/delete_query→invalidate_curie/invalidate_queryrenames. The
normalize_curiesresult seeding from the round-2 fixes is re-applied,and the "one entry per requested CURIE" guarantee is now documented in the docstring.
sources/google_sheets/google_sheet_test_cases.py— main'scache_ttl_secondsconstructor parameter and relative import. Nothing referenced the removed
CACHE_TTL_SECONDSclass attribute.tests/conftest.py— main'sunlink_if_existsrename, keeping theOSErrorwidening,the issue-cache cleanup, and the
selected_github_issuesfixture.CLAUDE.md,pyproject.toml— this branch is a superset of main's version.uv.lock— regenerated withuv lock.pytest -m unit: 56 passed post-merge. Note the two renames for anything downstream:clear_curie→invalidate_curie,delete_query→invalidate_query.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.