Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(adapter): don't duplicate multiline failure messages #294

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mitchellwills
Copy link

The current code to handle removing error messages for formatting does not
properly handle multiline error messages and results in the message being
repeated at the top of the stack trace. The code only searched for the message
in the first stack line, which meant it would never be found because the message
was multiple lines, which meant that the message would be pre-pended by the
message not in stack logic.

The existing multiline test actually triggers this behavior, but because it used
toMatch it only verified that the message was included, not that it only existed
once.

devoto13
devoto13 previously approved these changes Apr 20, 2022
Copy link
Collaborator

@devoto13 devoto13 left a comment

Choose a reason for hiding this comment

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

I'm not sufficiently familiar with the topic to say anything about potential implications in different browsers, but the code and reasoning look good to me. I think we can get this in as it addresses the actual bug and we can deal with any potential regressions if such arise.

The current code to handle removing error messages for formatting does not
properly handle multiline error messages and results in the message being
repeated at the top of the stack trace. The code only searched for the message
in the first stack line, which meant it would never be found because the message
was multiple lines, which meant that the message would be pre-pended by the
message not in stack logic.

The existing multiline test actually triggers this behavior, but because it used
toMatch it only verified that the message was included, not that it only existed
once.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants