err.showDiff = true breaks string diffs

[felixrabe]

Observed:

2)  should compare strings (raw):

    Error: bad stuff
    + expected - actual

    +"a\nb\nc\nd"
    -"a\nb\n"

Wanted:

2)  should compare strings (raw):

    Error: bad stuff
    + expected - actual

     a
     b
    +c
    +d

Example code: (see https://github.com/felixrabe/mocha-chai-string-diff/blob/master/test/test.js for the complete example code, run with mocha)

it('should compare strings (raw)', function() {
  var err = new Error('bad stuff');
  err.expected = 'a\nb\nc\nd';
  err.actual = 'a\nb\n';
  err.showDiff = true;  // breaks string diffs
  // err.showDiff = false;  // string diffs are fine
  throw err;
});

With Chai:

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

it('should compare strings (chai)', function() {
  try {
    expect('a\nb\n').to.equal('a\nb\nc\nd');
  } catch (err) {
    // string diffs broken because err.showDiff === true
    // err.showDiff = false;  // string diffs are fine
    throw err;
  }
});

If you uncomment the // err.showDiff = false; lines (ie. if you set showDiff to false), string diffs show up fine.

[boneskull]

@felixrabe Did you try the inlineDiffs option?

[rstacruz]

I added a bunch of comments that are related to this on #711.

[rstacruz]

Here are more details on behalf of @felixrabe: https://gist.github.com/rstacruz/4f7e5c11b5238056cf79

[felixrabe]

Thanks @rstacruz

[felixrabe]

@boneskull The thing is that I didn't want inline diffs, I wanted line-by-line diffs. I observed inlineDiffs behaviour even though I switched it off. (See original comment: "observed" vs "wanted".)

[felixrabe]

Hm, wait, bogus comment, will retest.

[notslang]

I'm getting this same issue - by default diffs seem to be off, and setting showDiff = true on the exception and rethrowing it doesn't enable the diffs. However, setting showDiff = false on the exception and rethrowing it does make diffs work.

I also noticed that there was a commit related to this (1f9c1bb), so I tried installing mocha from master, but now none of the above settings make the diffs show up.

[sebmck]

It's because Line 182-183 of lib/reporters/base.js calls util.stringify which runs JSON.stringify on the string escaping all newlines. This effectively removes all multiline string diff behaviour.

[dasilvacontin]

We should have tests asserting diff output using different options.

[matz3]

String diff were working with an exception from the node assert module in v1.21.4 but not anymore with v1.21.5 (and also not with v2.0.1).
As @slang800 already referenced, I guess 1f9c1bb is breaking this behavior.

Wouldn't if be better to change 1f9c1bb#diff-5c8a5788ecd4b4af2a29bc27ac4ae986R188 to

if (err.showDiff !== false ...

instead of?

if (err.showDiff ...

[domenic]

Ping. Going to have to go back to Mocha 1.21.4 until this is fixed.

[notslang]

tried out v2.1.0 & this is still broken

[a8m]

Fixed in v2.2.3

[caseywebdev]

Is this really fixed? If anything it looks like the specs were changed to expect the wrong thing. Is anyone else still having this issue in 2.2.4?

This is my temporary fix...

var utils = require('mocha/lib/utils');
var stringify = utils.stringify;
utils.stringify = function (value) {
  return typeof value === 'string' ? value : stringify(value);
};

[a8m]

Is this really fixed?

yes, you can play it yourself @caseywebdev
thx

[caseywebdev]

I don't think it's working correctly (v2.2.4).

Given this test

it('displays line diffs', function () {
  var err = new Error('The Error Message');
  err.expected = 'foo\nbar\nbuz\nbaz';
  err.actual = 'foo\nbar\n';
  throw err;
});

the reporter yields

  1) displays line diffs

  0 passing (11ms)
  1 failing

  1)  displays line diffs:

      Error: The Error Message
      + expected - actual

      +"foo\nbar\nbuz\nbaz"
      -"foo\nbar\n"

      at Context.<anonymous> (test/test.js:2:13)

The line diffs are far more useful than the diff of the JSON.stringified string...

Running the same test with my patch above yields

 1) displays line diffs

  0 passing (11ms)
  1 failing

  1)  displays line diffs:

      Error: The Error Message
      + expected - actual

       foo
       bar
      +buz
      +baz

      at Context.<anonymous> (test/test.js:7:13)

[a8m]

I don't get what's the problem, you want this string as a "raw data".
In your patch, the diff viewer ignored \n and "

[domenic]

No we dont, we want it back like it was in 1.21.

[caseywebdev]

Look at the original post by @felixrabe. He clearly wants the line diff because it's far more readable.

[a8m]

Iet me test 1.2.1

He clearly wants the line diff because it's far more readable.

my bad @caseywebdev
I'm on it.

[danielstjules]

For those who come across this issue, it has been fixed, and regression tests exist for the behavior now :)

$ cat test.js
it('should compare strings (raw)', function() {
  var err = new Error('bad stuff');
  err.expected = 'a\nb\nc\nd';
  err.actual = 'a\nb\n';
  err.showDiff = true;
  throw err;
});

it('displays line diffs', function () {
  var err = new Error('The Error Message');
  err.expected = 'foo\nbar\nbuz\nbaz';
  err.actual = 'foo\nbar\n';
  throw err;
});

$ mocha --version
2.2.5

$ mocha test.js


  1) should compare strings (raw)
  2) displays line diffs

  0 passing (12ms)
  2 failing

  1)  should compare strings (raw):

      Error: bad stuff
      + expected - actual

       a
       b
      +c
      +d

      at Context.<anonymous> (test.js:2:13)

  2)  displays line diffs:

      Error: The Error Message
      + expected - actual

       foo
       bar
      +buz
      +baz

      at Context.<anonymous> (test.js:10:13)