Skip to content

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

Open
mitchellwills wants to merge 2 commits intokarma-runner:masterfrom
mitchellwills:master
Open

fix(adapter): don't duplicate multiline failure messages#294
mitchellwills wants to merge 2 commits intokarma-runner:masterfrom
mitchellwills:master

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.

@XhmikosR XhmikosR requested review from devoto13 and jginsburgn April 19, 2022 05:11
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