Coverage file writing hook is called too early during 'exit'.

[Fishrock123]

It has come to my attention while I was looking through coverage.nodejs.org that my commit intended to add coverage of a specific if branch in process.nextTick() was still now showing up in in the coverage reports.

The commit is f65a48f and the relevant lines can be found at

node/lib/internal/process/next_tick.js

Lines 143 to 145 in b514bd2

	// on the way out, don't bother. it won't get fired anyway.
	if (process._exiting)
	return;

.

It appears that in the coverage setup, the exit handler is added too early (that is, before test code is able to attach exit handlers (or at least in all possible cases)):

node/lib/internal/process/write-coverage.js

Lines 35 to 44 in b514bd2

	function setup() {
	const reallyReallyExit = process.reallyExit;
	process.reallyExit = function(code) {
	writeCoverage();
	reallyReallyExit(code);
	};
	process.on('exit', writeCoverage);
	}

I tried to patch this a couple ways but I wasn't able to figure out the ideal way to make this exit listener always fire last without adding a new C++ hook.

Here is a diff that I think should work, but just appears to make things worse:

diff --git a/lib/internal/process/write-coverage.js b/lib/internal/process/write-coverage.js
index 6bbc59a..be06a5e 100644
--- a/lib/internal/process/write-coverage.js
+++ b/lib/internal/process/write-coverage.js
@@ -40,7 +40,11 @@ function setup() {
     reallyReallyExit(code);
   };
 
-  process.on('exit', writeCoverage);
+  process.on('exit', () => {
+    process.on('exit', () => {
+      process.on('exit', writeCoverage);
+    });
+  });
 }
 
 exports.setup = setup;

Refs: #10856

cc @CurryKitten, @mhdawson, @addaleax

[addaleax]

Have you tried just dropping the isWritingCoverage guards? That might do the trick, and I’m not a hundred percent sure whether that is actually necessary for anything…