search

qunitjs / js-reporters

📋 Common Reporter Interface (CRI) for JavaScript testing frameworks.
MIT License
60 stars 18 forks source link

Consider `diff` property on assertion object #106

Closed keithamus closed 2 years ago

keithamus commented 6 years ago

While it is generally well considered to have the expected and actual objects attached to the assertion object. They're useful because they allow reporters to print some kind of pretty-formatted diff to allow the developer to more easily reason about the failures in a test. I believe there should be a specced out (but optional) diff property also on the assertion object which can be used to make better diffs for the user to read.

Often times the assertion object is the best place to generate the diff, in a standardised format (perhaps Diff notation?), because it has the highest fidelity level of reasing about the two objects - much better than just passing expected and actual. There are sevaral concrete reasonings for this, I'll give a few examples:

Example 1. The assertion object was generated, but serialised to JSON. before being passed to an upstream reporter. This loses out on all object information in the expected and actual. Dates become strings, Maps/Sets lose their values and classes become POJOs. All hope of getting meaningful information is lost from expected and actual. If the assertion object managed to generate a diff though, this has high fidelity usable info.

Example 2. A more subtle variant of the above problem: objects are passed cross-realm, say for example the test runner runs inside of a worker. Now, without doing heaps of additional work, the test runner cannot safely reason about the types of objects that actual and expected are. Brand checks fail cross-realm (tools like Array.isArray fix this, shame we don't have the matching set for all types). A string diff doesn't care about this though, it is, after all, just a string.

Those just really enumerate the problems with actual and expected. But below are my reasons for why diff would be a much better addition in the assertion object:

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.

Some additional notes: I think a Diff format would be the best representation of this property, as by default, without syntax highlighting, a developer could reasonably be expected to read it and understand it. But it also provides a standard way for reporters to consume it and display it in other ways. For example an HTML reporter could display it as a side-by-side table, using CSS to colorise parts of it, while a terminal can use ANSI escape codes to format the diff.

trentmwillis commented 6 years ago

I agree that some standardization around diffing would be very valuable. There has been some previous talk on a related front in QUnit, that proposes a different format for representing diffs that may be easier to consume in JS libs.

That said, this seems like something that would be reasonable for the js-reporters project to provide as a lib. Then anyone implementing the standard only needs to pass actual and expected values to a function and will receive a proper diff payload at the end.

Krinkle commented 2 years ago

I'm closing this for now as there is no programmatic API or diff property in the TAP-based future (ref https://github.com/js-reporters/js-reporters/issues/133). The responsibility for rendering diffs etc then lies in individual TAP reporters, where something closer to the test framework would then have to expose the diagnostic information in some form or another and consumers will honour the format as plain text.

For QUnit, refer to: