[node:test] Infinite loop occurs when files is empty

Version

v20.4.0

Platform

Darwin MacBook-Pro.local 22.5.0 Darwin Kernel Version 22.5.0: Thu Jun 8 22:22:20 PDT 2023; root:xnu-8796.121.3~7/RELEASE_ARM64_T6000 arm64

Subsystem

No response

What steps will reproduce the bug?

This code occurs infinite loop.

import { run } from 'node:test' 
import { tap } from 'node:test/reporters';

const stream = run({
  // files: undefined
}).compose(tap);

stream.pipe(process.stdout);

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

everytime

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

Default is written astest runner execution model. But it does not work.
https://nodejs.org/api/test.html#runoptions

files: An array containing the list of files to run. Default matching files from test runner execution model

It seems that tests that are run during node --test should be run when files is empty.

What do you see instead?

https://github.com/koh110/minimum-nodejs-test

$ node test-run-null-files.mjs
TAP version 13
# TAP version 13
# \# TAP version 13
# \# \\\# TAP version 13
# \# \\\# \\\\\\\# TAP version 13
# \# \\\# \\\\\\\# \\\\\\\\\\\\\\\# TAP version 13
# \# \\\# \\\\\\\# \\\\\\\\\\\\\\\# \\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\# TAP version 13

Additional information

createTestFileList gets its own file path.
It seems that it have to exclude own file path when executing in run.

const hasUserSuppliedPattern = process.argv.length > 1;
const patterns = hasUserSuppliedPattern ? ArrayPrototypeSlice(process.argv, 1) : [kDefaultPattern];
console.log('createTestFileList:', hasUserSuppliedPattern, patterns);

node/lib/internal/test_runner/runner.js

Lines 90 to 92 in 9988130

    
           const cwd = process.cwd(); 
        
           const hasUserSuppliedPattern = process.argv.length > 1; 
        
           const patterns = hasUserSuppliedPattern ? ArrayPrototypeSlice(process.argv, 1) : [kDefaultPattern];

let testFiles = files ?? createTestFileList();

if (shard) {
  testFiles = ArrayPrototypeFilter(testFiles, (_, index) => index % shard.total === shard.index - 1);
}
console.log('testFiles: ', testFiles);

node/lib/internal/test_runner/runner.js

Lines 473 to 477 in 9988130

    
           let testFiles = files ?? createTestFileList(); 
        
           if (shard) { 
        
             testFiles = ArrayPrototypeFilter(testFiles, (_, index) => index % shard.total === shard.index - 1); 
        
           }

$ ~/dev/node/out/Release/node ./test-run-null-files.mjs
createTestFileList: true [ '/xxx/minimum-nodejs-test/test-run-null-files.mjs' ]
testFiles:  [ '/xxx/minimum-nodejs-test/test-run-null-files.mjs' ]
TAP version 13
# createTestFileList: true [ '/xxx/minimum-nodejs-test/test-run-null-files.mjs' ]
# testFiles:  [ '/xxx/minimum-nodejs-test/test-run-null-files.mjs' ]
# TAP version 13

I am not sure I would consider this a bug, you should either specify files or name the root file in some way that the default execution model won't detect it as a test file.
and, due to the fact the the default will change to be a glob pattern starting node 21 - I don't think it should be excluded in any way

Does it mean that the files should be specified? Or does it mean that it should not be excluded own file path?

I don't think it should be excluded in any way

Would you prefer to modify the document in that case?

Default is written astest runner execution model

the file calling run should not be excluded

So, I think Default matching in the document should be removed. What do you think about it?

why? it is still correct

I think Default is the initial value that works that way if not specified.
For example, the initial values for concurrency and inspectPort, timeout are described in the document.
However, files will not work if it is excluded.
I think this is different compared to other Default.

So I think it should state that it cannot be omitted instead of Default.

How do I call the default file pattern like node --test for run?

run({file: undefined}) or just run({}) will use the default behavior

When I run run({file: undefined}) or run({}), it's occurs Infinite loop. I want to solve it.

So, I think options.files has bug or Default in doc is not correct, when i create this issue.

Could you please exec this code?

$ node -v
v20.4.0

$ tree
.
└── run.js

$ cat run.js
require('node:test').run({ files: undefined }).compose(require('node:test/reporters').tap).pipe(process.stdout)

# this code occurs infinite loop
$ node run.js

run.js is included in process.argv in createFiles if i set files: undefined. It seems to refer to itself.
https://github.com/nodejs/node/blob/99881304c47138e09a06e6b2bd876cfdf840047e/lib/internal/test_runner/runner.js#L89C1-L93

This happens with any file name.
My first sample code with a file name starting with test- was bad. sorry.

@koh110

run({file: undefined}) or just run({}) will use the default behavior

Is that mean that 'node run.js' (which files: undefined) is equal to 'node --test run.js', as @koh110 explained? I don't think that such 'expected' behaviour is really expected. We should have note in docs, or some asserts in test-runner code to brake the loop.

@MoLow

@MoLow Hi! Could you please help me with what is wrong with this sample code?

@koshic Thanks for the supplement!

@koh110

@koh110 if you experience an infinate loop - that means the file called by run has a name that is detected as a test file (as described here - https://nodejs.org/api/test.html#test-runner-execution-model it probably has the word test in its name.
you can avoid that by changing the file name or not using the default files argument

@MoLow

@MoLow It occurs with any file name. Could you please run this sample?
#48823 (comment)
This sample file is named run.js and it is located in the root directory.
I think this is not part of the pattern.

run call createTestFileList when options.files is empty.

let testFiles = files ?? createTestFileList();

But hasUserSuppliedPattern in createTestFileList is always true when node --test run.js.

function createTestFileList() {
  const cwd = process.cwd();
  const hasUserSuppliedPattern = process.argv.length > 1;
  const patterns = hasUserSuppliedPattern ? ArrayPrototypeSlice(process.argv, 1) : [kDefaultPattern];

process.argv cannot be empty when trying to call a file containing run.

So, I think createTestFileList always returns path of run that named like root/a.js, root/b.js...

One way we could address this with a semver major change is to make options.files a required option. That would eliminate any surprise behavior.

@cjihrig

@cjihrig Do you mean that the default file pattern like node --test cannot be called from run?
#48823 (comment)

Correct. Although we could expose an API to get that list.

@cjihrig

@cjihrig Thanks. I understand thanks to your answer.
I look forward to seeing this issue resolved.

Should I send a PR to require options.files in the docs?

Should I send a PR to require options.files in the docs?

Making it a required argument would also require a code change, but first let's see how other collaborators feel. There isn't really a rush because it's a breaking change so it wouldn't be released until October anyway.

OK, thanks.
I would like to create a draft PR how other collaborators feel. Is it more appropriate to create new issue?

Should I send a PR to require options.files in the docs?

Making it a required argument would also require a code change, but first let's see how other collaborators feel. There isn't really a rush because it's a breaking change so it wouldn't be released until October anyway.

the next semver release will include glob support, and until we expose fs.glob (wich I am working on doing) - I dont think we should make this required

hoever we can easily detect if run is being called when process.env.NODE_TEST_CONTEXT is set and warn/stop the execution

It's sounds good for me. If you give me the sample, I will make a PR.

hoever we can easily detect if run is being called when process.env.NODE_TEST_CONTEXT is set and warn/stop the execution

@MoLow

I would like to work on this.
making options.files a required option in run() seems a good solution for this (ref ).
WDYT @MoLow.

It should not be required - it has a default value

test_runner: fix infinite loop when files are undefined in test runner

Navigation Menu

Repository navigation

[node:test] Infinite loop occurs when `files` is empty #48823

Version

Platform

Subsystem

What steps will reproduce the bug?

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

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

What do you see instead?

Additional information

2 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	const cwd = process.cwd();
	const hasUserSuppliedPattern = process.argv.length > 1;
	const patterns = hasUserSuppliedPattern ? ArrayPrototypeSlice(process.argv, 1) : [kDefaultPattern];

	let testFiles = files ?? createTestFileList();

	if (shard) {
	testFiles = ArrayPrototypeFilter(testFiles, (_, index) => index % shard.total === shard.index - 1);
	}

[node:test] Infinite loop occurs when files is empty #48823

Description

Version

Platform

Subsystem

What steps will reproduce the bug?

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

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

What do you see instead?

Additional information

Activity

MoLow commented on Jul 18, 2023

koh110 commented on Jul 18, 2023

MoLow commented on Jul 18, 2023

koh110 commented on Jul 18, 2023

MoLow commented on Jul 18, 2023

koh110 commented on Jul 18, 2023

koh110 commented on Jul 22, 2023

MoLow commented on Jul 22, 2023

koh110 commented on Jul 23, 2023

koshic commented on Jul 23, 2023

koh110 commented on Jul 28, 2023

MoLow commented on Jul 28, 2023

koh110 commented on Jul 28, 2023

koh110 commented on Jul 28, 2023

2 remaining items

cjihrig commented on Jul 29, 2023

koh110 commented on Jul 29, 2023

cjihrig commented on Jul 29, 2023

koh110 commented on Jul 29, 2023

cjihrig commented on Jul 29, 2023

koh110 commented on Jul 31, 2023

MoLow commented on Aug 2, 2023

MoLow commented on Aug 2, 2023

koh110 commented on Aug 6, 2023

pulkit-30 commented on Nov 28, 2023

MoLow commented on Nov 29, 2023

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

[node:test] Infinite loop occurs when `files` is empty #48823