Using the bundler via API doesn't terminate all workers

[tripodsan]

🐛 bug report

We have test suite in parcel-plugin-htl that uses the Bundler via API to test the plugin. The testrunner doesn't complete if running more than 1 test. I think this has something to do with the Workers used in Bundler not being shutdown correctly.

🎛 Configuration (.babelrc, package.json, cli command)

• checkout parcel-plugin-htl
• run npm test

🤔 Expected Behavior

Tests complete without error and test runner terminates.

😯 Current Behavior

Test complete without error; but test runner hangs.

💁 Possible Solution

when using a new farm instead of the shared one, it works:

Bundler.js
...
-    this.farm = WorkerFarm.getShared(this.options);
+    this.farm = new WorkerFarm(this.options);

🔦 Context

Without this fixed, our plugin tests don't complete and we have to disable them on the CI.

💻 Code Sample

https://github.com/adobe/parcel-plugin-htl/blob/master/test/unit/testHTML.js#L66

🌍 Your Environment

Software	Version(s)
Parcel	parcel-bundler": "^1.9.6"
Node	8.11.3
npm/Yarn	npm 6.1.0
Operating System	macOS

[devongovett]

hmm it should stop unless there is a watcher. Just looked in the parcel core tests and it adds the --exit argument to mocha. See https://mochajs.org/#--exit----no-exit.

[tripodsan]

@devongovett well, but I think this is just a workaround. parcel should cleanup all the workers when used via API. the hanging tests indicate that it doesn't do so.

as mentioned above, using a ne WorkerFarm instead of the shared one solves the problem.

[devongovett]

hmm not sure what's going on here then. perhaps because there are multiple bundler instances in the tests something weird is happening.

[DeMoorJasper]

The workerfarm ends asynchronously, so perhaps you could try something like await bundler.stop()?
I've experienced and fixed a lot of issues with killing workers, but it should work.

[tripodsan]

@DeMoorJasper you're correct. when I change this to await this.stop() it works:

$ git diff
diff --git a/src/Bundler.js b/src/Bundler.js
index dd7d173..d02c93e 100644
--- a/src/Bundler.js
+++ b/src/Bundler.js
@@ -329,7 +329,7 @@ class Bundler extends EventEmitter {

       // If not in watch mode, stop the worker farm so we don't keep the process running.
       if (!this.watcher && this.options.killWorkers) {
-        this.stop();
+        await this.stop();
       }
     }
   }

[tripodsan]

I'll create a PR. see #1760

[DeMoorJasper]

Merged! Thanks :)