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

--findRelatedTests for node:test #42992

Closed
bestickley opened this issue May 6, 2022 · 10 comments
Closed

--findRelatedTests for node:test #42992

bestickley opened this issue May 6, 2022 · 10 comments
Labels
feature request Issues that request new features to be added to Node.js. stale test_runner Issues and PRs related to the test runner subsystem.

Comments

@bestickley
Copy link

What is the problem this feature will solve?

I use lintstaged to run only the unit tests related to changed source files on each commit. I currently use jest with their CLI flag: --findRelatedTests (https://jestjs.io/docs/cli#--findrelatedtests-spaceseparatedlistofsourcefiles). Could something similar be added to node:test?

What is the feature you are proposing to solve the problem?

--findRelatedTests CLI flag

What alternatives have you considered?

Sticking with Jest or creating my own helper function.

@bestickley bestickley added the feature request Issues that request new features to be added to Node.js. label May 6, 2022
@aduh95
Copy link
Contributor

aduh95 commented May 6, 2022

/cc @nodejs/test_runner

@aduh95 aduh95 added the test_runner Issues and PRs related to the test runner subsystem. label May 6, 2022
@ljharb
Copy link
Member

ljharb commented May 7, 2022

How would node determine this?

jest does it because it has a bunch of complex code to build a dependency map - but node would have to actively parse all your code to find the static requires/imports, and would have to evaluate your code to find the dynamic ones. This seems like something way out of scope for node core to do.

@bestickley
Copy link
Author

I'd expect node to determine the dependency map. If you think this feature is outside of node core, that's fair. Would still love to see this in node core if not now, maybe later on.

@ljharb
Copy link
Member

ljharb commented May 8, 2022

I think that capability would need to land first, separately, before the test runner could use it.

@MoLow
Copy link
Member

MoLow commented Jun 19, 2022

@ljharb @bestickley basically this PR kind of does that,
when running inside describe it first maps all subtests, then runs only top-level tests, so it probably wont require a lot of modification to extract a list of all tests before they run (only when using describe and it)

if (parent === root) {
subtest.start();
}

@ljharb
Copy link
Member

ljharb commented Jun 19, 2022

"only when using describe and it" wouldn't be an acceptable caveat, imo, since all supported testing styles need to be usable with a feature.

@MoLow
Copy link
Member

MoLow commented Jun 19, 2022

"only when using describe and it" wouldn't be an acceptable caveat, imo, since all supported testing styles need to be usable with a feature.

I agree. I was just pointing out my PR lays the ground for that

@kmannislands
Copy link

jest does it because it has a bunch of complex code to build a dependency map - but node would have to actively parse all your code to find the static requires/imports, and would have to evaluate your code to find the dynamic ones. This seems like something way out of scope for node core to do.

Totally agree, this would be very complex. I, for one, am interested in a simpler, more reliable test runner after having struggled for long enough with jest & the like. It's my hope that node:test will avoid complexity bombs like this and provide a simple to understand substrate on which to build.

I think this feature would be more appropriate as a userland implementation. It's fairly complex and I think would also need to be too opinionated about what your project looks like (CJS? ESM? Typescript?), path resolution, and how changes should be identified. You can introduce a ton of config like jest does but that's best left to users IMO.

A rough sketch of how I'd do it:

  • Write a custom test entry using run that determines a list of test files
  • Use something like es module lexer to recursively parse imports from those test files to their dependencies, dependencies of dependencies etc to build up a module dependency graph. Dynamic imports would be a significant challenge here.
  • Identify changed files by parsing git state like the index file (it's fairly straight-forward to do) or using a node git client to do so.
  • Traverse the dependency graph to find test file leaves related to changed module nodes. Only note here is that module graphs aren't necessarily acyclic so some cation would be needed in the traversal implementation.
  • Pass the list of test file leafs related to changed files to run instead of the full list.

@github-actions
Copy link
Contributor

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

@github-actions github-actions bot added the stale label May 11, 2023
@github-actions
Copy link
Contributor

There has been no activity on this feature request and it is being closed. If you feel closing this issue is not the right thing to do, please leave a comment.

For more information on how the project manages feature requests, please consult the feature request management document.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jun 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. stale test_runner Issues and PRs related to the test runner subsystem.
Projects
None yet
Development

No branches or pull requests

5 participants