fast-check icon indicating copy to clipboard operation
fast-check copied to clipboard

Improve IDE support by adding `expected` and `actual` properties on test fail

Open dbartholomae opened this issue 5 years ago • 3 comments

🚀 Feature Request

Add expected and actual properties to the error thrown when failing a test.

Motivation

At the moment test runner IDE like WebStorm and Wallaby do not show their "Click to see difference" button which allows to inspect differences between expected and actual values. This button is shown based on the expected and actual properties that are added by jest. The topic came up in the following discussion for Wallaby, but it also affects usage in Webstorm's native test runner: https://github.com/wallabyjs/public/issues/2459

Example

describe('every string', () => {
  it('is foo', () => {
    assert(property(string(), (str) => {
      expect(str).toEqual('foo')
    }))
  })
})

This does not generate a clickable diff link, but it should.

dbartholomae avatar Jun 29 '20 12:06 dbartholomae

Thanks a lot for the report. There is indeed something to improve and do here ;)

Indeed as said in https://github.com/wallabyjs/public/issues/2459#issuecomment-650854293, fast-check totally wipes the original instance of error that has been raised during the run. I think that we should at least fix the fact that this error does not appear in the resulting RunDetails produced after a run (see 1. and 2.).

  1. In check/runner/reporter/RunExecution.ts, the method fail called in case of failure receive a string representation of the error. In addition of this string we should also include the original instance of error (maybe of type unknown as we do not have any certainty of its real type).
  2. In check/runner/reporter/RunDetails.ts, in addition of error we should also have errorInstance: unknown, so that those details get exposed into the RunDetails (which is the structure that will contain all the data used to render the final error).
  3. Two options: either fast-check adds those actual/expected or it can be done by a custom reporter (see https://github.com/dubzzz/fast-check/blob/master/documentation/1-Guides/Tips.md#customize-the-reported-error).

In order to do 1., we would need to change the signature of IProperty as today it declares run(v: Ts): PreconditionFailure | string | null. It would need to be updated into run(v: Ts): PreconditionFailure | string | { error: unknown, errorMessage: string } | null (or maybe without the string or maybe without errorMessage but anyway it will be a breaking change in terms of typings).

Nonetheless as stated in 3. I'm not sure that fast-check should expose actual/expected itself. But anyway, we need to do 1. and 2. before reaching that part.

dubzzz avatar Jun 29 '20 20:06 dubzzz

@dbartholomae Now that v3 is coming very very soon, I'm checking this issue to make the breaking change in the API for v3 and not wait yet another release for it. Here is my current WIP to add the feature you asked to #2957.

POC released as: https://github.com/dubzzz/fast-check/pull/2957#issuecomment-1122806453

yarn add https://627ac44fa85f791b2a3ab67a--dazzling-goodall-4f0e38.netlify.app/fast-check.tgz
npm i https://627ac44fa85f791b2a3ab67a--dazzling-goodall-4f0e38.netlify.app/fast-check.tgz

dubzzz avatar May 10 '22 20:05 dubzzz

Step 1 and 2 of my plan described on https://github.com/dubzzz/fast-check/issues/660#issuecomment-651362595 have been released in v3. For the step 3, I'm thinking about many things to get proper errors:

  • Copy attributes of the original error onto the one thrown by fast-check, so actual and expected would be too. I have two concerns with this one 1. it seems that it is still not enough for wallabyjs 2. except wallabyjs I don't know any other tools doing such reports, would be nice to see others to have a unified approach handling all the reporters at the same time if possible
  • Error cause seems to be the way to go in the longrun, instead of copying the message coming from the error and its stack, I could just simply attach it to my error and tada the reporter can do what it wants out of it (attempted on https://github.com/dubzzz/fast-check/pull/2965)

dubzzz avatar May 31 '22 07:05 dubzzz

On fast-check side, I think we can consider the problem as closed now that we forward the original error as-is via the official 'Error with cause' mechanism. For the moment, the feature is not enabled by default as the ecosystem is not fully ready yet (only one test runner support them).

dubzzz avatar Sep 24 '22 21:09 dubzzz