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

Show diff if test fails with an assertion that has a message #48465

Closed
remcohaszing opened this issue Jun 15, 2023 · 6 comments
Closed

Show diff if test fails with an assertion that has a message #48465

remcohaszing opened this issue Jun 15, 2023 · 6 comments
Labels
assert Issues and PRs related to the assert subsystem. feature request Issues that request new features to be added to Node.js.

Comments

@remcohaszing
Copy link

What is the problem this feature will solve?

Let’s say we have the following test:

import assert from 'node:assert/strict'
import {test} from 'node:test'

test('test', () => {
  assert.deepEqual({foo: 1}, {bar: 2})
})

This yields:

✖ test (2.74485ms)
  AssertionError [ERR_ASSERTION]: Expected values to be strictly deep-equal:
  + actual - expected
  
    {
  +   foo: 1
  -   bar: 2
    }
      at TestContext.<anonymous> (file:///tmp/test.js:5:10)
      at Test.runInAsyncScope (node:async_hooks:203:9)
      at Test.run (node:internal/test_runner/test:550:25)
      at Test.start (node:internal/test_runner/test:466:17)
      at startSubtest (node:internal/test_runner/harness:203:17) {
    generatedMessage: true,
    code: 'ERR_ASSERTION',
    actual: [Object],
    expected: [Object],
    operator: 'deepStrictEqual'
  }

ℹ tests 1
ℹ suites 0
ℹ pass 0
ℹ fail 1
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 9.405037

✖ failing tests:

✖ test (2.74485ms)
  AssertionError [ERR_ASSERTION]: Expected values to be strictly deep-equal:
  + actual - expected
  
    {
  +   foo: 1
  -   bar: 2
    }
      at TestContext.<anonymous> (file:///tmp/test.js:5:10)
      at Test.runInAsyncScope (node:async_hooks:203:9)
      at Test.run (node:internal/test_runner/test:550:25)
      at Test.start (node:internal/test_runner/test:466:17)
      at startSubtest (node:internal/test_runner/harness:203:17) {
    generatedMessage: true,
    code: 'ERR_ASSERTION',
    actual: [Object],
    expected: [Object],
    operator: 'deepStrictEqual'
  }

Now we want to provide some more context for the assertion, so we add a message.

import assert from 'node:assert/strict'
import {test} from 'node:test'

test('test', () => {
  assert.deepEqual({foo: 1}, {bar: 2}, 'objects should be equal')
})

Now this yields (the diff is gone):

✖ test (1.631264ms)
  AssertionError [ERR_ASSERTION]: objects should be equal
      at TestContext.<anonymous> (file:///tmp/test.js:5:10)
      at Test.runInAsyncScope (node:async_hooks:203:9)
      at Test.run (node:internal/test_runner/test:550:25)
      at Test.start (node:internal/test_runner/test:466:17)
      at startSubtest (node:internal/test_runner/harness:203:17) {
    generatedMessage: false,
    code: 'ERR_ASSERTION',
    actual: [Object],
    expected: [Object],
    operator: 'deepStrictEqual'
  }

ℹ tests 1
ℹ suites 0
ℹ pass 0
ℹ fail 1
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 9.309917

✖ failing tests:

✖ test (1.631264ms)
  AssertionError [ERR_ASSERTION]: objects should be equal
      at TestContext.<anonymous> (file:///tmp/test.js:5:10)
      at Test.runInAsyncScope (node:async_hooks:203:9)
      at Test.run (node:internal/test_runner/test:550:25)
      at Test.start (node:internal/test_runner/test:466:17)
      at startSubtest (node:internal/test_runner/harness:203:17) {
    generatedMessage: false,
    code: 'ERR_ASSERTION',
    actual: [Object],
    expected: [Object],
    operator: 'deepStrictEqual'
  }

Although the intent of adding an assertion message is good, IMO the former is much more useful.

What is the feature you are proposing to solve the problem?

Show both the custom message and the diff. I.e.

✖ test (2.74485ms)
  AssertionError [ERR_ASSERTION]: objects should be equal
  + actual - expected
  
    {
  +   foo: 1
  -   bar: 2
    }
      at TestContext.<anonymous> (file:///tmp/test.js:5:10)
      at Test.runInAsyncScope (node:async_hooks:203:9)
      at Test.run (node:internal/test_runner/test:550:25)
      at Test.start (node:internal/test_runner/test:466:17)
      at startSubtest (node:internal/test_runner/harness:203:17) {
    generatedMessage: true,
    code: 'ERR_ASSERTION',
    actual: [Object],
    expected: [Object],
    operator: 'deepStrictEqual'
  }

ℹ tests 1
ℹ suites 0
ℹ pass 0
ℹ fail 1
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 9.405037

✖ failing tests:

✖ test (2.74485ms)
  AssertionError [ERR_ASSERTION]: objects should be equal
  + actual - expected
  
    {
  +   foo: 1
  -   bar: 2
    }
      at TestContext.<anonymous> (file:///tmp/test.js:5:10)
      at Test.runInAsyncScope (node:async_hooks:203:9)
      at Test.run (node:internal/test_runner/test:550:25)
      at Test.start (node:internal/test_runner/test:466:17)
      at startSubtest (node:internal/test_runner/harness:203:17) {
    generatedMessage: true,
    code: 'ERR_ASSERTION',
    actual: [Object],
    expected: [Object],
    operator: 'deepStrictEqual'
  }

What alternatives have you considered?

None

@remcohaszing remcohaszing added the feature request Issues that request new features to be added to Node.js. label Jun 15, 2023
@MoLow MoLow added the assert Issues and PRs related to the assert subsystem. label Jun 15, 2023
@AdityaPimpalkar
Copy link

Hi @MoLow, I would like to work on this issue and submit a pull request.

@MoLow
Copy link
Member

MoLow commented Jul 16, 2023

@AdityaPimpalkar if you have a solution, you should open a PR. i'd be happy to review it

AdityaPimpalkar added a commit to AdityaPimpalkar/node that referenced this issue Jul 17, 2023
Addition of the error difference (actual and expected) when provided a custom message

Fixes: nodejs#48465
AdityaPimpalkar added a commit to AdityaPimpalkar/node that referenced this issue Jul 17, 2023
Addition of the error difference (actual and expected) when provided a custom message

Fixes: nodejs#48465
AdityaPimpalkar added a commit to AdityaPimpalkar/node that referenced this issue Jul 18, 2023
Addition of the error difference (actual and expected) when provided a custom message

Fixes: nodejs#48465
AdityaPimpalkar added a commit to AdityaPimpalkar/node that referenced this issue Jul 25, 2023
Addition of the error difference (actual and expected) when provided a custom message

Fixes: nodejs#48465
Copy link
Contributor

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

@github-actions github-actions bot added the stale label Jan 14, 2024
@remcohaszing
Copy link
Author

There is an open PR to resolve this awaiting code review.

@github-actions github-actions bot removed the stale label Jan 15, 2024
Copy link
Contributor

There has been no activity on this feature request for 5 months. To help maintain relevant open issues, please add the never-stale Mark issue so that it is never considered stale label or close this issue if it should be closed. If not, the issue will be automatically closed 6 months after the last non-automated comment.
For more information on how the project manages feature requests, please consult the feature request management document.

@github-actions github-actions bot added the stale label Jul 13, 2024
@remcohaszing
Copy link
Author

The PR is still awaiting review

@github-actions github-actions bot removed the stale label Jul 14, 2024
puskin94 added a commit to puskin94/node that referenced this issue Sep 4, 2024
puskin94 added a commit to puskin94/node that referenced this issue Sep 4, 2024
targos pushed a commit that referenced this issue Oct 4, 2024
Fixes: #48465
PR-URL: #54759
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this issue Oct 4, 2024
Fixes: #48465
PR-URL: #54759
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
louwers pushed a commit to louwers/node that referenced this issue Nov 2, 2024
Fixes: nodejs#48465
PR-URL: nodejs#54759
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
marco-ippolito pushed a commit that referenced this issue Nov 16, 2024
Fixes: #48465
PR-URL: #54759
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
marco-ippolito pushed a commit that referenced this issue Nov 17, 2024
Fixes: #48465
PR-URL: #54759
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
tpoisseau pushed a commit to tpoisseau/node that referenced this issue Nov 21, 2024
Fixes: nodejs#48465
PR-URL: nodejs#54759
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assert Issues and PRs related to the assert subsystem. feature request Issues that request new features to be added to Node.js.
Projects
None yet
3 participants