Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

this.equals not available in custom matcher #8295

Closed
christophgysin opened this issue Apr 9, 2019 · 8 comments
Closed

this.equals not available in custom matcher #8295

christophgysin opened this issue Apr 9, 2019 · 8 comments

Comments

@christophgysin
Copy link

🐛 Bug Report

Depending on how a custom matcher is used, this.equals might not be defined:

To Reproduce

Steps to reproduce the behavior:

expect.extend({
  customMatcher(/* received, expected */) {
    const response = {
      pass: this.equals !== undefined,
      message: `this.equals is ${this.equals ? 'defined' : 'undefined'}`,
    };
    console.debug('response:', response);
    return response;
  },
});

test('called as expect(value).customMatcher()', async () => {
  expect('foo').customMatcher('foo');
});

test('called as expect.customMatcher()', async () => {
  expect({ custom: 'foo' }).toEqual({
    custom: expect.customMatcher('foo'),
  });
});
$ npx jest equals.test.js
 FAIL  ./equals.test.js
  ✓ called as expect(value).customMatcher() (8ms)
  ✕ called as expect.customMatcher() (7ms)

  ● called as expect.customMatcher()

    expect(received).toEqual(expected)

    Difference:

    - Expected
    + Received

      Object {
    -   "custom": customMatcher<foo>,
    +   "custom": "foo",
      }

      15 |
      16 | test('called as expect.customMatcher()', async () => {
    > 17 |   expect({ custom: 'foo' }).toEqual({
         |                             ^
      18 |     custom: expect.customMatcher('foo'),
      19 |   });
      20 | });

      at Object.toEqual (equals.test.js:17:29)

  console.debug equals.test.js:7
    response: { pass: true, message: 'this.equals is defined' }

  console.debug equals.test.js:7
    response: { pass: false, message: 'this.equals is undefined' }

Note that I added a console.debug() statement because the message from the custom matcher is not actually shown (as discussed in #7492).

Expected behavior

I'd expect this.equals to be defined in both invocations.

Run npx envinfo --preset jest

$ npx envinfo --preset jest
npx: installed 1 in 0.974s

  System:
    OS: Linux 5.0 Arch Linux
    CPU: (4) x64 Intel(R) Core(TM) i5-6360U CPU @ 2.00GHz
  Binaries:
    Node: 11.13.0 - /usr/bin/node
    Yarn: 1.15.2 - /usr/bin/yarn
    npm: 6.9.0 - /usr/bin/npm
  npmPackages:
    jest: ^24.7.1 => 24.7.1
@pedrottimark
Copy link
Contributor

The custom matcher that you provide as argument to expect.extend is not interchangeable with so-called asymmetric matcher which can be item of array or value of property for certain matchers like toEqual toStrictEqual toHaveProperty toMatchObject

The built-in asymmetric matchers include expect.stringMatching and you can write your own matcher factory function to return object with asymmetricMatch method: { asymmetricMatch(received) { /* return true or false */; }}

However, expect package does not call built-in nor user-defined asymmetric matchers with equals as a property of this as described in Custom Matchers API

See https://github.com/facebook/jest/blob/master/packages/expect/src/jasmineUtils.ts#L40-L59

If expect package exported equals function (which it doesn’t currently) then you could require or import it into the file where you define your asymmetric matcher factory function.

Can you write the assertion in a way that you don’t need to embed the custom matcher?

For example, access the property path to get to the received value for custom matcher assertion:

const received = val.key.path;
expect(received).customMatcher(expected);

If you can give an example closer to your use case (without revealing propriety info) we might be able to give a more specific suggestion.

@christophgysin
Copy link
Author

Thanks for the elaborate response @pedrottimark. I see that I mixed up asymmetric and custom matchers. I made a concrete example for matching JSON encoded data with both a custom and a asymmetric matcher:

const diff = require('jest-diff');
const { equals } = require('expect/build/jasmineUtils');

expect.extend({
  toBeJson(received, expected) {
    const options = {
      comment: 'Object.is equality',
      isNot: this.isNot,
      promise: this.promise,
    };

    const pass = this.equals(JSON.parse(received), expected);

    return {
      pass,
      message: () => {
        if (pass) {
          return this.utils.matcherHint('toBeJson', undefined, undefined, options) +
          '\n\n' +
          `Expected: ${this.utils.printExpected(expected)}\n` +
          `Received: ${this.utils.printReceived(received)}`;
        }

        const difference = diff(expected, received, {
          expand: this.expand,
        });

        return (
          this.utils.matcherHint('toBeJson', undefined, undefined, options) +
          '\n\n' +
          (difference && difference.includes('- Expect')
            ? `Difference:\n\n${diffString}`
            : `Expected: ${this.utils.printExpected(expected)}\n` +
              `Received: ${this.utils.printReceived(received)}`)
        );
      },
    };
  }
});

class JsonMatching {
  constructor(expected) {
    this.expected = expected;
  }

  asymmetricMatch(received) {
    return equals(JSON.parse(received), this.expected);
  }

  toString() {
    return `JsonMatching`;
  }
}

const jsonMatching = expected => new JsonMatching(expected);

test('custom matcher success', () => {
  expect('{}').toBeJson({});
  expect('{"foo":"bar"}').toBeJson({foo:'bar'});
});

test('custom matcher fail', () => {
  expect('{"foo":"bar"}').toBeJson({foo:'baz'});
});

test('asymmetric matcher success', () => {
  expect({ foo: 'bar', body: '{}' }).toEqual({
    foo: 'bar',
    body: jsonMatching({}),
  });

  expect({ foo: 'bar', body: '{"foo":"bar"}' }).toEqual({
    foo: 'bar',
    body: jsonMatching({
      foo: 'bar',
    }),
  });
});

test('asymmetric matcher fail', () => {
  expect({ foo: 'bar', body: '{"foo":"bar"}' }).toEqual({
    foo: 'baz',
    body: jsonMatching({
      foo: 'baz',
    }),
  });
});

So I would still need to access equals, which I'm now just directly importing from jasmineUtils.

Is there cleaner way to build an asymmetric matcher and be able to use equals in the implementation?

@pedrottimark
Copy link
Contributor

Your example of requiring into the package is at the cutting edge of what is possible.

It reminds me of __SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED in React :)

Seriously I knew but had forgotten that equals is not a public API. Because several messy edge cases have come up recently, it is good to clean up the work shop before opening the door to creative makers.

Your example is very interesting to me, because a looong-term goal has been to compute a “data-driven diff” for the report after a deep equality assertion fails, to solve some problems with a diff of serialized data (for example, distracting bogus difference for expected asymmetric matcher that passes and the corresponding received value). Not sure, but good to think if there is any way to provide extension points for custom matchers, user-defined asymmetric matchers, and user-defined snapshot serializers.

A parting comment is to compare the cost and benefit of alternatives:

  • custom matcher or asymmetric matcher encapsulates transformation like JSON.parse
  • function transforms received value so assertion can have toStrictEqual or toMatchObject

In this particular case, I would be interested to know if either alternative is better or worse to trouble-shoot if JSON.parse throws a SyntaxError for the received value?

@christophgysin
Copy link
Author

I'm aware that I'm not supposed to require into the package. I'm merely trying to show why I would like to be able to use equals also in asymmetric matchers. It seems equals is already exposed to custom matchers, so all I would need is an official way to access it.

In this particular case, I would be interested to know if either alternative is better or worse to trouble-shoot if JSON.parse throws a SyntaxError for the received value?

Currently if the asymmetric matcher fails, I don't have any way to return a message with a pretty diff to show what caused it to fail. If this were a possibility, it would be easy to catch errors parsing JSON and showing that in the resulting error message.

@pedrottimark
Copy link
Contributor

Please forgive me for teasing. I needed a reminder that I can do that to experiment and trouble shoot.

You make an important point about limitation of (even built-in) asymmetric matchers to return the reason for failure. That is why I am thankful for your realistic example to stretch my thinking about what a more complete API would need to support.

Would you like me to include a mention of your user name in relevant future issues or pull requests, so you can follow and influence our attempts to improve assertions in Jest?

@christophgysin
Copy link
Author

Sure!

@SimenB
Copy link
Member

SimenB commented Oct 24, 2022

This was fixed in #11926 and #11930

@SimenB SimenB closed this as completed Oct 24, 2022
@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants