Flaky parallel/test-repl-history-navigation

[BridgeAR]

I added this test recently and it actually checks a race condition. Seems like the test itself is not solid enough tough.

20:40:29 not ok 1604 parallel/test-repl-history-navigation
20:40:29   ---
20:40:29   duration_ms: 3.189
20:40:29   severity: fail
20:40:29   exitcode: 1
20:40:29   stack: |-
20:40:29     Failed test # 6
20:40:29     Last outputs: [
20:40:29       �[32m"'> '"�[39m,
20:40:29       �[32m"'€'"�[39m,
20:40:29       �[32m"' // WOW'"�[39m,
20:40:29       �[32m"'> '"�[39m,
20:40:29       �[32m"'s'"�[39m,
20:40:29       �[32m"' // Always visible'"�[39m,
20:40:29       �[32m"'> '"�[39m,
20:40:29       �[32m"'€'"�[39m,
20:40:29       �[32m"' // WOW'"�[39m
20:40:29     ]
20:40:29     /usr/home/iojs/build/workspace/node-test-commit-freebsd/nodes/freebsd11-x64/test/parallel/test-repl-history-navigation.js:357
20:40:29                 throw e;
20:40:29                 ^
20:40:29     
20:40:29     AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
20:40:29     + actual - expected
20:40:29     
20:40:29     + ' // WOW'
20:40:29     - '> '
20:40:29         at Writable.write [as _write] (/usr/home/iojs/build/workspace/node-test-commit-freebsd/nodes/freebsd11-x64/test/parallel/test-repl-history-navigation.js:351:20)
20:40:29         at doWrite (_stream_writable.js:453:12)
20:40:29         at writeOrBuffer (_stream_writable.js:435:5)
20:40:29         at Writable.write (_stream_writable.js:326:11)
20:40:29         at Timeout._onTimeout (internal/repl/utils.js:231:19)
20:40:29         at listOnTimeout (internal/timers.js:551:17)
20:40:29         at processTimers (internal/timers.js:492:7) {
20:40:29       generatedMessage: true,
20:40:29       code: 'ERR_ASSERTION',
20:40:29       actual: ' // WOW',
20:40:29       expected: '> ',
20:40:29       operator: 'strictEqual'
20:40:29     }

[addaleax]

This seems easy to reproduce by changing common.platformTimeout(40) to common.platformTimeout(400) in the test.

Is there a chance we could remove the timeouts entirely? There shouldn’t be any in a REPL test, I think.

[BridgeAR]

@addaleax we never specified if a custom completion function may be async or not. Right now async is accepted and if someone types fast the completion could be triggered multiple times. In that case only the very last triggered completion should be accepted. We could pause the stream during the completion instead of using a counter but it's not possible to keep on typing while the completion is running in that case.

[addaleax]

@BridgeAR I assumed that completion functions may be async, but I don’t understand why that would need to be modelled though timers in the test?