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

Expect calls custom inspect() function unexpectedly #4478

Open
6 tasks done
matt-usurp opened this issue Nov 11, 2023 · 6 comments
Open
6 tasks done

Expect calls custom inspect() function unexpectedly #4478

matt-usurp opened this issue Nov 11, 2023 · 6 comments
Labels
p2-edge-case Bug, but has workaround or limited in scope (priority) upstream

Comments

@matt-usurp
Copy link

matt-usurp commented Nov 11, 2023

Describe the bug

Any object that has an inspect() function will be called when passed to expect(). If this function happens to expect an argument, for example inspect(fn: () => void) then the test will fail for the wrong reason. This doesn't seem to mentioned in the api and means you need to guard around this currently by doing more targeted tests.

Reproduction

Using vitest@^0.34.0 (including beta 1.0.0 releases) the following test:

class Example {
  public inspect(fn: () => void) {
    fn();
  }
}

it('custom inspect function', (): void => {
  expect(
    new Example(),
  ).toStrictEqual(undefined);
});

Will output as expected:

 FAIL  src/example.test.ts > custom inspect function
TypeError: fn is not a function
 ❯ Example.inspect src/event/service/foo.test.ts:3:5
      1| class Example {
      2|   public inspect(fn: () => void) {
      3|     fn();
       |     ^
      4|   }
      5| }
 ❯ inspectCustom node_modules/loupe/loupe.js:793:20
 ❯ Object.inspect node_modules/loupe/loupe.js:827:20

This appears to be down to loupe but again, this is unexpected.

System Info

System:
    OS: macOS 14.0
    CPU: (12) arm64 Apple M2 Max
    Memory: 2.27 GB / 64.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 18.18.1 - ~/.nvm/versions/node/v18.18.1/bin/node
    Yarn: 1.22.19 - ~/.nvm/versions/node/v18.18.1/bin/yarn
    npm: 10.2.0 - ~/.nvm/versions/node/v18.18.1/bin/npm
    pnpm: 8.9.0 - ~/.nvm/versions/node/v18.18.1/bin/pnpm
  Browsers:
    Chrome: 119.0.6045.123
    Safari: 17.0
  npmPackages:
    @vitest/coverage-v8: ^0.34.5 => 0.34.6 
    vitest: ^0.34.5 => 0.34.6

Used Package Manager

npm

Validations

@matt-usurp matt-usurp changed the title Expect calls custom inspect() function causing tests to break Expect calls custom inspect() function unexpectedly Nov 11, 2023
@matt-usurp
Copy link
Author

As mentioned this looks to be loupe, but potentially a configuration thing that can be disabled or made configurable. The stack frames in the reproduction relate to this line https://github.com/chaijs/loupe/blob/main/src/index.ts#L139C7-L139C20 which suggests it can be disabled with a configuration option.

@sheremet-va
Copy link
Member

We should probably just disable it by default on our side. PR welcome.

@sheremet-va sheremet-va added bug pr welcome p3-minor-bug An edge case that only affects very specific usage (priority) and removed pending triage labels Nov 13, 2023
@matt-usurp
Copy link
Author

matt-usurp commented Nov 13, 2023

I will see what I can do later today 👍🏻
To be clear, where should I cut from? The 0.34 branch?

@matt-usurp
Copy link
Author

matt-usurp commented Nov 14, 2023

I gave this a go and the contributing guide wasn't getting me anywhere, things were erroring. So I decided to try go in blind and discovered the following which suggests we cannot turn of the custom inspect for this specific case:

  • My example above is ran
  • This code is triggered
    def(['toHaveBeenCalledWith', 'toBeCalledWith'], function (...args) {
  • Which calls into utils.getMessage() which calls into chai
  • Specifically objDisplay() which calls inspect() from loupe, issue is there is no way to set options

This appears to be the only code path that triggers inspect() other than the custom wrapper in the @vitest/utils package. I am at a loss with trying to fix this without writing a custom utils.getMessage

@sheremet-va
Copy link
Member

There is a test.chaiConfig property in the config that can influence inspect options I think, but I'm not sure.

@matt-usurp
Copy link
Author

I followed the stack through these functions (now with links to chai github):

The final inspect() is not loupe but a wrapper which looks like this:

function inspect(obj, showHidden, depth, colors) {
  var options = {
    colors: colors,
    depth: (typeof depth === 'undefined' ? 2 : depth),
    showHidden: showHidden,
    truncate: config.truncateThreshold ? config.truncateThreshold : Infinity,
  };
  return loupe.inspect(obj, options);
}

So I don't think a chai config will take effect, unless its some magic stuff.

@sheremet-va sheremet-va added upstream p2-edge-case Bug, but has workaround or limited in scope (priority) and removed bug pr welcome p3-minor-bug An edge case that only affects very specific usage (priority) labels Feb 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p2-edge-case Bug, but has workaround or limited in scope (priority) upstream
Projects
None yet
Development

No branches or pull requests

2 participants