Skip to content

Commit

Permalink
test: fix test-vm-sigint flakiness
Browse files Browse the repository at this point in the history
Set the `SIGUSR2` handler before spawning the child process to make sure
the signal is always handled.

Fixes: #7767
PR-URL: #7854
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
santigimeno authored and addaleax committed Aug 2, 2016
1 parent 1b24b37 commit 93ac2ea
Showing 1 changed file with 4 additions and 4 deletions.
8 changes: 4 additions & 4 deletions test/parallel/test-vm-sigint.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,14 @@ if (process.argv[2] === 'child') {
}

process.env.REPL_TEST_PPID = process.pid;
const child = spawn(process.execPath, [ __filename, 'child' ], {
stdio: [null, 'pipe', 'inherit']
});

process.on('SIGUSR2', common.mustCall(() => {
process.kill(child.pid, 'SIGINT');
}));

const child = spawn(process.execPath, [ __filename, 'child' ], {
stdio: [null, 'pipe', 'inherit']
});

child.on('close', common.mustCall((code, signal) => {
assert.strictEqual(signal, null);
assert.strictEqual(code, 0);
Expand Down

9 comments on commit 93ac2ea

@targos
Copy link
Member

@targos targos commented on 93ac2ea Aug 2, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#7767 is a 404

@addaleax
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@targos that looks like a Github issue (bug?) then… if the issue didn’t exist, GH wouldn’t convert #7767 into a link. Also, the title text for the link is “Issue title is private”, which does not really make sense to me?

@targos
Copy link
Member

@targos targos commented on 93ac2ea Aug 2, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK I see. Here is the explanation: isaacs/github#579 (comment)

@addaleax
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh. Yeah. @Trott’s Github account is gone…

@Trott
Copy link
Member

@Trott Trott commented on 93ac2ea Aug 2, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if anyone will be able to see this, but GitHub thinks I'm a robot (probably because of tooling used to extract CTC agenda from issues, attn @rvagg). I can still see #7767 but everyone else apparently gets a 404. Presumably, it will be restored when my account has been restored. I've contacted GitHub support.

@addaleax
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, looks good again, @Trott and #7767 and everything else seems back (I didn’t get a notification for @Trott’s comment here, so maybe it’s best to @mention @rvagg again).

@Trott
Copy link
Member

@Trott Trott commented on 93ac2ea Aug 2, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Yes, they reinstated me speedily, thankfully.)

@rvagg
Copy link
Member

@rvagg rvagg commented on 93ac2ea Aug 4, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

haha wut??? oh man, I can't believe how much drama that tool has caused you @Trott. Maybe you should remove the token, get rid of ~/Library/Application Support/iojs-tools/ and run node-meeting-agenda again to refresh it. Sorry .. I wish I could explain how this has been so bad.

@Trott
Copy link
Member

@Trott Trott commented on 93ac2ea Aug 4, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rvagg I asked GitHub support if there was something specific I could/should do to prevent this from happening again. They indicated the problem was on their side so it's entirely possible that the tool is blameless.

Please sign in to comment.