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

Recommended way of terminating subprocess #762

Closed
SuperFun99 opened this issue Jan 6, 2018 · 5 comments
Closed

Recommended way of terminating subprocess #762

SuperFun99 opened this issue Jan 6, 2018 · 5 comments

Comments

@SuperFun99
Copy link

SuperFun99 commented Jan 6, 2018

I'm using Mocha to test an Express app. In a global before(), Mocha starts the Express app using child_process.fork() and then sends the server HTTP requests. In a global after(), I terminate the Express app.

I had trouble at first when I was using subprocess.kill() to terminate it. nyc wasn't reporting any results because it apparently wasn't receiving them from the subprocesss.

After much experimentation I discovered that it works if I send the Express app a message subprocess.send("You can stop now") and then have it terminate itself.

Is this the recommended way to handle subprocesses? Very slick how nyc automatically covers them. It'd be nice though if the parent processes could just kill them willy-nilly without having to add code to the app being tested.

Details

  • Windows 10
  • Node v8.9.1
  • npm v5.6.0
  • nyc v11.4.1

For those that may come after, here's my current working setup:

app.js

/* ... */

// Create the server
const port = 3007;
app.set("port", port);
let server = http.createServer(app);

// Attach callback for onListening
server.on("listening", function() {
    console.log("ModelApp listening");
});

// Start listening
server.listen(port);

// Listen for messages from Mocha
process.on("message", (m) => {
    if (m === "Dear Express, Please exit. Sincerely, Mocha")
        process.exit();
});

test.js

/* ... */

let expressApp;

// Runs before all test suites
before(function(done) {
    expressApp = child_process.fork("app.js", [], {stdio:"pipe"});
        
    // Wait until the Express app is listening
    expressApp.stdout.on("data", (data) => {
        if (data.toString().includes("ModelApp listening"))
            done();
    });
});


// Runs after all test suites
after(function() {
    expressApp.send("Dear Express, Please exit. Sincerely, Mocha");
});


// Our tests
describe("My Tests", function() {
   /* Do tests */
});

package.json

    ...

    "scripts": {
        "test": "mocha",
        "coverage": "nyc mocha"
    },

    ...
@SuperFun99
Copy link
Author

The approach above is only part of the solution. One's Mocha code should also wait until the subprocess emits the exit event. (A short sleep after sending the message to exit also works but seems less robust.)

nyc was intermittently not picking up the coverage from the subprocess. My guess is sometimes Mocha terminated before the data from the subprocess finished writing to disk.

Is this what other people have had to do or am I way off base?


Here's a test.js that works with the files above.

test.js

/* ... */

let expressApp;

// Runs before all test suites
before(function(done) {
    expressApp = child_process.fork("app.js", [], {stdio:"pipe"});
        
    // Wait until the Express app is listening
    expressApp.stdout.on("data", data => {
        if (data.toString().includes("ModelApp listening"))
            done();
    });
});


// Runs after all test suites
after(async function() {
    await new Promise(resolve => {
        expressApp.on("exit", () => resolve());
        expressApp.send("Dear Express, Please exit. Sincerely, Mocha");
    });
});


// Our tests
describe("My Tests", function() {
   /* Do tests */
});

@bcoe
Copy link
Member

bcoe commented Jan 15, 2018

@SuperFun99 when testing web-apps, the approach I've tended to use is to stand up a server in the beforeAll statement and stop it in the afterAll statement... If your heart is set on running the server in a separate subprocess, I would try sending SIGINT rather than SIGKILL (if I recall my unix signal handling, SIGKILL can't be caught).

@SuperFun99
Copy link
Author

Thanks for the idea @bcoe but using subprocess.kill("SIGINT") results in coverage unknown, even if I wait for the subprocess to report it has exited and even with a lengthy sleep.

@bcoe
Copy link
Member

bcoe commented Jan 22, 2018

@SuperFun99 I believe this app uses restify rather than Express, but it's a good example of the approach I've used in the past for testing web-applications with coverage:

https://github.com/npm/oauth2-server-pg/blob/master/test/server.js#L14

while we could get your current approach working, I think it might be a bit easier to forego the child_process.fork -- it's easier to start the server in the same process because it puts you in a position to simply call server.end.

@Morikko
Copy link

Morikko commented Sep 13, 2018

I got also the problem to have the coverage when I was launching nyc over a long living server (express). I ended by finding a solution thanks to how we handled a sub process that returned the coverage.

I added the following code to listen to Ctrl+C command and to exit from the program instead of outside of it.

  process.on('SIGINT', function() {
    process.exit()
  })

As a result, nyc was printing the coverage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants