Chai: Ability to show only changes and deletions in diffs, not additions

[Mingun]

I use chai assertion library and chai-like library for the partial comparing of objects: I am interested in existence and compliance only of those properties that are listed in an expected object. Existence of additional properties in an actual object permissibly. Unfortunately, when diff is created it is not considered in any way and those properties, existence which for the test it is indifferent get to diff. It would be desirable to have a way to suppress their generation in a diff. Example (runned with mocha):

'use strict';

let chai = require('chai');
let expect = chai.expect;
chai.use(require('chai-like'));

describe("diff test", function() {
  let AST = {
    type: 'grammar',
    rules: [
      { type: 'rule', name: 'rule1' },
      { type: 'rule', name: 'rule2' },
    ],
    // it is a lot more other properties
  };
  it("chai-like", function() {
    expect(AST).like({
      rules: [
        { name: 'rule' },
        { name: 'rule2' }
      ]
    });
  });
});

This is current actual output of mocha which is too large in some cases and contains unnecessary details

  1) diff test
       chai-like:

      AssertionError: expected { Object (type, rules) } to be like { Object (rules) }
      + expected - actual

       {
         "rules": [
           {
      -      "name": "rule1"
      -      "type": "rule"
      +      "name": "rule"
           }
           {
             "name": "rule2"
      -      "type": "rule"
           }
         ]
      -  "type": "grammar"
       }

      at Context.<anonymous> (test\test.js:24:17)

But I want that diff looked here so:

 {
   "rules": [
     {
-      "name": "rule1"
+      "name": "rule"
     }
   ]
 }

Now there is no opportunity to show diff in that look in what it is required as AssertionError has no signs that it is required. Mocha developers advised to create at first the task here before they are able to support this functionality (https://github.com/mochajs/mocha/issues/3193).

I suppose add to an AssertionError object new property which could control such behavior of diff.

If the idea is supported, I can make PRs for its implementation in all necessary libraries.

[meeber]

Just thinking outloud here: One option would be a modification to this line in chai-like so that instead of passing in the full object, pass in a new object that only includes the properties being checked in object based on expected.

[Mingun]

Unfotinatly this is not an option because expected used not only for creating diff but also for message building

Unfortunately it is not option as expected is used not only for creation of diff, but also message building. Besides, copying of an object -- extremely difficult and potentially the wasteful task, is much simpler to correct a problem where it arises - in diff output code.

[meeber]

@Mingun The AssertionError constructor already allows an arbitrary number of properties to be added onto the AssertionError object. So, even without first class support in Chai, a plugin could implement a suppressDiffAdditions option by passing along a custom property in the AssertionError object to the test runner in charge of diffing. To do this, the plugin would need to construct the AssertionError manually instead of using the .assert helper function. Doing so is inconvenient but not extreme.

Of course, first class support could be added in Chai by updating the .assert helper function to accept another parameter that controls whether or not a suppressDiffAdditions flag should be set in the AssertionError. But the issue of adding first class support to Chai seems secondary to the issue of whether or not test runners will support it. The test runner would need to do all of the heavy lifting for such a feature.

@boneskull what are your thoughts on this?

[Mingun]

Yes, it can be option, many thanks! Maybe it should be added the last optional properties argument to assert from which all keys will be in addition transferred to AssertionError? It will allow to use assert and also to pass any other parameters which can be required in the future. It is unlikely them will be much, and such decision both is backward-compatible, and it is expanded, and moderately simple.

[boneskull]

@meeber My thoughts are that I don't want to add more random properties that Mocha does stuff with.

Mocha's handling of diffs and whatnot can be avoided by an Error never having the showDiff property set to true, to begin with. All diff information can be stuffed into the message prop itself, which is likely a better way to handle it, since it provides more control and customization to assertion libraries and their users.

There's really no end to Mocha adding features to handle properties on Error objects. It can't reasonably be expected to support every use case.

@mingun You should be able to do what you want within the context of an assertion library, but I can't tell you whether or not Chai allows this level of customization; you would need to remove the showDiff, expected and actual properties from the Error, then create your own diff and append it to the message property. This is how unexpected creates customized diffs.

[keithamus]

@boneskull how would you feel about chai's AssertionError objects having their own diff property - which would use Diff notation, that mocha could format and colorise as necessary?

[boneskull]

@keithamus Ehh, I don't see that as necessarily any better than the current state of things. Even if Chai did that, Mocha would still need to retain its own diffing code, because other libs don't.

[boneskull]

either way, assertion libraries can use the built-in mocha capabilities, or just go their own way, which I think is fine.

[meeber]

If I understand what has been said so far, currently there's two options: 1) an Assertion library can delegate both the diffing of the actual and expected to Mocha, as well as the formatting of that diff, by passing an AssertionError with the expected, actual, and showDiff properties set; or, 2) an Assertion library can perform both the diffing of the actual and expected itself, as well as the formatting of that diff, by not passing those properties to Mocha and instead appending the message property with the formatted diff.

The way I interpret @keithamus' last question is asking if it may be possible to support a third option where the assertion library performs the diffing of the actual and expected, and then delegates formatting of that diff to Mocha by passing it an AssertionError with a diff property. And I assume the purpose of this approach is to give Assertion libraries an opportunity to customize the diff while still being able to leverage Mocha's diff formatting capabilities.

Did I get all of that right or am I misunderstanding how it works currently or what's being proposed?

[boneskull]

I'm probably misunderstanding something...

[keithamus]

Well I've been looking into chai supporting a diff property that runners could consume - as chai is in the best place to know about what kind of diff the user is after - based on the assertion. It perhaps wouldn't be helpful to mocha, per se, until all assertion libraries sent diffs to mocha

[meeber]

@boneskull Not necessarily :D. It could be me misunderstanding; I don't have a firm mental picture on the diffing stuff yet.

@keithamus Was my last post accurate? If not, can you correct my misunderstanding?

[keithamus]

@meeber your explanation was correct, yes.

[Mingun]

@boneskull

then create your own diff and append it to the message property

Of course, it is possible, but how in that case to guarantee uniformity of diff? I like the idea with @keithamus with diff property in AssertionError object. @meeber very precisely described a situation. You are interested in that its support appeared in mocha? If so, then I can begin to work on the appropriate PRs to chai and mocha and probable other assertion libraries.

There's really no end to Mocha adding features to handle properties on Error objects. It can't reasonably be expected to support every use case.

Actually there only two use-cases: full equality and similarity so it is enough to support only these two cases. diff with full equality is already supported, there was a case with similarity. Originally I stated a sentence for a solution with the minimum expenses, but the decision with diff property more general and I agree, more preferable.

[keithamus]

If mocha could agree to a standard of the following I would be happy to begin work on the changes for chai:

if the `showDiff` property exists in assertionerror and is `true`
  if property 'diff' exists in assertionerror and is a string
    parse the contents as `Unified Diff Format`
  else property `expected` and `equal` exist in assertionerrror
    use the existing algorithm to parse `expected` and `actual`

In other words a string diff property takes precedent, then expected/actual, then no diff.

For Unifieid Diff Format I mean a format that comes from diff(1) with the -u flag. It looks like this:

--- <optional filename> <optional ISO date>
+++ <optional filename> <optional ISO date>
@@ -1,3 +1,3 @@ <optional block header>
-removed text
+added text
 unchanged text

(The ---, +++ lines may be omitted entirely)

How mocha interprets that diff is up to their implementation (I would imagine terminals could colorise the ouput with ansi escape codes, while html reporter might use CSS).

[boneskull]

With a unified diff, we will lose context; it's for text files. expected and actual give us much more information, since we're talking about JavaScript objects and whatnot here.

But this would solve the use case by adding some flag to Chai which suppresses the additions from the diff and then Mocha has to add a special hook for it? (It's "special" because Chai supporting such a property is not a standard :wink:)

If the idea is for Mocha to simply print the contents of the diff property (cause I don't really want to try to parse it further), we're really no better off than suppressing the whole thing and appending the diff to the message property string...

[boneskull]

In other words, this is what I said originally; either an assertion lib can use's Mocha stuff, or it can do whatever other thing it wants to and just use message. @keithamus's proposal is a "other thing".

[keithamus]

Just before I respond to your points, I'll make an additional point. We get a regular stream of issues about mocha's diffing, and how it relates to chais error messages, and how they can become better representations and more consistent for each failure. We need to work on our error messages to make them better, so it makes sense for us to also own the diff output.

expected and actual give us much more information

As I allude to over in https://github.com/js-reporters/js-reporters/issues/106; the assertion library (chai) can provide much nuanced context about the expected and actual properties than the test runner (mocha) ever can. You say expected and actual give _us_ (as in mocha) much more info, but I disagree. I'll quote my comments from that issue here:

Reason 1. The assertion object already generated a message. The likelyhood is this message has inspected and serialised the actual and expected properties in some way (for e.g. saying expected "foo" to equal "bar"). We can therefore assert that the assertion object has a mechanism for pretty-printing objects. It is likely that the best place to add pretty-printing with diffs, is therefore, in the same formatting library the assertion functions already consume. I know this to be the case for chai, I assume it to be right for others.

Reason 2. The assertion call itself knows about the intent behind the assertion more than the object can hint at. It is reasonably easy to imagine up an assertion where the expected and actual properties are like-for-like, but the assertion is failing for other means. For example expect({foo: 1}).to.have.non.enumerable.property('foo', 1). The assertion here knows that the user is expecting the given object to have a non enumerable property foo equal to 1. The problem is, it will fail the assertion with an object that has the expected property as 1 and the actual property as 1 - so both equal from the perspective of the reporter. An alternative for the assertion object could be to provide the property descriptors as expected and actual properties - but this could confuse things further (what if they pass that assertion but differ because the object has a getter). The only way for the assertion object to reasonably inform the user of the subtle failure is with the message property - or - a more explicit diff property which could provide a higher fidelity output than a message, but still cross the boundary to the reporter without losing fidelity.

To give a concrete example, currently Buffers look like Arrays with mocha's diff. I'm hoping with work Im doing around Chai's inspection engine we can produce diffs which get much closer to what the user cares about, so they can see what's failing much easier. Sure, mocha could pick this up, but chai already has to have this logic for the .message property, but it'd be nice to have more detailed output.

supporting such a property is not a standard 😉

Which standard is this? We mostly adhere to CommonJS Unit Testing 1.0 but this doesn't cover anything about reporter errors, so properties like showDiff aren't considered standard (but are still consumed my mocha). Standards are less important than interoperability here - which is why I'd like to see us work to a solution for this that _is_ interoperable.

If the idea is for Mocha to simply print the contents of the diff property (cause I don't really want to try to parse it further)...

That would not be what I want. I would want mocha to parse the contents of a diff and represent it in a way which is useful for the chosen reporter. Taking the diff and colorising it would be a useful step.

appending the diff to the _message_ property string

We definitely _could_ do this, but ultimately it's not that usable. The reason an explicit diff property would be useful over simply tacking it onto the message is precisely that you could output it in a higher fidelity format (like adding colours).

Ultimately if we (chai) can't get diff support, I'll probably push for making showDiff always false, and adding our diff to the message property.

[Mingun]

@keithamus I agree with @boneskull that diff must not be just text, but shall have a machine-readable format for simplification of its processing. After googling it seems to me, the most optimum will use the JSON Patch format (RFC 6902). The patch shall be provided as a difference between the canonized representations of objects (properties are sorted, circular references are represented by special value). The patch shall be provided as the list of operations on the expected to conversion it to actual (the current behavior of mocha does on the contrary that in my opinion a little illogically -- for example, in the absence of mandatory property in an object it is logical to see in results that it was deleted -- then it is clear that a problem that the property was not added to an object).

[keithamus]

@Mingun I don't understand how JSON patch could work. What would be the JSON patch for expect(1).to.equal('1') - what steps can you take to make 1 be '1'? With a unified diff text representation, I can simply log the entire object as added/removed:

@@ -1,1 +1,1 @@
- 1
+ '1'

A more complex example, what would be the JSON patch for expect(Buffer.from([1,2,3])).to.eql(new Set(['a', 'b', 'c']))?

[boneskull]

@keithamus

supporting such a property is not a standard 😉
Which standard is this? We mostly adhere to CommonJS Unit Testing 1.0 but this doesn't cover anything about reporter errors, so properties like showDiff aren't considered standard (but are still consumed my mocha). Standards are less important than interoperability here - which is why I'd like to see us work to a solution for this that is interoperable.

I think I was replying to you here, so I probably just misunderstood:

If mocha could agree to a standard of the following [etc...]

I'll revisit "interop" below...

---

If it seems like I'm being stubborn, I just want to be clear:

I'm trying to avoid adding and maintaining support for "random" properties coming out of 3p libraries. As originally proposed, a flag to suppress portions of a diff, requested by a single individual (sorry @Mingun; don't take it personally), isn't something I'm willing to entertain.

...just so you know where I'm coming from.

We definitely could do this, but ultimately it's not that usable. The reason an explicit diff property would be useful over simply tacking it onto the message is precisely that you could output it in a higher fidelity format (like adding colours).
Ultimately if we (chai) can't get diff support, I'll probably push for making showDiff always false, and adding our diff to the message property.

So, a couple things:

1. You can send colors. Mocha won't prevent them from being displayed. However, you should probably check tty.isatty
2. unexpected does just this--it appends a colorized diff to message, because Mocha's built-in capabilities weren't going to cut it for what they were trying to do.

Granted, it's kind of lame to just attach the diff to message, because it handicaps what the test framework can do with the information. However, appending to message is probably the best solution for Chai in the short-term, because it doesn't require Mocha to change anything, and will be backwards-compatible.

INTEROP

Interop is tricky, because we're not just talking about Chai and Mocha, we're talking about Chai and n other assertion libraries and m other testing frameworks. The chances we'll get strong cooperation--especially in a timely manner--are near-nil (e.g. the non-adoption of the js-reporters spec, but it could still have some life in it).

That being said, we could certainly ask around, but I would prefer action over consensus. As the respectively most popular testing framework and assertion lib, we have a chance to lead "by example" or otherwise bilaterally throw our weight around.

I don't claim to know what other things Chai's users want out of diffs--besides what @mingun has proposed. I do know some requests Mocha would have of these libraries, however!

Please let me know what you want to do here. You can just use message, or we can talk more in general about how we're like the libs to work together, and see if there are any other opinions out there.

[keithamus]

FWIW here is an (incomplete) list of issues we have relating to diffs and Mocha: https://github.com/chaijs/chai/issues/246, https://github.com/chaijs/chai/issues/249, https://github.com/chaijs/chai/issues/270, https://github.com/chaijs/chai/issues/304, https://github.com/chaijs/chai/issues/363, https://github.com/chaijs/chai/issues/469, https://github.com/chaijs/chai/issues/654, https://github.com/chaijs/chai/issues/678, https://github.com/chaijs/chai/issues/703, https://github.com/chaijs/chai/issues/1055, https://github.com/chaijs/chai/issues/1060, https://github.com/chaijs/chai/issues/1103

You can see these issues have been surfaced with a fairly regular cadence since 2014 which is somewhat shortly after when mocha started supporting expected and actual.

We can see the same thing in Mocha's issue tracker: https://github.com/mochajs/mocha/issues/1832, https://github.com/mochajs/mocha/issues/1348, https://github.com/mochajs/mocha/issues/1071 ... I won't go on, you know enough about mocha's issues.

All of these issues are roughly duplicates of eachother - the general theme is that the error output isn't sufficient.

requested by a single individual

FWIW @Mingun is looking for the same answer all the above issues are. I'm derailing this issue with my own agenda, which is something I've been perculating on for a long time. I've been hoping to add diff for a long time, not just because of this recently filed issue.

interop

That is _exactly_ what I'm after. Given mocha+chai is the most used combo, like you say, we could easily throw a bit of weight around and end up with everyone happy. IMO this is where the diff property would help. It would not break backward compat, and libraries could simply opt into it where they want to. But maybe I'm just flogging a dead horse now, so let's restart.

---

Problem

Both mocha and chai need to improve diff output, to stop the influx off issues we both receive. How do we do this in an interoperable way?

Requirements (chai)

• We _have_ to inspect objects and convert them to strings for our .message property (we call this lib loupe, it is going through a huge rewrite currently)
• Future versions of loupe will use object-diffing as a heuristic for providing concise and effective .message strings.
• loupes algorithms can be used to represent the differences between two objects, in _some_ format, object or string.

What are Mocha's requirements for this interop here?

[Mingun]

@keithamus 1 and '1' are both valid JSON values, so in you examples diff will be look like:

{ op: "replace", path: [], oldValue: 1, newValue: "1" }
{ op: "replace", path: [], oldValue: <ref to Buffer.from([1,2,3])>, newValue: <ref to new Set(['a', 'b', 'c'])> }

It is not necessary to consider JSON Patch as a direct guide to action, however the resultant format can be its superset. For example, it is in addition possible to support a possibility of representation of diff of strings/Buffer/Set/etc. collections. This format is not obliged to be serialized in JSON, it may contain references to any objects.

---

Studying the list of diff libraries, I managed to select 2 approaches in representation of diff:

1. Representation in the form conceptually described in JSON Patch -- the list of value changes on some JSON Pointer paths
2. Representation in the form of an object which each key describes what happened with it. For example from start message (diff actual AST from unnamed expected):

// patch, that if applied to expected produce actual
diff(expected, actual) ->
// changed all object
{
  // added key "type" with value "grammar"
  type: { op: 'add', value: 'grammar' },
  // changed key "rules"
  rules: {
    // changed key 0
    0: {
      // added key "type" with value "rule"
      type: { op: 'add', value: 'rule' },
      // value of the key "name" changed from "rule" to "rule1"
      name: { op: 'replace', oldValue: 'rule', newValue: 'rule1', diff: [...string diff...] },
    },
    // changed key 1
    1: {
      // added key "type" with value "rule"
      type: { op: 'add', value: 'rule' },
    },
  },
}

Maybe second approach will be better. Each object represent a changeset. If op key not set, then it just represent changeset of the inner object. op: 'add' adds key to object, op: 'remove' removed key and op: 'replace' changes value of the key. In last case for strings/arrays/other collections there may be diff property with structured diff of the values. Maybe even only this property

[keithamus]

I'm not completely averse to implementing JSON Diff is thats how we want to go, but it looks more complex to generate and consume than a unified diff.

[boneskull]

brain dump follows

Where Do I File A Bug?

What sucks for users, I've noticed, is that they don't know which lib is responsible for the diff. Is it Chai? Is it Mocha? Sometimes we don't even know!

I'm open to suggestions on how to attack that.

Baseline Value Representation

What also sucks--and could suck more, if we don't do this right--is inconsistent diffs from one lib to the next.

How to represent a numeric primitive is not especially controversial. But ...

• How to represent a Number instance? What about String?
• How do we represent a value that doesn't exist, e.g. undefined or void 0?
• How about null?
• How do we diff Date objects?
• Buffer objects? ArrayBuffer objects? SharedArrayBuffer objects (j/k)?
• Promise objects? Non-native Promise objects?
• In what way does this output change if we do deep object comparison?
• When is deep object comparison unnecessary? What about if the "types" differ?
• Any considerations for environments not fully supporting ES2015+?
• How do we treat Arrays vs array-like objects? What's considered an array-like object?
• What about Map, and Set?
• And WeakMap and WeakSet, both of which cannot be enumerated?

• What about a strict equality check against two "equivalent" objects (e.g. {foo: 'bar'} is not the same object as {foo: 'bar'}, but it's equivalent)?

  Mocha has its own ideas about many of these, but not necessarily the "best" or "most correct" ones. Some of these may be best left out of the "core"--and/or we can choose to delay decisions until they become necessary.

  • In short, getting on the same page in terms of a subset of value representation is needed from a UX standpoint.
  • We also need to define the "general" case, whether that's JSON diffs or whatever.
  • We may be able to consume the same base library for this.

Extensibility

• ...but there's likely more than one way to represent any of the given types I've mentioned above.
  Different kinds of diffs--not just object comparison--may (?) need support, such as a standard unified or inline output from diff.

• A lib or plugin author should be able to write their own custom implementation to diff objects of a certain "type". This can be defined by instanceof, or type, or a custom comparison function.

• Going a step further, end users should be able to leverage a valueOf()-like-function to tailor the diffs to their liking, so you don't need to do a Magic Plugin Dance to get that diff for your MonkeyButt class looking the way you want.

Metadata, Statistics

One thing Mocha can't do, because of a lack of a built-in assertion library, is set an expectation about how many assertions were expected to be made. Mocha doesn't know what an "assertion" is. Some may find this capability more useful than others, but it is another tool to avoid false positives.

But I don't know if the user should touch the assertion lib API or Mocha's API to use it.

(This is also not currently possible from within a standalone assertion framework, because it doesn't know when a "test" or "suite" finishes. Or what a "test" or "suite" is.)

Open to ideas. First thing that comes to mind is some sort of EventEmitter bus.

If there are numbers that Chai keeps that may be useful to Mocha, let's expose those.

Backwards Compat

Mocha must continue to work fine with other assertion libraries. That said, Mocha's diff output will necessarily change, since I don't want to maintain two separate algorithms for this.

Modules

• Given that currently an assertion library can create and display the entire diff without any assistance from Mocha short of a console.log(message), I'm not sure what Chai expects Mocha to implement. @keithamus mentioned "colors", etc., but this is not something Mocha must do because Chai cannot. What I'm looking for is to understand:

  • What (relevant) responsibilities Chai does not want
  • What (relevant) responsibilities Chai wants to delegate to another module

• As mentioned earlier, Mocha & Chai could consume the same module to generate diffs.

  • Could the "diff module" be swapped out?
  • Are "diff plugins" reasonable? Are they plugins to the "diff module"?
  • A separate module & plugins could make it more difficult for end users b/c they really won't know where to file an issue

Justification

If we're going to disrupt users by screwing with their diffs, the new diffs should be better than the old ones.

They should be so freaking awesome that users of other frameworks & libs create issues demanding adoption. :wink:

[Mingun]

I ask all interested to review https://github.com/mochajs/mocha/pull/3251

[keithamus]

This discussion was hugely productive and it's made us think a lot about what we can implement in chai 5! We're probably going to support diffs in chai 5, we've added this to our project board over on https://github.com/chaijs/chai/projects/2. I'll close this issue because I think it's taken its logical conclusion, and nothing is specifically actionable here.