🐛 Bug: Comparing buffers hangs when computing the diff report

[astorije]

I am having the very same issue as #1433 mocha 2.2.1 and chai 2.1.1. When 2 buffers are not equal, the diff computation hangs and CPU gets crazy.

Any chance of a regression bug here?

Sample code to reproduce this:

var chai = require('chai');
var expect = chai.expect;

// chai.config.showDiff = false; // Without this line, the diff generation hangs

describe('Testing buffer equality', function () {
  it('should not take foreveeeeeer', function () {
    var file1 = Fs.readFileSync('/tmp/file1');
    var file2 = Fs.readFileSync('/tmp/file2');
    expect(file1).to.deep.equal(file2);
  });
});

At this point of writing, I am unclear if this issue comes from mocha or from chai, though...

[a8m]

I tested it right now(with a very small files), and it work pretty good. here's the output:

 Testing buffer equality
    1) should not take foreveeeeeer

  0 passing (6ms)
  1 failing

  1) Testing buffer equality should not take foreveeeeeer:

      AssertionError: expected <Buffer 66 6f 6f 0a> to deeply equal <Buffer 62 61 72 0a>
      + expected - actual

       [
      +  98
      +  97
      +  114
      -  102
      -  111
      -  111
         10
       ]

Are you dealing with large files(=buffers) @astorije ?

[astorije]

Hi @a8m, thanks for answering!

Both files are PNG images, one is 20K, the other one is 40K, so too large to paste in Google Translate, but not large enough to expect my Core i7 quad core to hang for 10 minutes... :-/

Could you try with similar images, maybe?

[a8m]

OK, I get the point. but how this related to mocha? Are you aware that the difference can be greater than ~20,000(in my example at least). I'm just curious, why do you want to show the difference between two chunks?

[danielstjules]

@astorije Rather than comparing such large buffers, can you compare their checksums using something like crypto.createHash('sha1')? It'll be significantly faster.

[a8m]

using something like crypto.createHash('sha1')

Good point.

[astorije]

Hi guys,

I don't want to show the difference, but it's the default behavior when running the simple test case I pasted above.

Comparing a checksum is doable, but that would obscure a tiny bit the tests, and I'd like to avoid that for simple object comparison.

Is there a way the code above can be used without displaying the diff (nor hiding it by default for all tests)?

[astorije]

@a8m, I see this as related to mocha because I wasn't expecting a short test case to completely hang. I would have expected either some sort of a timeout or the diff disabled on such cases. Does that make sense?

[a8m]

Is there a way the code above can be used without displaying the diff (nor hiding it by default for all tests)?

Yes, this is just a patch, but it should work.

try {
  expect(x).to.equal(y)
} catch(e) {
  e.showDiff = false
  throw e
}

[boneskull]

We should suppress Buffer diffs longer than n lines.

[a8m]

We should suppress Buffer diffs longer than n bytes.

this is something that changes depending on the machine(the cpu, not the size), maybe timeout could work better ?

[astorije]

@a8m, a timeout seems like a good way to go, but not the same timeout that is used for each assertion/test. The rationale here is that you might want diffs that are long to compute to timeout after 2 or 5 seconds, while you would want tests that rely on HTTP requests to timeout after, say, 30s.

[boneskull]

this is something that changes depending on the machine(the cpu, not the size), maybe timeout could work better ?

I'm not sure. How helpful is a huge buffer diff?

[stale[bot]]

I am a bot that watches issues for inactivity. This issue hasn't had any recent activity, and I'm labeling it stale. In 14 days, if there are no further comments or activity, I will close this issue. Thanks for contributing to Mocha!

[JPMardelli]

Had a similar issue to this with chai-subset. The test itself didn't timeout, but when mocha tried to print the failure at the end, it failed to do so and completely stalled/halted. Mocha never terminated.

[thatsmydoing]

I can confirm this also happens when comparing a large object structure as well,

const { expect } = require('chai');

describe('Test', () => {
  it('should not hang', () => {
    let obj = {};
    for (let i = 0; i < 50; ++i) {
      obj = { obj, obj2: obj };
    }
    expect(obj).to.eql({});
  });
});

The provided workaround works but it would be nice if the default behavior would not just cause it to hang.

[udaybuie]

Hi are we looking to fix the issue, as i am also facing the same issue and not able to generate a diff report for my consolidated api diff regression suit. Would appreciate, if someone can pick up this. we can use https://www.npmjs.com/package/json-diff to the diff its quite fast, but not sure how its result can be published to mocha so test report(mochawesome) can show those diffs.