overhaul of testing suite and fix bug where rows dropped if no thumbnail

.github/workflows/examples.yml

@@ -8,6 +8,13 @@ on:
   schedule:
     - cron: '0 0 1 * *'  # Runs at 00:00 UTC on the 1st day of every month
 
+# One live run per branch — the README examples run real queries against the
+# production VFB backend, so superseded runs are cancelled rather than left to
+# pile up alongside the other live-backend workflows.
+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: true
+
 jobs:
   test-examples:
     runs-on: ubuntu-latest
@@ -21,8 +28,10 @@ jobs:
       - name: Install dependencies
         run: |
             python -m pip install --upgrade pip
-            pip install -r requirements.txt
-            pip install deepdiff colorama
+            # Runtime set + test tooling. deepdiff and colorama, previously
+            # installed ad hoc here, are now declared in tests/requirements.txt
+            # alongside the rest of the test-only dependencies.
+            pip install -r requirements.txt -r tests/requirements.txt
             pip install .
       - name: Check SOLR availability
         run: |

.github/workflows/performance-test.yml

@@ -9,6 +9,15 @@ on:
   schedule:
     - cron: '0 2 * * *'  # Runs daily at 2 AM UTC
 
+# One live run per branch. This job hammers production VFB infra for up to two
+# hours; several stacked runs of it are the heaviest load this repo can put on
+# the backend. cancel-in-progress is limited to pull_request events because the
+# push and scheduled runs commit the refreshed performance.md back to main —
+# cancelling one of those loses the measurement rather than just duplicating it.
+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: ${{ github.event_name == 'pull_request' }}
+
 jobs:
   performance:
     name: "Performance Test"
@@ -35,12 +44,12 @@ jobs:
       - name: Install dependencies
         run: |
           python -m pip install --upgrade pip
-          python -m pip install --upgrade -r requirements.txt
+          # Runtime set + test tooling (pytest, pytest-timeout, pytest-xdist —
+          # the last powers the parallel Connectivity Tests step below). The two
+          # files are separate so test tooling stays out of the published
+          # package and the Docker image; see tests/requirements.txt.
+          python -m pip install --upgrade -r requirements.txt -r tests/requirements.txt
           python -m pip install -e .  # Editable install ensures we test the actual source code
-          # pytest-xdist powers the `-n auto` parallel run in the
-          # Connectivity Tests step below. Installed here rather than in
-          # requirements.txt because it's only needed at test time.
-          python -m pip install pytest-xdist
 
       - name: Test Owlery Connectivity
         run: |
@@ -127,20 +136,23 @@ jobs:
         run: |
           # These files are pytest-style (plain classes + @pytest.mark.integration).
           # Run with pytest so the markers are honoured and collection works.
-          # `-n auto` parallelises across all available CPU cores via
-          # pytest-xdist (typically 2-4 on GitHub-hosted ubuntu runners). The
-          # connectivity tests hit the live upstream and don't share fixtures
-          # or in-process state, so they parallelise cleanly. SOLR cache writes
-          # are idempotent so a race between two cold workers on the same
-          # term_id just produces two identical writes.
+          # `-n 4` parallelises via pytest-xdist. The connectivity tests hit the
+          # live upstream and don't share fixtures or in-process state, so they
+          # parallelise cleanly; SOLR cache writes are idempotent, so a race
+          # between two cold workers on the same term_id just produces two
+          # identical writes.
+          # Capped at 4 (was 8): the GitHub-hosted ubuntu runner only has 4
+          # vCPUs, so 8 workers bought no extra throughput — it just doubled the
+          # number of concurrent connectomics queries pointed at production
+          # Neo4j. 4 matches the runner and the "Run Tests" workflow.
           # Auto-retry once on failure — same rationale as Run Performance Test.
           # Per-step log is concatenated into the canonical
           # performance_test_output.log only after the final (possibly retried)
           # attempt, so the failure-detection step at the end of the workflow
           # grades on the last attempt only.
           set +e
           echo "=== Connectivity test attempt 1/2 (parallel) ==="
-          pytest -v -s -n 8 \
+          pytest -v -s -n 4 \
             src/test/test_neuron_neuron_connectivity.py \
             src/test/test_neuron_region_connectivity.py \
             src/test/test_upstream_class_connectivity.py \
@@ -154,7 +166,7 @@ jobs:
           fi
           echo ""
           echo "=== Connectivity attempt 1 failed (exit $FIRST_EXIT). Retrying once with warm cache. ==="
-          pytest -v -s -n 8 \
+          pytest -v -s -n 4 \
             src/test/test_neuron_neuron_connectivity.py \
             src/test/test_neuron_region_connectivity.py \
             src/test/test_upstream_class_connectivity.py \

.github/workflows/python-test.yml

@@ -9,6 +9,23 @@ on:
   schedule:
     - cron: '0 0 2 * *'  # Runs at 00:00 UTC on the 2nd day of every month
 
+# One live run per branch. Every run drives the full correctness suite against
+# the production VFB backend (Neo4j / SOLR / Owlery), so a series of quick
+# pushes to a PR would otherwise stack several full suites against production
+# simultaneously. Superseded runs are cancelled — only the newest commit's
+# result is meaningful anyway.
+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: true
+
+# Needed by the "Comment skip warning on PR" step to post/update a sticky
+# comment on the PR conversation. (A ::warning:: annotation alone only shows on
+# the Checks/Files tabs — the conversation timeline stays green despite skips.)
+permissions:
+  contents: read
+  pull-requests: write   # sticky skip-warning comment on the PR conversation
+  checks: write          # a neutral (grey) "Backend coverage" check when skipped
+
 jobs:
   notebooks:
     name: "Run Tests"
@@ -19,18 +36,154 @@ jobs:
       - name: Set up Python
         uses: actions/setup-python@v2
         with:
-          python-version: 3.8
+          # Match the Performance Test workflow (the repo's other pytest runner)
+          # rather than the retired 3.8 this job used when it ran a single
+          # unittest file, so pytest / pytest-xdist resolve the same versions.
+          python-version: '3.10'
       - name: Install dependencies
         run: |
          python -m pip install -U pip
-         python -m pip install -U -r requirements.txt
+         # Runtime set + test tooling (pytest, pytest-timeout, pytest-xdist).
+         # See tests/requirements.txt for why the two are separate files.
+         python -m pip install -U -r requirements.txt -r tests/requirements.txt
          python -m pip install .
-      - name: Run term_info_queries_test
+      - name: Run full test suite
         env:
           VFBQUERY_CACHE_ENABLED: 'false'
           MPLBACKEND: 'Agg'
           VISPY_GL_LIB: 'osmesa'
           VISPY_USE_EGL: '0'
         run: |
           export PYTHONPATH=$PYTHONPATH:$PWD/
-          python -m unittest -v src/test/term_info_queries_test.py
+          set -o pipefail
+          # Full correctness suite across src/test and tests (was: only
+          # term_info_queries_test.py). Parallel via pytest-xdist, grouped per
+          # file (--dist loadscope) so each file's backend connections stay on
+          # one worker; the 300s per-test timeout from pyproject.toml bounds any
+          # single hung upstream call. `-ra` prints a summary of skips/failures.
+          # A backend outage SKIPS the affected tests (see conftest.py) rather
+          # than failing them; empty-but-connected results still fail. The next
+          # step turns any skips into a PR-visible warning.
+          # Excludes: test_query_performance.py — wall-clock threshold
+          # assertions that flap under parallel load, already gated by the
+          # dedicated "Performance Test" workflow; and test_examples_diff.py /
+          # test_examples_code.py — README-example scripts (no pytest tests,
+          # pull in deepdiff/colorama) run by the "Test VFBquery examples"
+          # workflow instead.
+          # -n 4 rather than -n auto: an explicit cap on how many concurrent
+          # query streams one run points at production. `auto` happens to be 4
+          # on today's GitHub-hosted ubuntu runner, so this is not a slowdown —
+          # it just stops the load on VFB infra changing silently if the hosted
+          # runner spec grows.
+          pytest -v -ra -n 4 --dist loadscope \
+            --ignore=src/test/test_query_performance.py \
+            --ignore=src/test/test_examples_diff.py \
+            --ignore=src/test/test_examples_code.py \
+            src/test tests 2>&1 | tee pytest_output.log
+      - name: Flag skipped tests (backend unavailable)
+        if: always()
+        run: |
+          # Skips are invisible on the PR otherwise (a pass+skip run is a green
+          # check). Surface them as a warning annotation so a backend outage —
+          # which the conftest.py skip hook turns into skips rather than a false
+          # red — is visible without opening the Actions logs.
+          if [ ! -f pytest_output.log ]; then
+            echo "No test output captured."; exit 0
+          fi
+          summary=$(grep -Eo '[0-9]+ skipped' pytest_output.log | tail -1 || true)
+          if [ -n "$summary" ]; then
+            echo "::warning title=Tests skipped — VFB backend unreachable::${summary}. 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. See the job log for the list."
+          else
+            echo "No tests skipped."
+          fi
+      - name: Comment skip warning on PR
+        # The ::warning:: above only surfaces on the Checks/Files tabs; the PR
+        # conversation still shows a green check. Post a sticky comment there so a
+        # skipped (== incomplete) run is visible without opening the Actions logs.
+        # Same-repo PRs only — a fork PR gets a read-only token and can't comment.
+        if: always() && github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository
+        uses: actions/github-script@v7
+        with:
+          script: |
+            const fs = require('fs');
+            const marker = '<!-- vfb-skipped-tests-warning -->';
+
+            let skipped = 0, summary = '';
+            try {
+              const log = fs.readFileSync('pytest_output.log', 'utf8');
+              const s = [...log.matchAll(/(\d+) skipped/g)];
+              if (s.length) skipped = parseInt(s[s.length - 1][1], 10);
+              const line = log.match(/^=+ (.+ in [\d.]+s.*?) =+\s*$/gm);
+              if (line) summary = line[line.length - 1].replace(/=/g, '').trim();
+            } catch (e) {
+              core.info('No pytest_output.log to read: ' + e.message);
+            }
+
+            const { owner, repo } = context.repo;
+            const issue_number = context.issue.number;
+
+            // Direct link to THIS run so the reader re-runs the right thing: the
+            // "Run Tests" job (this "${{ github.workflow }}" workflow), NOT the
+            // neutral status check below — that check has no job behind it, so
+            // re-running it would do nothing.
+            const runUrl = `${process.env.GITHUB_SERVER_URL}/${owner}/${repo}/actions/runs/${context.runId}`;
+            const rerun = `To re-run: open [this workflow run](${runUrl}) and click ` +
+              `**Re-run all jobs** once the backend is healthy (re-running the ` +
+              `“Run completeness” check itself does nothing — it has no job behind it).`;
+
+            // A neutral (grey) status check so the PR's checks box stops reading
+            // as a plain green pass when the run was actually incomplete. Neutral
+            // does not fail the PR or block merge — it just isn't "success". Named
+            // "Run completeness" (a verdict, not a runnable job) so it isn't
+            // mistaken for the thing to re-run.
+            const head_sha = context.payload.pull_request.head.sha;
+            await github.rest.checks.create({
+              owner, repo, head_sha,
+              name: 'Run completeness',
+              status: 'completed',
+              conclusion: skipped > 0 ? 'neutral' : 'success',
+              details_url: runUrl,
+              output: {
+                title: skipped > 0
+                  ? `${skipped} test(s) skipped — backend unreachable (incomplete run)`
+                  : 'All backend tests ran',
+                summary: skipped > 0
+                  ? (`**${skipped}** test(s) were skipped because the VFB backend ` +
+                     `(Neo4j / SOLR / Owlery) did not answer, so those queries went ` +
+                     `unverified. This is not a branch failure — but the run is ` +
+                     `incomplete.\n\n${rerun}` +
+                     (summary ? '\n\n```\n' + summary + '\n```' : ''))
+                  : 'Every backend-dependent test reached the VFB backend and ran.',
+              },
+            });
+            const comments = await github.paginate(github.rest.issues.listComments,
+              { owner, repo, issue_number, per_page: 100 });
+            const existing = comments.find(c => c.body && c.body.includes(marker));
+
+            if (skipped > 0) {
+              const body = [
+                marker,
+                `### ⚠️ ${skipped} test(s) skipped — VFB backend was unreachable`,
+                '',
+                `The full suite ran, but **${skipped}** test(s) were **skipped** because the ` +
+                `VFB backend (Neo4j / SOLR / Owlery) did not answer during this run.`,
+                '',
+                'These are **not failures** and **not a problem with this branch** — but those ' +
+                'queries went **unverified**, so a green check here is an *incomplete* run.',
+                '',
+                '> ' + rerun,
+                summary ? '\n```\n' + summary + '\n```' : '',
+                '',
+                '<sub>Posted automatically. This comment is removed once a run completes with zero skips.</sub>',
+              ].join('\n');
+              if (existing) {
+                await github.rest.issues.updateComment({ owner, repo, comment_id: existing.id, body });
+              } else {
+                await github.rest.issues.createComment({ owner, repo, issue_number, body });
+              }
+              core.warning(`${skipped} test(s) skipped — posted PR comment.`);
+            } else if (existing) {
+              // Clean run: drop the stale warning so the conversation reflects reality.
+              await github.rest.issues.deleteComment({ owner, repo, comment_id: existing.id });
+              core.info('Zero skips — removed the previous skip-warning comment.');
+            }

.github/workflows/test-lint.yml

@@ -0,0 +1,35 @@
+name: Test Lint
+
+# Fast, static check that a PR doesn't introduce the silently-passing test
+# anti-patterns documented in TESTING.md (empty-suppressing guards, stale keys,
+# error-swallowing except blocks). Only the lines the PR ADDS are checked, and a
+# `# test-lint: allow` comment opts a line out for a genuine exception.
+
+on:
+  pull_request:
+    branches: [ main, dev ]
+  workflow_dispatch:
+
+# Purely static — no backend involved — but there is no value in finishing a
+# lint of a commit that has already been superseded.
+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: true
+
+jobs:
+  test-lint:
+    name: "Test conventions"
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          fetch-depth: 0   # need history to diff against the base branch
+
+      - uses: actions/setup-python@v5
+        with:
+          python-version: '3.10'
+
+      - name: Lint added test lines
+        run: |
+          git fetch --no-tags --quiet origin "${{ github.base_ref }}"
+          python scripts/lint_tests.py "origin/${{ github.base_ref }}"

README.md

@@ -9,6 +9,11 @@ pip install --upgrade vfbquery
 HTTP API, the `/combine` set-algebra reference, the lightweight `vfbquery-client` package, and the
 query catalogue, all rendered and cross-linked. This README is the quick start.
 
+🧪 **Adding or changing a test?** Read **[TESTING.md](TESTING.md)** first. The suite runs live
+queries against the VFB backend, so tests must assert real content (never suppress an empty result,
+never swallow errors, always verify fixtures return data). Those rules exist because a batch of
+silently-passing tests was found and fixed — the doc is how we keep them fixed.
+
 ## 🚀 Performance & Caching
 
 VFBquery includes intelligent SOLR-based caching for optimal performance: