[tests] Require exact error messages in assertConsole helpers#35497
[tests] Require exact error messages in assertConsole helpers#35497rickhanlonii merged 16 commits intofacebook:mainfrom
Conversation
Update test assertions to include the complete component stack trace rather than partial stacks. This ensures tests validate the full owner stack as it would appear in development.
|
Comparing: 5aec1b2...e61f352 Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: (No significant changes) |
Add two new placeholders for console error assertions: 1. [Server] - expands to the ANSI escape sequence for server badge Instead of: '\u001b[0m\u001b[7m Server \u001b[0mError: message' Write: '[Server] Error: message' 2. \n in <stack> - matches JavaScript Error stack traces Instead of matching the full error stack manually Write: 'Error: message\n in <stack>' The error stack placeholder validates that it's only used for actual Error stack traces (messages starting with "Error:" that have file:line:col frames), not for React component stacks. Also adds validation to catch misuse of these placeholders and provides helpful error messages guiding developers to the correct usage.
1a33282 to
eb04af7
Compare
| // Error stack traces start with "Error:" and contain "at" frames with file paths | ||
| // Component stacks contain "in ComponentName" patterns | ||
| // This helps validate that \n in <stack> is used correctly | ||
| const isLikelyAnErrorStackTrace = message => | ||
| typeof message === 'string' && | ||
| message.includes('Error:') && |
There was a problem hiding this comment.
Your implementation already accounts for e.g. TypeError() having TypeError: bla when stringified but the comments still says "start". Should prob update the comment to call out that includes is used to handle other kind of errors.
Also the input here is the stringified error instance not just the message. .message doesn't include the error name.
There was a problem hiding this comment.
Good call, I noticed this while vibe coding too and meant to get back to it.
There was a problem hiding this comment.
Updated the comment - The message does include the error name (and stack trace). I was surprised about this too.
There was a problem hiding this comment.
I updated the tests to log an error, showing the behavior (and confirmed with logging).
There was a problem hiding this comment.
Updated the comment - The message does include the error name (and stack trace). I was surprised about this too.
That sounds wrong. Maybe you're reading .stack instead of .message? .stack would include name, message, and stacktrace.
There was a problem hiding this comment.
If it was wrong, then this wouldn't work right? Like this function would noop, but if you noop it then the tests fail. Maybe there's a shim or console override adding it?
…ok#35497) Requires full error message in assert helpers. Some of the error messages we asset on add a native javascript stack trace, which would be a pain to add to the messages and maintain. This PR allows you to just add `\n in <stack>` placeholder to the error message to denote a native stack trace is present in the message. --- Note: i vibe coded this so it was a pain to backtrack this to break this into a stack, I tried and gave up, sorry. DiffTrain build for [3e1abcc](facebook@3e1abcc)
…ok#35497) Requires full error message in assert helpers. Some of the error messages we asset on add a native javascript stack trace, which would be a pain to add to the messages and maintain. This PR allows you to just add `\n in <stack>` placeholder to the error message to denote a native stack trace is present in the message. --- Note: i vibe coded this so it was a pain to backtrack this to break this into a stack, I tried and gave up, sorry. DiffTrain build for [3e1abcc](facebook@3e1abcc)
Stacked on #35497 ----- Now that the assert helpers require a component stack, we don't need the `{withoutStack: true}` option.
Requires full error message in assert helpers.
Some of the error messages we asset on add a native javascript stack trace, which would be a pain to add to the messages and maintain. This PR allows you to just add
\n in <stack>placeholder to the error message to denote a native stack trace is present in the message.Note: i vibe coded this so it was a pain to backtrack this to break this into a stack, I tried and gave up, sorry.