X Tutup
The Wayback Machine - https://web.archive.org/web/20230103070948/https://github.com/nodejs/node/pull/46056
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

Jump to bottom

test: fix tap parser fails if a test logs a number #46056

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

test: fix tap parser fails if a test logs a number #46056

wants to merge 1 commit into from
+9 −1

Conversation

pulkit-30
Copy link
Contributor

@pulkit-30 pulkit-30 commented Jan 2, 2023 •
edited

fixes: #46048

code: test.mjs

import { it } from 'node:test';
import { strict as assert } from 'node:assert';

it('hello world', async () => {
  console.log(123)
  const a = 1234;
  const b = 1234;
  assert.strictEqual(a, b);
});

Before changes:

TAP version 13
# Subtest: /Users/pulkitgupta/Desktop/node/test.mjs
not ok 1 - /Users/pulkitgupta/Desktop/node/test.mjs
  ---
  duration_ms: 40.977208
  failureType: 'uncaughtException'
  error: "Cannot read properties of undefined (reading 'value')"
  code: 'ERR_TEST_FAILURE'
  stack: |-
    #Plan (node:internal/test_runner/tap_parser:653:20)
    #TAPDocument (node:internal/test_runner/tap_parser:552:28)
    #parseChunk (node:internal/test_runner/tap_parser:274:35)
    #parseTokens (node:internal/test_runner/tap_parser:249:23)
    TapParser.parse (node:internal/test_runner/tap_parser:105:24)
    TapParser._transform (node:internal/test_runner/tap_parser:187:10)
    Transform._write (node:internal/streams/transform:175:8)
    writeOrBuffer (node:internal/streams/writable:392:12)
    _write (node:internal/streams/writable:333:10)
    Writable.write (node:internal/streams/writable:337:10)
  ...
1..1
# tests 1
# pass 0
# fail 1
# cancelled 0
# skipped 0
# todo 0
# duration_ms 52.074334

After changes:

TAP version 13
# Subtest: /Users/pulkitgupta/Desktop/node/test.mjs
    # 123
    # Subtest: hello world
    ok 1 - hello world
      ---
      duration_ms: 1.487791
      ...
    # 1..1
ok 1 - /Users/pulkitgupta/Desktop/node/test.mjs
  ---
  duration_ms: 51.272208
  ...
1..1
# tests 1
# pass 1
# fail 0
# cancelled 0
# skipped 0
# todo 0
# duration_ms 53.397625

I tried to fix this bug, let me know if these changes are acceptable or to be done at some other place.

@nodejs-github-bot
Copy link
Contributor

nodejs-github-bot commented Jan 2, 2023

Review requested:

@nodejs-github-bot nodejs-github-bot added dont-land-on-v14.x PRs that should not land on the v14.x-staging branch and should not be released in v14.x. needs-ci PRs that need a full CI run. test_runner labels Jan 2, 2023
@aduh95
Copy link
Contributor

aduh95 commented Jan 2, 2023

Can you add a test case?

@pulkit-30
Copy link
Contributor Author

pulkit-30 commented Jan 2, 2023

Can you add a test case?

Sure.
also fixing failed checks. 🚀

@pulkit-30 pulkit-30 marked this pull request as draft Jan 2, 2023
@pulkit-30 pulkit-30 marked this pull request as ready for review Jan 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
dont-land-on-v14.x PRs that should not land on the v14.x-staging branch and should not be released in v14.x. needs-ci PRs that need a full CI run. test_runner
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Tap parser fails if a test logs a number
3 participants
@pulkit-30 @nodejs-github-bot @aduh95
X Tutup