c8 icon indicating copy to clipboard operation
c8 copied to clipboard

Follow-fork mode (usage with cluster module)

Open zbjornson opened this issue 5 years ago • 6 comments

  • Version: c8 6.0.1, Node.js v12.10.0
  • Platform: Win10 x64

This module doesn't appear to cover forked processes, including those created by Node.js' cluster module. Would it be possible to implement a follow-fork mode?

(Super happy you made this module, thank you!)

zbjornson avatar Nov 19 '19 00:11 zbjornson

@zbjornson to follow a fork, I think you would just need to make sure that the NODE_V8_COVERAGE flag persists in Node.js when a fork occurs, we do this for child_process.spawn, I wonder if we could potentially address this in Node?

bcoe avatar Nov 28 '19 21:11 bcoe

Spawn preserves environment variables by default, so there's something else that's preventing coverage data from being generated. I think this has to do with how signals are handled or with how cluster workers are killed when the master is killed, but I'm still a bit confused.

The way I've gotten coverage to work is by:

  • Installing this in the master process:
process.on("SIGINT", () => {
  for (const worker of Object.values(cluster.workers))
    worker.send("shutdown")
  process.exit();
})
  • Installing this in the worker process:
process.on("SIGINT", () => /* ignore */)
process.on("message", msg => {
  if (msg === "shutdown") process.disconnect()
})
  • Running the app with $env:NODE_V8_COVERAGE="somepath"; node myapp.js and ending it with ctrl+c.
  • Running c8 report.

If I just add the SIGINT+process.exit() handler to the master and do nothing to the worker, then I only get coverage for the master.

If I do all of the above but run c8 node myapp.js then I get no coverage files saved.

(Side note: I still don't know what the canonical way is to gracefully terminate a Node.js cluster. There are lots of old issues and not-very-trustworthy articles online about it but I haven't found the "one true way.")

zbjornson avatar Jan 14 '20 04:01 zbjornson

@zbjornson If you provide the env option to spawn then parent environment variables are not propagated anymore because env vars are not merged with process.env:

Use env to specify environment variables that will be visible to the new process, the default is process.env.

https://nodejs.org/api/child_process.html#child_process_child_process_spawn_command_args_options

I don't know how cluster mode launches the child processes.

aalexgabi avatar Apr 07 '20 16:04 aalexgabi

Hi, thx for developing and maintaining c8. Is there any update or workaround for this issue? I tried to add:

process.on("SIGINT", () =>{console.log("SIGINT"); process.exit(0)})
process.on("SIGTERM", () =>{console.log("SIGTERM"); process.exit(0)})

the signals are received (printed on console) but no coverage report is created. If I kill the process by itself with a timeout, then it works, eg

setTimeout(()=> process.exit(0), 10000)

but of course this is not a viable option...

I am using NodeJS version 16.14.2

arcuri82 avatar Apr 15 '22 08:04 arcuri82

Hi there!

I am running:

c8 --all --reporter=lcov lerna run test

And my integration test forks a sub process. The coverage of this sub process does not work. e.g. I run a mysql test and the mysql.js is not covered, although the test execute the file.

@zbjornson to follow a fork, I think you would just need to make sure that the NODE_V8_COVERAGE flag persists in Node.js when a fork occurs, we do this for child_process.spawn, I wonder if we could potentially address this in Node?

Does not work for me. I am forwarding process.env to the fork. NODE_V8_COVERAGE exists.

NODE_V8_COVERAGE /dev/project/coverage/tmp

What do I need to do to make it work? How can we help to add support for it? Forking a process in a test suite is a default use case.

Thanks!

Version: v9

kirrg001 avatar Jan 03 '24 11:01 kirrg001

As @zbjornson figured out, adding this code to the forked process works:

parent.js

child = fork(...)
child.kill()

child.js

process.on('SIGTERM', code => {
  process.disconnect();
  process.exit(0);
});

kirrg001 avatar Mar 27 '24 13:03 kirrg001