test_runner: before hook runs even if it's not followed by any test

[urugator]

Version

v20.13.1

Platform

Microsoft Windows NT 10.0.19045.0 x64

Subsystem

test_runner

What steps will reproduce the bug?

import { before, describe, test } from 'node:test';

describe('suite1', (s) => {
  before(async () => {
    console.log(`${s.name} before`);
  });

  test('test1', async (t) => {
    console.log(`${s.name} ${t.name}`);
  });

  test('test2', async (t) => {
    console.log(`${s.name} ${t.name}`);
  });
});

Execute with:
node --test --test-name-pattern="nonsence"

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

Always. No.

What is the expected behavior? Why is that the expected behavior?

before hook shouldn't run if it's not followed by any test. There is no practial or logical reason to run before hook alone, it wastes time and resources.

What do you see instead?

suite1 before
▶ suite1
  ﹣ test1 (2.2042ms) # test name does not match pattern
  ﹣ test2 (0.3612ms) # test name does not match pattern
▶ suite1 (8.3937ms)
ℹ tests 2
ℹ suites 1
ℹ pass 0
ℹ fail 0
ℹ cancelled 0
ℹ skipped 2
ℹ todo 0
ℹ duration_ms 141.0891

Additional information

No response

[marco-ippolito]

I've had the same thought but in reality just because you don't call any assertion it doesn't mean the test is not doing anything.
Example: #51997
The same it applies to t.before, changing it would break many things.

[urugator]

1. It's not a test, it's a suite. If test and suite are the technically the same thing, then the suite should respect --test-name-pattern and shouldn't run. Atm there is no way to run a single suite as at least before hook of all other suites are always executed. That makes zero sense. The setup logic in our tests takes minutes. So executing a single test in one suite can take hours, just because all these other suites have their own setup logic in before.
2. Doc: This function is used to create a hook running before subtest of the current test.
   Firstly: Before subtest is quite different from before any code/assertions/whatever of the current test. If there are no subtests, then before doesn't run before anything - it just runs.
   Secondly: If test has no subtests, then before hook is completely useless, because it won't run before the code inside the test - that code is already running at that point. This is different from after, which you can actually register at any point during the execution and be sure it runs after your code.

[marco-ippolito]

I'm not sure I understand your point. I'll reopen to hear other collaborators opinion

[cjihrig]

I think the current behavior for subtests is correct - before() and after() should always run. For suites, I think we should do whatever other test runners do for before() and after().

[urugator]

My main problem is with suits, because unlike tests, they are not subject to --test-name-pattern filtering.
So there is no way to prevent executing the code inside them, including before hook.

I think the current behavior for subtests

What do you mean by subtests? Don't you mean just tests?
before should always run before any subtests, there is no question about that.
The question is whether it should run for test, without any substests.
Imo it should not. If there are no subtests, then you can't even sensibly define before what the hook is supposed to run:

test('test without subtests', async () => {
  before(async () => {
    // runs after body1 and body2 (not before)
  })

  console.log('body1')
  
  before(() => {
    // runs after body1 (not before)
  });

  console.log('body2');
})

So you're saying it should always run, but I am asking before what it should always run?
Now if a test does contain subtests, but none of them is executed, it's exactly equivalent to a test without subtests.

[urugator]

I think it basically boils down to whether before means before the current test or before subtests in the current test. I am arguing it can't refer to the current test, because current test is already running, so you can no longer schedule any job before it. So if before refers to subtests, then give me a practical example where you want to run before if none of the subtests are executed.
The problem is asymmetry with after, which can actually refer to the currently running test. And indeed this is reflected in the documentation:

This function is used to create a hook running before subtest of the current test.

This function is used to create a hook that runs after the current test finishes.

[cjihrig]

This appears to be fixed in v22. My guess is that #52221 being labeled semver-major is the reason it's not fixed in v20.

[cjihrig]

Closing per my last comment.