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

node --test does not show skipped tests in TAP result #45833

Closed
Dzelix opened this issue Dec 12, 2022 · 4 comments · Fixed by #46440
Closed

node --test does not show skipped tests in TAP result #45833

Dzelix opened this issue Dec 12, 2022 · 4 comments · Fixed by #46440

Comments

@Dzelix
Copy link

Dzelix commented Dec 12, 2022

Version

v18.12.1

Platform

Darwin Kernel Version 22.1.0 root:xnu-8792.41.9~2/RELEASE_ARM64_T8103 arm64

Subsystem

node:test

What steps will reproduce the bug?

In my project I have a folder named 'test' with two files inside.

The first file (Unit one) contains describe and test with different variations of skip. All the asserts are intentionally failed.

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

describe('Unit one: group one', { skip: true }, () => {
  it('test one', () => {
    assert.strictEqual(1, 10);
  })
})

describe.skip('Unit one: group two', () => {
  it('test two', () => {
    assert.strictEqual(1, 10);
  })
})

test('Unit one: test three', { skip: true }, () => {
  assert.strictEqual(1, 10);
})

The second file contains describe and test with correct asserts (Unit two).

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

describe('Unit two: group one', () => {
  it('test one', () => {
    assert.strictEqual(1, 1);
  })
})

test('Unit two: test two', () => {
  assert.strictEqual(1, 1);
})

Additionally, if I have the only file with combination of both skipped and ordinary describe and test (Unit three). This file was run separately.

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

describe('Unit three: group one', () => {
  it('test one', () => {
    assert.strictEqual(1, 1);
  })
})

test('Unit three: test two', { skip: true }, () => {
  assert.strictEqual(1, 1);
})

describe.skip('Unit three: group three', () => {
  it('test one', () => {
    assert.strictEqual(1, 1);
  })
})

test('Unit three: test three', () => {
  assert.strictEqual(1, 1);
})

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

Every time when I use node --test

What is the expected behavior?

When I run node --test, I expect to see information about skipped tests in the final TAP result.

What do you see instead?

When I run node --test with 'test' folder, contains of the first and the second files, there are no skipped tests.

TAP version 13
# Subtest: /Users/deni/Desktop/web-tools/test/one.mjs
ok 1 - /Users/deni/Desktop/web-tools/test/one.mjs
  ---
  duration_ms: 47.325042
  ...
# Subtest: /Users/deni/Desktop/web-tools/test/two.mjs
ok 2 - /Users/deni/Desktop/web-tools/test/two.mjs
  ---
  duration_ms: 44.268208
  ...
1..2
# tests 2
# pass 2
# fail 0
# cancelled 0
# skipped 0
# todo 0
# duration_ms 55.691167

When I run the third file alone I have the same result without skipped tests as well

ok 1 - /Users/deni/Desktop/web-tools/test/three.mjs
  ---
  duration_ms: 47.394208
  ...
1..1
# tests 1
# pass 1
# fail 0
# cancelled 0
# skipped 0
# todo 0
# duration_ms 54.821583

Additional information

On the contrary, if I run node test/one.mjs I have correct TAP result with information about skipped tests.

# Subtest: Unit one: group one
ok 1 - Unit one: group one # SKIP
  ---
  duration_ms: 0.565083
  ...
# Subtest: Unit one: group two
ok 2 - Unit one: group two # SKIP
  ---
  duration_ms: 0.044209
  ...
# Subtest: Unit one: test three
ok 3 - Unit one: test three # SKIP
  ---
  duration_ms: 0.052666
  ...
1..3
# tests 3
# pass 0
# fail 0
# cancelled 0
# skipped 3
# todo 0
# duration_ms 4.347417

As well as I run node test/three.mjs

TAP version 13
# Subtest: Unit three: group one
    # Subtest: test one
    ok 1 - test one
      ---
      duration_ms: 0.1995
      ...
    1..1
ok 1 - Unit three: group one
  ---
  duration_ms: 1.209541
  ...
# Subtest: Unit three: test two
ok 2 - Unit three: test two # SKIP
  ---
  duration_ms: 0.05375
  ...
# Subtest: Unit three: group three
ok 3 - Unit three: group three # SKIP
  ---
  duration_ms: 0.042459
  ...
# Subtest: Unit three: test three
ok 4 - Unit three: test three
  ---
  duration_ms: 0.043833
  ...
1..4
# tests 4
# pass 2
# fail 0
# cancelled 0
# skipped 2
# todo 0
# duration_ms 4.859
@cjihrig
Copy link
Contributor

cjihrig commented Dec 13, 2022

When you run with --test, each test file is run separately and their outputs are combined into a single TAP document. That's why the test count in the summary matches the number of test files run.

Since Node v19.2.0, the output is combined more intelligently. Now that we have the other pieces in place, I think we should go a step further now and remove the separation by test file in the output. In other words, if file 1 had 3 top level tests, and file 2 had 4 top level tests, then the output from --test would have 7 top level tests instead of 2 as it currently does.

@Dzelix
Copy link
Author

Dzelix commented Dec 15, 2022

But what about the case when I have only one file? The result is the same - all tests passed and no information about skipped tests. Does it mean that node --test combine result into TAP even if there is only one test file?

@cjihrig
Copy link
Contributor

cjihrig commented Dec 15, 2022

What I'm proposing in #45833 (comment) would apply to the case where there is a single file. The summary printed at the end of a run only applies to top level tests. If you run without --test, the top level is all of the top level test() calls in the file. If you run with --test, the top level is currently the collection of files (even if there is only one). I'm proposing we change the behavior with --test.

I think you already realize this based on your original bug report, but if you only have one file, then as a workaround for now, you can run without --test.

@cjihrig
Copy link
Contributor

cjihrig commented Dec 19, 2022

cc @manekinekko in case you have any interest in working on this ^ - I think the changes would be confined to the CLI runner and parser.

nodejs-github-bot pushed a commit that referenced this issue Feb 18, 2023
PR-URL: #46440
Fixes: #45833
Refs: #45833
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
MoLow added a commit to MoLow/node that referenced this issue Feb 18, 2023
PR-URL: nodejs#46440
Fixes: nodejs#45833
Refs: nodejs#45833
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
MoLow added a commit to MoLow/node that referenced this issue Feb 19, 2023
PR-URL: nodejs#46440
Fixes: nodejs#45833
Refs: nodejs#45833
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
MoLow added a commit to MoLow/node that referenced this issue Feb 25, 2023
PR-URL: nodejs#46440
Fixes: nodejs#45833
Refs: nodejs#45833
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
MoLow added a commit to MoLow/node that referenced this issue Feb 25, 2023
PR-URL: nodejs#46440
Fixes: nodejs#45833
Refs: nodejs#45833
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
MoLow added a commit to MoLow/node that referenced this issue Feb 25, 2023
PR-URL: nodejs#46440
Fixes: nodejs#45833
Refs: nodejs#45833
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
MoLow added a commit to MoLow/node that referenced this issue Feb 25, 2023
PR-URL: nodejs#46440
Fixes: nodejs#45833
Refs: nodejs#45833
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
juanarbol pushed a commit that referenced this issue Mar 3, 2023
PR-URL: #46440
Backport-PR-URL: #46839
Fixes: #45833
Refs: #45833
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
juanarbol pushed a commit that referenced this issue Mar 5, 2023
PR-URL: #46440
Backport-PR-URL: #46839
Fixes: #45833
Refs: #45833
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
targos pushed a commit that referenced this issue Mar 13, 2023
PR-URL: #46440
Fixes: #45833
Refs: #45833
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
targos pushed a commit that referenced this issue Mar 14, 2023
PR-URL: #46440
Fixes: #45833
Refs: #45833
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@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