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

assert.deepEqual can generate invalid YAML output #47075

Closed
connor4312 opened this issue Mar 13, 2023 · 0 comments · Fixed by #47088
Closed

assert.deepEqual can generate invalid YAML output #47075

connor4312 opened this issue Mar 13, 2023 · 0 comments · Fixed by #47088

Comments

@connor4312
Copy link
Contributor

connor4312 commented Mar 13, 2023

Version

v19.7.0

Platform

Linux Helios 5.10.102.1-microsoft-standard-WSL2 #1 SMP Wed Mar 2 00:30:59 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

test_runner

What steps will reproduce the bug?

Have this test file:

const { test } = require("node:test");
const { deepEqual } = require("node:assert");

test("b", () => {
  deepEqual({ foo: 1 }, { foo: 2 });
});

Run it with node file.js and examine the output, or use something like tap-parser and node file.js | tap-parser

How often does it reproduce? Is there a required condition?

100%

What is the expected behavior?

It should output valid TAP

What do you see instead?

TAP version 13
# Subtest: b
not ok 1 - b
  ---
  duration_ms: 2.116422
  failureType: 'testCodeFailure'
  error: |-
    Expected values to be loosely deep-equal:
    
    {
      foo: 1
    }
    
    should loosely deep-equal
    
    {
      foo: 2
    }
  code: 'ERR_ASSERTION'
  foo: 2
  foo: 1
  operator: 'deepEqual'
  stack: |-
    TestContext.<anonymous> (/home/connor/Github/nodejs-testing/testCases/simple/workspace/test-WithADash.js:5:3)
    Test.runInAsyncScope (node:async_hooks:203:9)
    Test.run (node:internal/test_runner/test:549:25)
    Test.start (node:internal/test_runner/test:465:17)
    test (node:internal/test_runner/harness:172:18)
    Object.<anonymous> (/home/connor/Github/nodejs-testing/testCases/simple/workspace/test-WithADash.js:4:1)
    Module._compile (node:internal/modules/cjs/loader:1275:14)
    Module._extensions..js (node:internal/modules/cjs/loader:1329:10)
    Module.load (node:internal/modules/cjs/loader:1133:32)
    Module._load (node:internal/modules/cjs/loader:972:12)
  ...
1..1
# tests 1
# pass 0
# fail 1
# cancelled 0
# skipped 0
# todo 0
# duration_ms 7.64052

TAP uses YAML in its output, and here properties from the object in the deep equal check are added to the YAML. Duplicate mapping keys are invalid in YAML, and thus it breaks tap-parser, resulting in the following output:

[
  [ 'version', 13 ],
  [ 'comment', '# Subtest: b\n' ],
  [
    'assert',
    Result {
      name: 'b',
      id: 1,
      buffered: false,
      tapError: null,
      skip: false,
      todo: false,
      previous: null,
      plan: null,
      diag: null,
      time: null,
      fullname: '',
      ok: false
    }
  ],
  [
    'extra',
    '---\n' +
      '  duration_ms: 2.24284\n' +
      "  failureType: 'testCodeFailure'\n" +
      '  error: |-\n' +
      '    Expected values to be loosely deep-equal:\n' +
      '    \n' +
      '    {\n' +
      '      foo: 1\n' +
      '    }\n' +
      '    \n' +
      '    should loosely deep-equal\n' +
      '    \n' +
      '    {\n' +
      '      foo: 2\n' +
      '    }\n' +
      "  code: 'ERR_ASSERTION'\n" +
      '  foo: 2\n' +
      '  foo: 1\n' +
      "  operator: 'deepEqual'\n" +
      '  stack: |-\n' +
      '    TestContext.<anonymous> (/home/connor/Github/nodejs-testing/testCases/simple/workspace/test-WithADash.js:5:3)\n' +
      '    Test.runInAsyncScope (node:async_hooks:203:9)\n' +
      '    Test.run (node:internal/test_runner/test:549:25)\n' +
      '    Test.start (node:internal/test_runner/test:465:17)\n' +
      '    test (node:internal/test_runner/harness:172:18)\n' +
      '    Object.<anonymous> (/home/connor/Github/nodejs-testing/testCases/simple/workspace/test-WithADash.js:4:1)\n' +
      '    Module._compile (node:internal/modules/cjs/loader:1275:14)\n' +
      '    Module._extensions..js (node:internal/modules/cjs/loader:1329:10)\n' +
      '    Module.load (node:internal/modules/cjs/loader:1133:32)\n' +
      '    Module._load (node:internal/modules/cjs/loader:972:12)\n' +
      '...\n'
  ],
  [ 'plan', Plan { start: 1, end: 1, comment: '' } ],
  [ 'comment', '# tests 1\n' ],
  [ 'comment', '# pass 0\n' ],
  [ 'comment', '# fail 1\n' ],
  [ 'comment', '# cancelled 0\n' ],
  [ 'comment', '# skipped 0\n' ],
  [ 'comment', '# todo 0\n' ],
  [ 'comment', '# duration_ms 7.464937\n' ],
  [ 'comment', '# failed 1 test\n' ],
  [
    'complete',
    FinalResults {
      ok: false,
      count: 1,
      pass: 0,
      fail: 1,
      bailout: false,
      todo: 0,
      skip: 0,
      plan: FinalPlan {
        start: 1,
        end: 1,
        skipAll: false,
        skipReason: '',
        comment: ''
      },
      failures: [
        Result {
          name: 'b',
          id: 1,
          buffered: false,
          tapError: null,
          skip: false,
          todo: false,
          previous: null,
          plan: null,
          diag: null,
          time: null,
          fullname: '',
          ok: false
        }
      ],
      time: null
    }
  ],
  [ 'finish' ],
  [ 'close' ]
]

You can also verify this error by pasting the contents of the YAMLBlock in something like yamllint.

Additional information

First reported on connor4312/nodejs-testing#5

@MoLow MoLow changed the title assert.deepEqual can generate invalid node:test output assert.deepEqual can generate invalid YAML output Mar 14, 2023
nodejs-github-bot pushed a commit that referenced this issue Apr 4, 2023
PR-URL: #47088
Fixes: #47075
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
RafaelGSS pushed a commit that referenced this issue Apr 5, 2023
PR-URL: #47088
Fixes: #47075
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
RafaelGSS pushed a commit that referenced this issue Apr 5, 2023
PR-URL: #47088
Fixes: #47075
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
RafaelGSS pushed a commit that referenced this issue Apr 6, 2023
PR-URL: #47088
Fixes: #47075
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
RafaelGSS pushed a commit that referenced this issue Apr 7, 2023
PR-URL: #47088
Fixes: #47075
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
RafaelGSS pushed a commit that referenced this issue Apr 8, 2023
PR-URL: #47088
Fixes: #47075
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
RafaelGSS pushed a commit that referenced this issue Apr 13, 2023
PR-URL: #47088
Fixes: #47075
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
danielleadams pushed a commit that referenced this issue Jul 6, 2023
PR-URL: #47088
Fixes: #47075
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MoLow added a commit to MoLow/node that referenced this issue Jul 6, 2023
PR-URL: nodejs#47088
Fixes: nodejs#47075
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants