[alex-rawlings-yyc]

Closes #595.

Summary

• A custom Jest environment, jest.environment.ts, extends the stock jsdom one to fail any test that calls console.error or console.warn, without adding a dependency. A log fails the test that is running when it lands. A log from a test's cleanup hook fails that test. A log from loading a test file, from its afterAll, or after its last test fails the file. A test that expects a log silences it with jest.spyOn(console, …).mockImplementation(...), as before. Redux Toolkit's dev-only warning that an invariant check ran slowly is silenced, because load alone can cause it.
• The environment waits one real timer turn before a file is torn down. Before, logs that landed this late were lost on fast machines, which is why the warnings showed only in CI. Fake timers installed by a test file never replace the environment's own setTimeout, so a file that leaves them on still finishes.
• Fixes the warnings the issue lists. Most tests now wait for the settled state with waitFor or findBy*. Where the async work lands without any visible change, the test unmounts at the end to abandon it.
• console-guard.test.ts runs a child Jest over fixtures in src/__tests__/fixtures/ to check the guard's behavior. It adds about 3 seconds to a run.
• AGENTS.md describes the guard.

Considered and rejected

• An afterEach in jest.setup.ts. It is shorter and keeps the stock environment, but hooks in setupFilesAfterEnv run before a test file's own top-level hooks. So a log from a file's afterEach, RTL's auto-cleanup included, would be blamed on the next test, or lost after the last one. A log from afterAll would be lost. So would a log that lands after the last test, which is the CI-only symptom from Fail Jest tests that log unexpected console errors or warnings #595. A log from loading the file would be blamed on its first test.
• jest-fail-on-console. It adds a dependency and also works through setup-file hooks. Its afterEach puts the original console back, so a log from a file's own afterEach, from afterAll, or after the last test prints but fails nothing. A log from loading the file fails every test in it.

Test plan

• npm test passes with no ● Console blocks
• npm run lint
• CI test jobs log no console output

🤖 Generated with Claude Code

---

This change is 

Summary by CodeRabbit

• Tests
  • Test runs now report unexpected console errors and warnings as failures, including messages logged after a test finishes.
  • Test guidance now covers handling expected console output and asynchronous work.
  • Updated tests wait for asynchronous results and clean up rendered components and hooks, improving test reliability.

[coderabbitai]

Important

• Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: c352cc49-2778-4990-ba8a-f81672a6a502

Walkthrough

Jest now uses a custom JSDOM environment that reports unexpected console.error and console.warn calls as test or file errors. Fixture-based tests cover the guard. Affected tests also wait for asynchronous updates or unmount rendered components.

Changes: Jest Console Guard

Layer / File(s)	Summary
Console guard and validation AGENTS.md, jest.config.ts, jest.environment.ts, src/__tests__/console-guard.test.ts, src/__tests__/fixtures/console-guard-*.fixture.ts	Jest uses a custom environment to capture console errors and warnings and report them at test or file completion. Fixture-based tests cover unexpected and late logs, mocked console output, cleanup hooks, teardown, and fake timers. Test guidance describes the guard.
Asynchronous assertions and cleanup src/__tests__/components/AnalysisCatalogPanel.test.tsx, src/__tests__/components/CatalogSenseLinks.test.tsx, src/__tests__/components/modals/SaveAsProjectModal.test.tsx, src/__tests__/hooks/useLexiconRegistry.test.ts, src/__tests__/hooks/useSourceTextReader.test.ts	Affected tests wait for resolved content or state and unmount rendered components after their assertions.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Low

Sequence Diagram(s)

sequenceDiagram
  participant JestRunner
  participant ConsoleGuardEnvironment
  participant Console
  JestRunner->>ConsoleGuardEnvironment: Set up environment and install guard
  Console->>ConsoleGuardEnvironment: Record error or warning
  JestRunner->>ConsoleGuardEnvironment: Report test completion
  ConsoleGuardEnvironment->>JestRunner: Add buffered logs to test errors
  JestRunner->>ConsoleGuardEnvironment: Report run completion
  ConsoleGuardEnvironment->>JestRunner: Add remaining logs to file errors

Loading

Possibly related PRs

• sillsdev/interlinearizer-extension#514: Both PRs address the lifecycle of the asynchronous lexicon watch used by useLexiconRegistry.
• sillsdev/interlinearizer-extension#553: Both PRs concern asynchronous sense resolution and display in CatalogSenseLinks.

Suggested reviewers: imnasnainaec

Merge Risk: Low · previous review 6782f

Unexpected console output in the guard’s own test can pass unnoticed. The gap is limited to that test harness.

Pre-merge checks: 5

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: failing Jest tests that log unexpected console errors or warnings.
Linked Issues check		Directly linked issue [#595] has four coding requirements. (1) Unexpected console.error or console.warn fails the test that logs it. Implemented in jest.environment.ts: ConsoleGuardEnvironment replace…
Out of Scope Changes check		Each changed area connects to [#595]. jest.environment.ts and jest.config.ts implement the guard and select it as the test environment. console-guard.test.ts and the console-guard fixtures verify the …
Docstring Coverage		No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…

Full details: Linked Issues check

Explanation

Directly linked issue [#595] has four coding requirements. (1) Unexpected console.error or console.warn fails the test that logs it. Implemented in jest.environment.ts: ConsoleGuardEnvironment replaces both methods at the setup event, buffers formatted arguments and stacks, and adds buffered logs to event.test.errors at test_done. Fixture cases in console-guard.test.ts cover this. (2) Expected logs stay silenced with jest.spyOn(console, ...).mockImplementation. A spy replaces the guarded method, so the log does not reach the buffer. Covered by the 'logs an expected error through a spy' fixture. (3) Late logs fail the running test or the file. run_finish waits one real setTimeout turn, then adds remaining logs to state.unhandledErrors. Fixture cases cover late errors and afterAll logs. (4) The 20 React act(...) warnings in the five named suites are addressed. The test diffs for useSourceTextReader, useLexiconRegistry, CatalogSenseLinks, SaveAsProjectModal, and AnalysisCatalogPanel add waitFor/findBy waits or unmount calls. Whether these edits remove all 20 calls, and whether npm test shows no '● Console' blocks, was not confirmed by running the suites. The author reports both as passing. Two notes: the PR description says jest.setup.ts holds the guard, but jest.setup.ts at the reviewed head only imports @testing-library/jest-dom; the guard is in jest.environment.ts. This is a description error, not a missing requirement. The unmount fixes abandon pending work instead of asserting settled state. The issue lists waits or act flushing as examples, and its done-when criteria concern warnings only, so unmount is acceptable.

Full details: Out of Scope Changes check

Explanation

Each changed area connects to [#595]. jest.environment.ts and jest.config.ts implement the guard and select it as the test environment. console-guard.test.ts and the console-guard fixtures verify the guard behavior. .eslintrc.js gives jest.setup* files Jest globals, which supports the guard setup. AGENTS.md documents the guard and its silencing and waiting rules. The test edits in the five suites named in the issue are the warning fixes the issue requests. The added child Jest run costs about 3 seconds, as the PR description states. This cost is a test-runtime trade-off, not an unrelated change. No change at the reviewed head lacks a connection to the issue requirements. The fixture test matching and jest.config.ts exclusions were not inspected in full, so this assessment does not confirm that fixtures are excluded from the default npm test run. The PR reports npm test as passing.

Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 13 files. (1 skipped: 1 unsupported.)

Finishing Touches

Generate docstrings

• Commit to this branch
• Create a new PR

Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Tip

🎯 Customize your risk assessments

Guide how CodeRabbit assesses merge risk and security architecture risk. Highlight sensitive changes and explain your security boundaries using Merge Risk Instructions and Security Risk Instructions in your review settings. Security architecture risk assessments require the Advanced plan or above.

✅ Automatic approvals, with your rules

Let CodeRabbit approve pull requests once required threads are resolved, pre-merge checks pass, and your risk limits are met. Enable Automatic approvals in review settings, with overrides for teams or file paths.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 45f69ced-c64e-4d24-911d-d72107cf5525

Commits

Reviewing files that changed from the base of the PR and between 3fffffe and 6782feb.

Files selected for processing: 14

• AGENTS.md
• jest.config.ts
• jest.environment.ts
• src/__tests__/components/AnalysisCatalogPanel.test.tsx
• src/__tests__/components/CatalogSenseLinks.test.tsx
• src/__tests__/components/modals/SaveAsProjectModal.test.tsx
• src/__tests__/console-guard.test.ts
• src/__tests__/fixtures/console-guard-after-all.fixture.ts
• src/__tests__/fixtures/console-guard-after-last-test.fixture.ts
• src/__tests__/fixtures/console-guard-cleanup-hooks.fixture.ts
• src/__tests__/fixtures/console-guard-fake-timers.fixture.ts
• src/__tests__/fixtures/console-guard.fixture.ts
• src/__tests__/hooks/useLexiconRegistry.test.ts
• src/__tests__/hooks/useSourceTextReader.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

[imnasnainaec]

Drafted by Claude Opus 5.5, mostly reviewed by me.

The description doesn't match the diff. It says the guard is in jest.setup.ts and that .eslintrc.js gives jest.setup* files Jest globals. In the diff, the guard is in jest.environment.ts and .eslintrc.js isn't changed. Those lines look left over from an earlier version.

Module-scope logs fail an innocent test. Jest sets the environment's console and sends setup before it loads setupFilesAfterEnv and the test module, so a console.warn at a test file's module scope is caught. But the next test_done reports it, which fails the file's first test rather than the file. Circus sends run_start after the module loads and before any beforeAll, so one more case would move those logs onto the file:

case 'run_start':
  // Logs from loading the test file belong to the file, not its first test.
  this.takeUnexpectedLogs(state.unhandledErrors);
  break;

No fixture covers module-scope logging yet. One that warns at module scope and then defines two passing tests would pin down the behavior, whichever way you go.

A simpler alternative to weigh. The description rules out adding a dependency, which covers jest-fail-on-console. It doesn't rule out doing the same thing in jest.setup.ts, which keeps the stock jsdom environment and takes about a dozen lines:

import { format } from 'node:util';

const unexpectedLogs: string[] = [];
(['error', 'warn'] as const).forEach((level) => {
  console[level] = (...args: unknown[]) => {
    unexpectedLogs.push(`console.${level}: ${format(...args)}\n${new Error().stack}`);
  };
});

afterEach(() => {
  const logs = unexpectedLogs.splice(0);
  if (logs.length > 0) throw new Error(`Unexpected console output:\n\n${logs.join('\n\n')}`);
});

Like the environment, this records logs and fails the test afterwards. It doesn't throw from console.error itself, which would unwind through React's setState and could be swallowed by app-level catch handlers. It fails a test that logs, a test that is running when a late log lands, and the first test after a module-scope log. Expected logs are still silenced with jest.spyOn.

From how jest-circus orders hooks, it would miss three cases that jest.environment.ts catches:

• A log from a top-level afterEach in the test file, including RTL's auto-cleanup. Hooks in setupFilesAfterEnv are declared before the test file's own top-level hooks, so this afterEach checks first. Those logs are blamed on the next test, or lost after the last one.
• A log from afterAll: lost.
• A log that lands after the file's last test: lost. That's the CI-only symptom from Fail Jest tests that log unexpected console errors or warnings #595, so I expect this is why the PR uses an environment.

That's 4 of the 9 cases in console-guard.test.ts.

If the earlier version used setupFilesAfterEnv (the leftover jest.setup.ts and .eslintrc.js lines suggest it did), could you add a short "Considered and rejected" section to the description? It would say why the setup-file approach and jest-fail-on-console weren't enough: mainly logs after the last test and from cleanup hooks. That way the extra ~35 lines and the custom testEnvironment explain themselves to the next person who wonders why this isn't a setup file.

[alex-rawlings-yyc]

Rewrote the description to match the diff and added a "Considered and rejected" section. Took the run_start case in c07f430, with a fixture that warns at module scope ahead of two passing tests: the file fails and both tests pass. One correction for that section: jest-fail-on-console doesn't have quite the same gaps as a hand-written afterEach. Its afterEach puts the original console back, so a log from a file's own afterEach, from afterAll, or after the last test prints but fails nothing. It checks module-scope logs in a beforeAll, so they fail every test in the file. Rebasing onto main also turned up Redux Toolkit's dev-only "invariant check took Nms" warning, which fires at random under full-suite load; the guard now lets that one message through.