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

Support for repeating test files with multiple env settings #586

Closed
coreyfarrell opened this issue Aug 8, 2019 · 8 comments
Closed

Support for repeating test files with multiple env settings #586

coreyfarrell opened this issue Aug 8, 2019 · 8 comments

Comments

@coreyfarrell
Copy link
Member

As you know I've been working on graceful-fs lately. I'd love the eliminate the ./test.js file which calls tap.spawn in a loop. The ability to tell tap to loop over a list of environment overrides would be extremely helpful to this. Mock package.json setup:

{
  "tap": {
    "envs": [
      {},
      {"id": "🐵", "env": {"TEST_GRACEFUL_FS_GLOBAL_PATCH": "1"}}
    ]
  }
}

In this case I'd want tap to serially run all test files twice, or npx tap test/close.js would run that one file twice. For test/close.js only I'd expect an output like:

 PASS  test/close.js 10 OK 800ms
 PASS  test/close.js [🐵] 10 OK 800ms

Where test/close.js would be run with normal env, test/close.js [🐵] would be run with the additional value added to process.env. For this feature I'd expect that lack of envs would do the same as envs = [{}], that is only run once with nothing added to the test id's or default env. I'd think it would normally be expected that at most one entry in envs exclude the id, not sure if that would necessarily have to be a hard rule. I'm not sure if envs = [] should be an error or simply ignored.

@isaacs
Copy link
Member

isaacs commented Aug 9, 2019

I've actually been kicking around in my head lately the idea of defining a test matrix, like you would on Travis or CircleCI.

I have a bunch of modules that I test in latest node, then run nave use 6, and run all the tests again. I'm not sure what the data layout should look like, but it probably wouldn't be too bad to just bundle nave and shell out to it to run tests if they want a node version we aren't currently using.

@coreyfarrell
Copy link
Member Author

How would nyc (specifically spawn-wrap) react to having multiple versions of node.js run within?

I have been thinking about giving nyc a way to bypass spawn-wrap. So an option where instead of running spawn-wrap the master nyc process would use NODE_OPTIONS to --require the absolute path of the nyc source which initializes a sub-process. Ref nodejs/node#27344

@isaacs
Copy link
Member

isaacs commented Aug 12, 2019

I'd probably just run each item in the matrix in a separate NYC invocation. Tap already manages NYC usage, so this wouldn't be too hard. But yeah, if you did nyc tap ... then it might get complicated. Though, istr that spawn-wrap runs the bin that was requested, so maybe it'd Just Work. That is, /v12.8.0/bin/node foo.js would get spawn-wrap munged to /v12.8.0/bin/node /path/to/wrap/node, which would execute /v12.8.0/bin/node /path/to/nyc/wrap.js foo.js, regardless of the execPath of the caller.

@coreyfarrell
Copy link
Member Author

In the case of graceful-fs I think it would be important for coverage from both env's to be merged otherwise it wouldn't reach 100% branch coverage.

@isaacs
Copy link
Member

isaacs commented Aug 12, 2019

Yeah, they'd be run with --clean=false, just like when tap runs in --changed or --watch mode.

@coreyfarrell
Copy link
Member Author

OK cool. So I guess the only other question is how would parallel work (tap -J)? Would each item in the matrix run serially or would the master tap process manage parallelizing matrix-wide?

@coreyfarrell
Copy link
Member Author

While writing a comment in the package.json overrides RFC I realized that it's probably a footgun to have tap able to run tests against multiple versions of node.js - node_modules installed by node 6 isn't always the same as node_modules installed by node 8 (I think this is already an issue for native modules). Not sure maybe you're suggesting that node_modules be deleted and reinstalled when switching node versions but then that would generally delete tap itself.

@isaacs
Copy link
Member

isaacs commented Sep 3, 2023

Yeah, this feels like a job for CI.

@isaacs isaacs closed this as completed Sep 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants