test: fix race condition in repl history test

[evanlucas]

Previously, there were cases when the end event could be emitted
before the flushHistory event. This change makes the next test fire
off after the specified event based on the test.

Ref: #2319

R=@Fishrock123

[cjihrig]

LGTM

[Fishrock123]

Interesting, I'll try to take a look into repl about this, nice patch though.

[evanlucas]

CI: https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/234/

[silverwind]

Fixed CI: https://jenkins-iojs.nodesource.com/job/node-test-pull-request/78/

[Fishrock123]

Are you sure this works? I don't think the repl actually emits this at all. Console.log something in on('close',...) / on(opts.event,...) and make sure it calls the same number as the number of tests.

[Fishrock123]

Alternatively, use this patch, which I am adding to #2356:

diff --git a/test/sequential/test-repl-persistent-history.js b/test/sequential/test-repl-persistent-history.js
index 8d550f6..27aca28 100644
--- a/test/sequential/test-repl-persistent-history.js
+++ b/test/sequential/test-repl-persistent-history.js
@@ -144,6 +144,13 @@ const tests = [{
 }];


+var testsNotRan = tests.length;
+
+process.on('beforeExit', function() {
+  assert.strictEqual(testsNotRan, 0);
+});
+
+
 // Copy our fixture to the tmp directory
 fs.createReadStream(historyFixturePath)
   .pipe(fs.createWriteStream(historyPath)).on('unpipe', runTest);
@@ -185,6 +192,7 @@ function runTest() {
     repl.on('close', function() {
       // Ensure everything that we expected was output
       assert.strictEqual(expected.length, 0);
+      testsNotRan--;
       setImmediate(runTest);
     });

EDIT: FIXED

[evanlucas]

The end event worked fine for me. To really see it, you can run with the NODE_DEBUG=repl. It makes me wonder if we should defer emitting close until after flushHistory though?

[Fishrock123]

It makes me wonder if we should defer emitting close until after flushHistory though?

That's what I was thinking.

I can't find where the 'end' event is emitted on the repl/readline though..

[Fishrock123]

We could also probably just listen to flushHistory, so long as we don't have a test that does anything after ENTER.

[Fishrock123]

@evanlucas can you see if Fishrock123@9a0fabb fixes it / LGTY?

[evanlucas]

I can this evening.

[evanlucas]

Closing in favor of #2356