Skip to content

Commit ffe229b

Browse files
Clare72Clare72
authored
Merge pull request #118 from VirtualFlyBrain/pub_split_query_fix
Pub + split query fix
2 parents a8218ba + fa7c6be commit ffe229b

12 files changed

Lines changed: 1116 additions & 42 deletions

‎.github/workflows/python-test.yml‎

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,10 @@ on:
77
branches: [ main, dev ]
88
workflow_dispatch: # Enables manual triggering
99
schedule:
10-
- cron: '0 0 2 * *' # Runs at 00:00 UTC on the 2nd day of every month
10+
- cron: '0 0 * * 0' # Runs at 00:00 UTC every Sunday (weekly data-health audit)
1111

1212
# One live run per branch. Every run drives the full correctness suite against
13-
# the production VFB backend (Neo4j / SOLR / Owlery), so a series of quick
13+
# the production data sources (VFB's Neo4j / SOLR / Owlery and FlyBase Chado), so a series of quick
1414
# pushes to a PR would otherwise stack several full suites against production
1515
# simultaneously. Superseded runs are cancelled — only the newest commit's
1616
# result is meaningful anyway.
@@ -75,9 +75,19 @@ jobs:
7575
# on today's GitHub-hosted ubuntu runner, so this is not a slowdown —
7676
# it just stops the load on VFB infra changing silently if the hosted
7777
# runner spec grows.
78+
#
79+
# data_health: the '*_live' term_info variants assert on the external
80+
# indexer's built vfb_json content, not this repo's render code, so
81+
# an incomplete/stale production document fails them. They must NOT gate
82+
# code PRs — the PR-blocking versions are fixture-based and deterministic.
83+
# So they run ONLY on the schedule (a data-health audit); on push /
84+
# pull_request they are deselected. See TESTING.md.
85+
MARK=(-m "not data_health")
86+
[ "${{ github.event_name }}" = "schedule" ] && MARK=()
7887
pytest -v -ra -n 4 --dist loadscope \
7988
--ignore=src/test/test_query_performance.py \
8089
--ignore=src/test/test_example_queries.py \
90+
"${MARK[@]}" \
8191
src/test tests 2>&1 | tee pytest_output.log
8292
- name: Flag skipped tests (backend unavailable)
8393
if: always()
@@ -105,7 +115,7 @@ jobs:
105115
} >> "$GITHUB_STEP_SUMMARY"
106116
fi
107117
if [ -n "$summary" ]; then
108-
echo "::warning title=Tests skipped — VFB backend unreachable::${summary}. ${digest:+First failure: ${digest}. }These are NOT test failures and not a problem with this branch: the VFB backend (Neo4j / SOLR / Owlery) did not answer, so those queries went unverified this run. Treat a green check with skips as an incomplete run — re-run once the backend is healthy before relying on it. Full detail (every skipped test with its reason; failing URLs and probe verdicts for backend failures): the job summary and the PR comment."
118+
echo "::warning title=Tests skipped — upstream data source unreachable::${summary}. ${digest:+First failure: ${digest}. }These are NOT test failures and not a problem with this branch: an upstream data source (VFB's Neo4j / SOLR / Owlery, or FlyBase's Chado) did not answer, so those queries went unverified this run. Treat a green check with skips as an incomplete run — re-run once the data sources are healthy before relying on it. Full detail (every skipped test with its reason; failing URLs and probe verdicts for backend failures): the job summary and the PR comment."
109119
else
110120
echo "No tests skipped."
111121
fi
@@ -151,7 +161,7 @@ jobs:
151161
// re-running it would do nothing.
152162
const runUrl = `${process.env.GITHUB_SERVER_URL}/${owner}/${repo}/actions/runs/${context.runId}`;
153163
const rerun = `To re-run: open [this workflow run](${runUrl}) and click ` +
154-
`**Re-run all jobs** once the backend is healthy (re-running the ` +
164+
`**Re-run all jobs** once the data sources are healthy (re-running the ` +
155165
`“Run completeness” check itself does nothing — it has no job behind it).`;
156166
157167
// A neutral (grey) status check so the PR's checks box stops reading
@@ -168,16 +178,16 @@ jobs:
168178
details_url: runUrl,
169179
output: {
170180
title: skipped > 0
171-
? `${skipped} test(s) skipped — backend unreachable (incomplete run)`
181+
? `${skipped} test(s) skipped — upstream data source unreachable (incomplete run)`
172182
: 'All backend tests ran',
173183
summary: skipped > 0
174-
? (`**${skipped}** test(s) were skipped because the VFB backend ` +
175-
`(Neo4j / SOLR / Owlery) did not answer, so those queries went ` +
184+
? (`**${skipped}** test(s) were skipped because an upstream data source ` +
185+
`(VFB's Neo4j / SOLR / Owlery, or FlyBase's Chado) did not answer, so those queries went ` +
176186
`unverified. This is not a branch failure — but the run is ` +
177187
`incomplete.\n\n${rerun}` +
178188
(summary ? '\n\n```\n' + summary + '\n```' : '') +
179189
(outage ? '\n\n' + outage : ''))
180-
: 'Every backend-dependent test reached the VFB backend and ran.',
190+
: 'Every backend-dependent test reached its data source and ran.',
181191
},
182192
});
183193
const comments = await github.paginate(github.rest.issues.listComments,
@@ -187,10 +197,10 @@ jobs:
187197
if (skipped > 0) {
188198
const body = [
189199
marker,
190-
`### ⚠️ ${skipped} test(s) skipped — VFB backend was unreachable`,
200+
`### ⚠️ ${skipped} test(s) skipped — an upstream data source was unreachable`,
191201
'',
192-
`The full suite ran, but **${skipped}** test(s) were **skipped** because the ` +
193-
`VFB backend (Neo4j / SOLR / Owlery) did not answer during this run.`,
202+
`The full suite ran, but **${skipped}** test(s) were **skipped** because ` +
203+
`an upstream data source (VFB's Neo4j / SOLR / Owlery, or FlyBase's Chado) did not answer during this run.`,
194204
'',
195205
'These are **not failures** and **not a problem with this branch** — but those ' +
196206
'queries went **unverified**, so a green check here is an *incomplete* run.',

‎TESTING.md‎

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,7 @@ Most VFBquery tests run **live queries against the production VFB backend**
44
(SOLR, Neo4j, Owlery, FlyBase Chado). That makes them powerful — they catch real
55
regressions in query results — but also easy to write badly: a test that never
66
checks its query returned anything passes forever while the query is silently
7-
broken. A whole class of such tests was found and fixed in Aug 2026; this doc
8-
exists so they don't come back.
7+
broken. This doc exists so that class of test does not creep back in.
98

109
Read this before adding or changing a test.
1110

@@ -69,6 +68,50 @@ So an empty result is **never** an acceptable outcome for a known-populated
6968
term — it is a bug. A backend outage is handled *for you*; you do not need
7069
(and must not add) your own try/except to survive it.
7170

71+
## Fixture vs live (`data_health`): what an empty result actually means
72+
73+
The "empty ⇒ bug" rule assumes empty means **our** code is wrong. That holds
74+
when a test reads the **source of truth (pdb / Neo4j) through this repo's own
75+
query code** — connectivity, the neurotransmitter queries, and the like. There,
76+
an empty result *is* a defect here, so the test asserts real content and is
77+
**PR-blocking**.
78+
79+
It does **not** hold for **term_info** tests. `get_term_info` / `process` /
80+
`term_info_parse_object` *render* a pre-built SOLR `vfb_json` document; that
81+
document is built by a **separate repository** (`VFB_json_schema_indexer`), not
82+
here. If that built document is stale or incomplete (e.g. a pub with no title, a
83+
split class missing its `Expression_pattern` type), our render code faithfully
84+
produces an empty/partial result — which is a problem with the **build**, not
85+
with this repo. Gating a code PR on it is a false signal.
86+
87+
**The rule of thumb:** *if the build code is in this repo, test it here against
88+
the source (empty = fail, PR-blocking); if the build is in another repo, use a
89+
fixture here and test the build at its source.* Connectivity/NT queries build
90+
from pdb here → tested here. term_info documents are built by
91+
`VFB_json_schema_indexer` → **fixtures here, build tested there.**
92+
93+
So term_info tests come in two variants:
94+
95+
| Variant | Reads | Runs | A failure means |
96+
|---|---|---|---|
97+
| **fixture** (plain name) | a committed complete `vfb_json` doc in `src/test/fixtures/term_info/` | **every PR** (blocking) | this repo's **render code** regressed |
98+
| **live** (`<name>_live`, `@pytest.mark.data_health`) | the live SOLR `vfb_json` document | **schedule only** | the **built document / pipeline** (`VFB_json_schema_indexer`) is wrong — a data-health signal, not a code gate |
99+
100+
Mechanics:
101+
102+
* Fixtures are captured from the PDB (the source of truth) — i.e. what the
103+
indexer *should* produce — and verified to render the asserted output. They
104+
make the code assertion **deterministic and independent of the live SOLR
105+
`vfb_json` document and the caches**, so code improvements are never blocked by
106+
stale data.
107+
* The live variant carries `@pytest.mark.data_health`. PRs run
108+
`-m 'not data_health'`; the weekly `schedule` in `python-test.yml` runs the
109+
full set. A red scheduled `data_health` run means production `vfb_json` (or the
110+
indexer that builds it) needs attention — it does **not** block merges.
111+
* Do **not** paper over an empty term_info doc with a skip guard — that violates
112+
rule 2. Use the fixture for the code assertion; let the `_live` variant fail on
113+
the schedule, where empty is exactly the signal you want.
114+
72115
## The rules
73116

74117
### 1. Assert content, not just shape

‎pyproject.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ build-backend = "setuptools.build_meta"
1212
# blocks in performance.md — pure noise.
1313
markers = [
1414
"integration: integration tests that exercise the live VFB upstream (Neo4j, SOLR, owlery). Skip via -m 'not integration'.",
15+
"data_health: live SOLR content-completeness checks (the '*_live' variants of the term_info tests). These assert on the external indexer's built 'vfb_json' documents, not this repo's render code, so an empty/incomplete production document fails them. Run on the schedule; deselected on PRs via -m 'not data_health'. See TESTING.md.",
1516
]
1617

1718
# Ceiling on any single test. The upstream Neo4j intermittently stops answering

‎src/test/fixtures/term_info/pub_FBrf0242477.json‎

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
{
2+
"term": {
3+
"core": {
4+
"iri": "http://flybase.org/reports/FBrf0242477",
5+
"symbol": "",
6+
"types": [
7+
"Entity",
8+
"Individual",
9+
"pub"
10+
],
11+
"short_form": "FBrf0242477",
12+
"unique_facets": [
13+
"pub"
14+
],
15+
"label": "Dolan et al., 2019, eLife 8: e43079"
16+
},
17+
"description": [],
18+
"comment": []
19+
},
20+
"query": "Get JSON for pub",
21+
"version": "0dcd736",
22+
"dataset_license": [
23+
{
24+
"license": {
25+
"core": {
26+
"iri": "http://virtualflybrain.org/reports/VFBlicense_CC_BY_4_0",
27+
"symbol": "CC_BY",
28+
"types": [
29+
"Entity",
30+
"Individual",
31+
"License"
32+
],
33+
"short_form": "VFBlicense_CC_BY_4_0",
34+
"unique_facets": [],
35+
"label": "CC-BY_4.0"
36+
},
37+
"icon": "http://mirrors.creativecommons.org/presskit/buttons/88x31/png/by.png",
38+
"link": "https://creativecommons.org/licenses/by/4.0/legalcode"
39+
},
40+
"dataset": {
41+
"core": {
42+
"iri": "http://virtualflybrain.org/reports/LateralHorn2019",
43+
"symbol": "",
44+
"types": [
45+
"Entity",
46+
"Individual",
47+
"DataSet"
48+
],
49+
"short_form": "LateralHorn2019",
50+
"unique_facets": [],
51+
"label": "FlyLight split-GAL4 lines for Lateral Horn"
52+
},
53+
"link": ""
54+
}
55+
},
56+
{
57+
"license": {
58+
"core": {
59+
"iri": "http://virtualflybrain.org/reports/VFBlicense_CC_BY_4_0",
60+
"symbol": "CC_BY",
61+
"types": [
62+
"Entity",
63+
"Individual",
64+
"License"
65+
],
66+
"short_form": "VFBlicense_CC_BY_4_0",
67+
"unique_facets": [],
68+
"label": "CC-BY_4.0"
69+
},
70+
"icon": "http://mirrors.creativecommons.org/presskit/buttons/88x31/png/by.png",
71+
"link": "https://creativecommons.org/licenses/by/4.0/legalcode"
72+
},
73+
"dataset": {
74+
"core": {
75+
"iri": "http://virtualflybrain.org/reports/Dolan2019",
76+
"symbol": "",
77+
"types": [
78+
"Entity",
79+
"Individual",
80+
"DataSet"
81+
],
82+
"short_form": "Dolan2019",
83+
"unique_facets": [],
84+
"label": "GAL4 Split expression patterns from Dolan et al. 2019"
85+
},
86+
"link": ""
87+
}
88+
},
89+
{
90+
"license": {
91+
"core": {
92+
"iri": "http://virtualflybrain.org/reports/VFBlicense_CC_BY_4_0",
93+
"symbol": "CC_BY",
94+
"types": [
95+
"Entity",
96+
"Individual",
97+
"License"
98+
],
99+
"short_form": "VFBlicense_CC_BY_4_0",
100+
"unique_facets": [],
101+
"label": "CC-BY_4.0"
102+
},
103+
"icon": "http://mirrors.creativecommons.org/presskit/buttons/88x31/png/by.png",
104+
"link": "https://creativecommons.org/licenses/by/4.0/legalcode"
105+
},
106+
"dataset": {
107+
"core": {
108+
"iri": "http://virtualflybrain.org/reports/FlyLight2019LateralHorn2019",
109+
"symbol": "",
110+
"types": [
111+
"Entity",
112+
"Individual",
113+
"DataSet"
114+
],
115+
"short_form": "FlyLight2019LateralHorn2019",
116+
"unique_facets": [],
117+
"label": "FlyLight split-GAL4 lines for Lateral Horn"
118+
},
119+
"link": ""
120+
}
121+
},
122+
{
123+
"license": {
124+
"core": {
125+
"iri": "http://virtualflybrain.org/reports/VFBlicense_CC_BY_4_0",
126+
"symbol": "CC_BY",
127+
"types": [
128+
"Entity",
129+
"Individual",
130+
"License"
131+
],
132+
"short_form": "VFBlicense_CC_BY_4_0",
133+
"unique_facets": [],
134+
"label": "CC-BY_4.0"
135+
},
136+
"icon": "http://mirrors.creativecommons.org/presskit/buttons/88x31/png/by.png",
137+
"link": "https://creativecommons.org/licenses/by/4.0/legalcode"
138+
},
139+
"dataset": {
140+
"core": {
141+
"iri": "http://virtualflybrain.org/reports/FafbDolan2019",
142+
"symbol": "",
143+
"types": [
144+
"Entity",
145+
"Individual",
146+
"DataSet"
147+
],
148+
"short_form": "FafbDolan2019",
149+
"unique_facets": [],
150+
"label": "EM FAFB Dolan et al. 2019"
151+
},
152+
"link": "https://fafb.catmaid.virtualflybrain.org"
153+
}
154+
}
155+
],
156+
"pub_specific_content": {
157+
"title": "Neurogenetic dissection of the Drosophila lateral horn reveals major outputs, diverse behavioural functions, and interactions with the mushroom body.",
158+
"FlyBase": "FBrf0242477",
159+
"PubMed": "31112130",
160+
"DOI": "10.7554/eLife.43079"
161+
}
162+
}

‎src/test/fixtures/term_info/pub_FBrf0243986.json‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
{
2+
"term": {
3+
"core": {
4+
"iri": "http://flybase.org/reports/FBrf0243986",
5+
"symbol": "",
6+
"types": [
7+
"Entity",
8+
"Individual",
9+
"pub"
10+
],
11+
"short_form": "FBrf0243986",
12+
"unique_facets": [
13+
"pub"
14+
],
15+
"label": "Sayin et al., 2019, Neuron 104(3): 544--558.e6"
16+
},
17+
"description": [],
18+
"comment": []
19+
},
20+
"query": "Get JSON for pub",
21+
"version": "0dcd736",
22+
"dataset_license": [
23+
{
24+
"license": {
25+
"core": {
26+
"iri": "http://virtualflybrain.org/reports/VFBlicense_CC_BY_4_0",
27+
"symbol": "CC_BY",
28+
"types": [
29+
"Entity",
30+
"Individual",
31+
"License"
32+
],
33+
"short_form": "VFBlicense_CC_BY_4_0",
34+
"unique_facets": [],
35+
"label": "CC-BY_4.0"
36+
},
37+
"icon": "http://mirrors.creativecommons.org/presskit/buttons/88x31/png/by.png",
38+
"link": "https://creativecommons.org/licenses/by/4.0/legalcode"
39+
},
40+
"dataset": {
41+
"core": {
42+
"iri": "http://virtualflybrain.org/reports/Sayin2019",
43+
"symbol": "",
44+
"types": [
45+
"Entity",
46+
"Individual",
47+
"DataSet"
48+
],
49+
"short_form": "Sayin2019",
50+
"unique_facets": [],
51+
"label": "EM FAFB Sayin et al 2019"
52+
},
53+
"link": "https://fafb.catmaid.virtualflybrain.org"
54+
}
55+
}
56+
],
57+
"pub_specific_content": {
58+
"title": "A Neural Circuit Arbitrates between Persistence and Withdrawal in Hungry Drosophila.",
59+
"FlyBase": "FBrf0243986",
60+
"PubMed": "31471123",
61+
"DOI": "10.1016/j.neuron.2019.07.028"
62+
}
63+
}

0 commit comments

Comments
 (0)