Repository navigation
Tests that pass while hiding problems: audit findings and guards #321
Description
Activity
Another one, seen 27 Sep:
nav-links.spec.ts› every link in the navigation reaches a real page failed once (7 links reported as not rendering) while four spec files ran in parallel locally, then passed alone on re-run. It crawls all 73 links, so a slow page under load reads as a missing one. It needs the same treatment: find what it waits on, and make it wait for the page rather than a fixed time.Progress: the first guard is in (#325). Any e2e test whose pages report an Angular error now fails with the error and its stack. It found two more beyond the tissue form: the FAQ (NG0100) and the qualitative-analysis grid (NG0951), both fixed there. #325 also fixed the nav-links flake noted above (it waits for each page to render now) and a release check that #303 had left looking for the old inferred-events figure. Next: the recordings check.
Second guard in (#326). Replays no longer serve an answer that was cut off mid-stream as a successful empty one. Recording waits for a test's backend requests to finish, which took truncated answers to zero.
npm run check:harrefuses any new ones in CI. Next: reporting skips against a committed list of expected ones. CI skips 17 today, each with a reason; 11 of them are render-service tests that CI has no service for.- added a commit that references this issue
on Sep 28, 2026 Third guard in (#327). Every skipped test is now listed with its reason in each shard's job summary, and a skip that
e2e/expected-skips.jsondoesn't list fails the shard. Of the 17 listed, 11 are render-service tests CI can't run; that gap is next.Render tests now run before each push (#328). The 11 tests that need the render service still skip in CI, where there's no render service and no data. They now run in preflight, the pre-push hook, whenever a push touches what figures are made from. They run against a render service and dev server started from the working tree, with the local backend, and `E2E_REQUIRE_RENDER` makes a missing service a failure rather than a skip. With #317's bug put back in, the step fails the two reaction-download tests.
Still open from this audit:
- the order-dependent content-pages probe;
- self-skipping tests: interactor-threshold, back-button, content-page-urls, hierarchy-scroll;
- tests with no recording falling back to the pool;
- the analysis-results filter race;
- preflight running unit tests before `build:libs`;
- about 40 `catchError(() => of(empty))` in app code.
Progress, 10 Oct:
- Ask for the content endpoints in each test that needs them #392, the order-dependent content-pages probe: each test that needs the content endpoints now runs the probe itself. Before the fix, moving the icon library test to the top made the toc test skip.
- Assert what a test's own setup guarantees, instead of skipping on it #393, the self-skips: these became assertions in back-button, content-page-urls, hierarchy-scroll and interactor-threshold. Each was shown failing where it used to skip. The PSICQUIC second-resource check stays a skip, because a third party decides it, but it now waits for the counts instead of reading them once.
- Wait for the filtered results instead of counting after a pause #391, the analysis-results filter: the test now waits for the filtered count and reads it from the results paginator. The race itself didn't reproduce, even at a 60× CPU slowdown.
- Preflight running unit tests before
build:libs: already fixed. Libraries build first.
Still open: tests with no recording of their own falling back to the shared pool, and the roughly 40
catchError(() => of(empty))in app code.Tests with no recording of their own (#394): the harness now fails a test that has no recording and was answered from the shared pool. The failure names the first borrowed request and the escaped command to record the test. With the guard in, 25 tests failed on it, and each now has its own recording. A recording with no entries doesn't count as the test's own. The full suite passes with the guard.
Left from this audit: the roughly 40
catchError(() => of(empty))in app code that turn a failed request into "nothing here".
A flaky tissue test turned out to have three hidden causes (#320): dead animation bindings that only break development builds, a test acting while a timer was still running, and a recording cut off mid-response. A read-only audit of the whole repo for the same classes of problem found the following. They are listed most impactful first.
Findings
content-pages.spec.ts:52-59probes the node endpoints once per worker, from whichever test runs first. Only some tests have a probe recording, so the toc, DOI, subpathway and contributors tests skip depending on shard order.hierarchy-scroll, where the ancestors call is empty. (Fix the tissue form's leftover animation bindings and its flaky test #320 makes replay prefer a complete copy when one exists. Single truncated copies remain.)downloads.spec.ts,download-feedback.spec.tsanddetail-contents.spec.tsskip because/RenderService/healthisn't recorded and CI has no render service. This is how the reaction-figure bug (Draw a reaction's own figure again when layout=reaction is asked for #317) reached beta.interactor-threshold.spec.ts:542and:361(races the counts),content-page-urls.spec.ts:60,back-button.spec.ts:85, andhierarchy-scroll.spec.ts:43(swallows failed clicks, then skips).search.spec.ts.analysis-results.spec.ts:69-86sleeps a fixed time, then counts once.preflight.shruns unit tests before building the libraries, so local runs test whatever staledist/exists. (CI's order is fixed in Have ReactomeGSA deliver beta's results to this server #319.)NG0…), which is why Fix the tissue form's leftover animation bindings and its flaky test #320's bug went unnoticed. Separately, about 40catchError(() => of(empty))calls in app code turn failures into "nothing here" for readers, e.g. a failed request showing as "no authored reactions".Guards
pageerroror on aconsole.errormatching/NG0\d+/.scripts/check-har.mjsin CI and after recording. It fails on backend entries cut off with no complete copy, 200s with an empty body but a nonzero content-length, and tests that call the backend with no recording.e2e/against.catch(() => {})and runtime-conditionedtest.skipwithout a stated reason.