Test line that fails is well hidden if a helper method fails

[Artur-]

Describe the bug

Given a test like

    it('should fail at the second expect', () => {
      expect(1).to.equal(1);
      expect(2).to.equal(1);
    });

you get the expect output, e.g.

 FAIL  my/test/selection.test.ts > selection > single > should fail at the second expect
AssertionError: expected 2 to equal 1

- Expected
+ Received

- 1
+ 2

 ❯ my/test/selection.test.ts:123:19
    121|     it('should fail at the second expect', () => {
    122|       expect(1).to.equal(1);
    123|       expect(2).to.equal(1);
       |                   ^
    124|     });
    125|   });

But when expect is extracted to a helper like

  function e(a: number, b: number) {
    expect(a).to.equal(b);
  }

the output of the test

    it('should fail at the second expect', () => {
      e(1, 1);
      e(2, 1);
    });

is

 FAIL  my/test/selection.test.ts > selection > single > should fail at the second expect
AssertionError: expected 2 to equal 1

- Expected
+ Received

- 1
+ 2

 ❯ e my/test/selection.test.ts:80:17
     78| 
     79|   function e(a: number, b: number) {
     80|     expect(a).to.equal(b);
       |                 ^
     81|   }
     82| 
 ❯ my/test/selection.test.ts:123:6

and actually I did not realize before pasting the error here that the line number is actually present for the test case (my/test/selection.test.ts:123:6). I thought it was completely missing. Could maybe the line of the actual test always be printed?

Reproduction

As above

System Info

System:
    OS: macOS 14.0
    CPU: (10) arm64 Apple M1 Max
    Memory: 870.36 MB / 64.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.9.0 - ~/.nvm/versions/node/v20.9.0/bin/node
    npm: 10.1.0 - ~/.nvm/versions/node/v20.9.0/bin/npm
    bun: 1.0.7 - ~/.bun/bin/bun
  Browsers:
    Brave Browser: 107.1.45.123
    Chrome: 119.0.6045.123
    Chrome Canary: 121.0.6125.0
    Edge: 119.0.2151.58
    Safari: 17.0
  npmPackages:
    @vitejs/plugin-react: 4.1.1 => 4.1.1 
    @vitest/browser: ^0.34.6 => 0.34.6 
    @vitest/ui: ^0.34.6 => 0.34.6 
    vite: 4.5.0 => 4.5.0 
    vitest: ^0.34.6 => 0.34.6

Used Package Manager

npm

Validations

• Follow our Code of Conduct
• Read the Contributing Guidelines.
• Read the docs.
• Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
• Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
• The provided reproduction is a minimal reproducible example of the bug.

[AriPerkkio]

You could do something like explained here: https://kentcdodds.com/blog/improve-test-error-messages-of-your-abstractions

This:

test("add", async () => {
  customExpect(add(1, 2), 4);
});

function customExpect(expected, actual) {
  expect(expected).toBe(actual);
}

 ❯ customExpect tests/math.test.ts:9:20
      7| 
      8| function customExpect(expected, actual) {
      9|   expect(expected).toBe(actual);
       |                    ^
     10| }
     11| 
 ❯ tests/math.test.ts:5:3

becomes this:

function customExpect(expected: any, actual: any) {
  try {
    expect(expected).toBe(actual);
  } catch (error) {
    Error.captureStackTrace(error, customExpect);
    throw error;
  }
}

 ❯ tests/math.test.ts:5:3
      3| 
      4| test("add", async () => {
      5|   customExpect(add(1, 2), 4);
       |   ^
      6| });
      7| 

[sheremet-va]

This is not the line in the test file, this is just the stack trace.

I guess we can color it differently though.

[Artur-]

Coloring differently would help but it would also be great if the failing line from the test method would always be shown, in my case something like

 ❯ e my/test/selection.test.ts:80:17
     78| 
     79|   function e(a: number, b: number) {
     80|     expect(a).to.equal(b);
       |                 ^
     81|   }
     82| 
 ❯ my/test/selection.test.ts:123:6
    121|    it('should fail at the second expect', () => {
    122|      e(1, 1);
    123|      e(2, 1);
       |        ^
    124|    });

[Artur-]

In another case I have, I see

 ❯ my/test/selection.test.ts:75:77
     73|   async function assertSelection(expected: HTMLElement[]) {
     74|     await waitForExpect(() => {
     75|       expect(something).to.deep.equal(expected);
       |                                                                             ^
     76|     }, 100);
     77|   }
 ❯ runExpectation ../../../../../node_modules/.vite/deps/wait-for-expect.js:67:29

There the actual test method, which looks like

      await assertSelection([button]);

is not even mentioned so based on the output I cannot know which of the assertSelection calls has failed

[sheremet-va]

I don't think it makes sense to show error code example inside a test by default. It's possible that error was thrown by external tool or your non-assertion function, and this will only make it harder to debug.

Playwright has "box" feature that enables this, we can go in that direction, but right now I think if you want to have assertions in a separate function/file, you need to control this manually like @AriPerkkio mentioned.

[sheremet-va]

I experimented a bit with different displays for a test file, but I did not like any of them.

I don't think it is a good idea to manipulate how we display code blocks from our side because we will need the information about test position (line/column) which will just slow down every test file. We can provide a wrapper to strip stack trace though (like https://playwright.dev/docs/release-notes#hide-implementation-details-box-test-steps) so people can opt-in for this kind of behavior.

[sheremet-va]

we will need the information about test position (line/column) which will just slow down every test file

There is actually a solution for this - populate the test position only if the test failed. With this we can even support something like vitest filepath:30

[Katli95]

We can provide a wrapper to strip stack trace though (like https://playwright.dev/docs/release-notes#hide-implementation-details-box-test-steps) so people can opt-in for this kind of behavior.

@sheremet-va, That'd be awesome, has there been any further discussion around introducing such a thing?