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

Limit the size of 'actual'/'expected' strings before generating a diff #4638

Merged
merged 2 commits into from
May 30, 2021

Conversation

juergba
Copy link
Contributor

@juergba juergba commented May 24, 2021

Description

Mocha prints its own diff on assertion errors by stringifying err.actual and err.expected, then passing both strings to jsdiff for generating a diff-patch. For big strings this synchronous process can take several minutes.

jsdiff has some known performance issues, so in mid-term we evtl. should evaluate a jsdiff alternative.
On the other hand it's not a good idea to calculate a diff for two huge strings, even with a faster algorithm. We should limit the size of the input strings, as eg. done by Node's assert as well.

Description of the Change

  • no change: stringify err.actual and err.expected
  • new: when too big, truncate the resulting strings and add a message ... Lines skipped

Applicable issues

closes #3675

@juergba juergba self-assigned this May 24, 2021
@juergba juergba added type: bug a defect, confirmed by a maintainer dependencies Pull requests that update a dependency file type: performance Performance improvements semver-major implementation requires increase of "major" version number; "breaking changes" labels May 24, 2021
@juergba juergba added this to the v9.0.0 milestone May 24, 2021
@juergba juergba requested a review from a team May 24, 2021 14:25
@juergba juergba merged commit a93d759 into master May 30, 2021
@juergba juergba deleted the juergba/diff branch May 30, 2021 16:29
@@ -164,6 +164,34 @@ describe('Base reporter', function() {
' \n actual expected\n \n a foobar inline diff\n '
);
});

it("should truncate overly long 'actual' ", function() {
var actual = '';

Choose a reason for hiding this comment

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

Hello, I'm trying to contribute to Mocha and before I do I'm trying to get idea of mocha's coding style. I want to know why you used 'var' instea of 'const' or 'let'. Thank you in advance.

@joshgoebel
Copy link

joshgoebel commented Oct 7, 2021

Can this be a feature flag please so it can be disabled? I was having NO speed issues before but not my test output is entirely broken (it doesn't show the actual diff at all) because it's a bit over 2000 characters. ☹️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file semver-major implementation requires increase of "major" version number; "breaking changes" type: bug a defect, confirmed by a maintainer type: performance Performance improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mocha failing when error message too big
3 participants