Skip to content

Commit

Permalink
Be more general in stripping off stack frames to fix Firefox tests (#…
Browse files Browse the repository at this point in the history
…2425)

* Revert breaking change to stack-stripping regex

This regex was made more specific in an attempt to avoid incorrectly
stripping off parts of messages - we strip off anything after " at",
which means "to have been called at least once" becomes "to have been
called".

However, the new regex was too specific and made faulty assumptions
about stack traces - namely, that they'll always have parentheses after
the at. Firefox's stack traces, however, look like so:

func() at callFn@about:blank line 2 > injectedScript:47353:21

So, we could try to come up with a better regex that matches all
browsers that we test with, or perhaps add some sort of indicator when
we're appending the stack traces, but since this change is breaking the
tests for everyone right now, for expedience we'll just revert to how we
were stripping things before.

This means changing one of the asserts back to how it was before, where
it was asserting against a partial message due to the over-eager
stripping, so add a comment about that as well.

* Update with a more general regex

This should match any stack frame format that has at least one non-word,
non-space character in it, which should be a pretty safe assumption, and
holds for the three browsers we test in anyway.

This gets us back to not trimming messages improperly, while also not
breaking firefox, and I think is reasonable as far as regexes go.

This seems like a reasonable enough regex - the original fixed one was
overly complicated because it tried to remove an arbitrary number of
stack frames, which was unnecessary because we only append one.
  • Loading branch information
cincodenada authored Jan 25, 2022
1 parent 56b0612 commit 1d5ab86
Showing 1 changed file with 9 additions and 3 deletions.
12 changes: 9 additions & 3 deletions test/assert-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1438,9 +1438,15 @@ describe("assert", function () {
[].slice.call(arguments, 1)
);
} catch (e) {
// We sometimes append stack frames to the message and they
// make assertions messy, so strip those off here
return e.message.replace(/( at.*\(.*\)$)+/gm, "");
/* We sometimes append stack frames to the message and they
* make assertions messy, so strip those off here
*
* In the regex we assume that a stack frame will have at
* least one "special character" (not a word or space) and
* use that to make sure we don't strip off the end of
* legitimate messages that end with "at least once..."
*/
return e.message.replace(/ at.*?[^\w\s].*/g, "");
}
};
});
Expand Down

0 comments on commit 1d5ab86

Please sign in to comment.