Skip to content

[noop] Fail tests on unasserted recoverable errors#648

Closed
everettbu wants to merge 1 commit into
mainfrom
sebbie/03-02-_noop_fail_tests_on_unasserted_recoverable_errors
Closed

[noop] Fail tests on unasserted recoverable errors#648
everettbu wants to merge 1 commit into
mainfrom
sebbie/03-02-_noop_fail_tests_on_unasserted_recoverable_errors

Conversation

@everettbu

@everettbu everettbu commented Mar 3, 2026

Copy link
Copy Markdown

Mirror of facebook/react#35948
Original author: eps1lon


react-noop-renderer previously swallowed recoverable errors (from onRecoverableError). Now we pass those through to console.error so that we fail the test unless assertConsoleError

@everettbu everettbu added CLA Signed React Core Team Opened by a member of the React Core Team labels Mar 3, 2026
@everettbu
everettbu force-pushed the sebbie/03-02-_noop_fail_tests_on_unasserted_recoverable_errors branch 2 times, most recently from 0425201 to 4d351a7 Compare March 3, 2026 13:35
@everettbu
everettbu force-pushed the sebbie/03-02-_noop_fail_tests_on_unasserted_recoverable_errors branch from 4d351a7 to 1b61b6a Compare March 3, 2026 14:26
@everettbu
everettbu marked this pull request as ready for review March 3, 2026 14:26
@greptile-apps

greptile-apps Bot commented Mar 3, 2026

Copy link
Copy Markdown

Greptile Summary

This PR enables react-noop-renderer to surface recoverable errors (from onRecoverableError) via console.error instead of silently swallowing them. This ensures that tests using the noop renderer will fail if they trigger recoverable errors without explicitly asserting them via assertConsoleErrorDev, improving test reliability and error visibility.

  • createReactNoop.js: onRecoverableError now calls console.error(error) with Flow type annotations and appropriate eslint-disable comments
  • 3 test files updated: Added assertConsoleErrorDev assertions for the concurrent rendering recovery error message in all tests that trigger error recovery
  • useMemoCache-test.js: Replaced manual spyOnDev(console, 'error') spy with proper assertConsoleErrorDev assertion, and consolidated internal-test-utils imports

Confidence Score: 5/5

  • This PR is safe to merge — it improves test strictness without changing runtime behavior.
  • The changes are minimal, well-scoped, and follow established patterns. The core change (enabling console.error in onRecoverableError) only affects the test renderer, and all affected tests have been updated with proper assertions. No runtime code is modified.
  • No files require special attention.

Important Files Changed

Filename Overview
packages/react-noop-renderer/src/createReactNoop.js Core change: onRecoverableError now logs to console.error instead of silently swallowing errors, enabling test infrastructure to catch unasserted recoverable errors. Added Flow type annotations.
packages/react-reconciler/src/tests/ReactIncrementalErrorHandling-test.internal.js Added 4 assertConsoleErrorDev calls to tests that trigger concurrent rendering error recovery, properly asserting the now-surfaced recoverable error messages.
packages/react-reconciler/src/tests/ReactIncrementalErrorReplay-test.js Added assertConsoleErrorDev import and assertion for the recoverable error in the error replay test.
packages/react-reconciler/src/tests/useMemoCache-test.js Replaced manual spyOnDev(console, 'error') with proper assertConsoleErrorDev assertion and consolidated internal-test-utils imports into a single destructured require.

Last reviewed commit: 1b61b6a

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@everettbu

Copy link
Copy Markdown
Author

Upstream PR was closed or merged. Code is synced via branch mirror.

@everettbu everettbu closed this Mar 4, 2026
@everettbu
everettbu deleted the sebbie/03-02-_noop_fail_tests_on_unasserted_recoverable_errors branch March 4, 2026 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed React Core Team Opened by a member of the React Core Team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants