From a4b9b173ace6b6d539ffa68688a2db3a13a34059 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Thu, 15 Sep 2016 01:26:52 +0200 Subject: [PATCH 1/6] test: make test-tick-processor.js non-flaky Wait for a sought-for symbol to appear instead of just hard-killing subprocesses at 2s timeout. Fix: #4427 --- test/parallel/test-tick-processor.js | 92 ++++++++++++++++++++-------- 1 file changed, 68 insertions(+), 24 deletions(-) diff --git a/test/parallel/test-tick-processor.js b/test/parallel/test-tick-processor.js index 53dabeec638499..367cd12146b9e4 100644 --- a/test/parallel/test-tick-processor.js +++ b/test/parallel/test-tick-processor.js @@ -3,6 +3,7 @@ const common = require('../common'); const fs = require('fs'); const assert = require('assert'); const cp = require('child_process'); +const path = require('path'); // TODO(mhdawson) Currently the test-tick-processor functionality in V8 // depends on addresses being smaller than a full 64 bits. Aix supports @@ -15,52 +16,95 @@ if (common.isAix) { } common.refreshTmpDir(); -process.chdir(common.tmpDir); + +const tests = []; + +const LOG_FILE = path.join(common.tmpDir, 'tick-processor.log'); +const RETRY_TIMEOUT = 750; + // Unknown checked for to prevent flakiness, if pattern is not found, // then a large number of unknown ticks should be present -runTest(/LazyCompile.*\[eval\]:1|.*% UNKNOWN/, - `function f() { +tests.push({ + pattern: /LazyCompile.*\[eval\]:1|.*% UNKNOWN/, + code: `function f() { for (var i = 0; i < 1000000; i++) { i++; } setImmediate(function() { f(); }); }; - setTimeout(function() { process.exit(0); }, 2000); - f();`); + f();` +}); + if (common.isWindows || common.isSunOS || common.isAix || common.isLinuxPPCBE || common.isFreeBSD) { common.skip('C++ symbols are not mapped for this os.'); - return; +} else { } -runTest(/RunInDebugContext/, - `function f() { +tests.push({ + pattern: /RunInDebugContext/, + code: `function f() { require(\'vm\').runInDebugContext(\'Debug\'); setImmediate(function() { f(); }); }; - setTimeout(function() { process.exit(0); }, 2000); - f();`); + f();` +}); -runTest(/Builtin_DateNow/, - `function f() { +tests.push({ + pattern: /Builtin_DateNow/, + code: `function f() { this.ts = Date.now(); setImmediate(function() { new f(); }); }; - setTimeout(function() { process.exit(0); }, 2000); - f();`); + f();` +}); + +runTest(); + +function runTest() { + if (tests.length === 0) + return; + + const test = tests.shift(); + + const proc = cp.spawn(process.execPath, [ + '--no_logfile_per_isolate', + `--logfile=${LOG_FILE}`, + '--prof', + '-pe', test.code + ], { + stdio: [ null, null, 'inherit' ] + }); + + // Try to match after timeout + setTimeout(() => { + match(test.pattern, proc, runTest); + }, RETRY_TIMEOUT); +} + +function match(pattern, parent, cb) { + const proc = cp.spawn(process.execPath, [ + '--prof-process', + '--call-graph-size=10', + LOG_FILE + ], { + stdio: [ null, 'pipe', 'inherit' ] + }); -function runTest(pattern, code) { - cp.execFileSync(process.execPath, ['-prof', '-pe', code]); - var matches = fs.readdirSync(common.tmpDir); + let out = ''; + proc.stdout.on('data', (chunk) => { + out += chunk; + }); + proc.stdout.on('end', () => { + // Retry after timeout + if (!pattern.test(out)) + return setTimeout(() => match(pattern, parent, cb), RETRY_TIMEOUT); - assert.strictEqual(matches.length, 1, 'There should be a single log file.'); + parent.kill('SIGTERM'); - var log = matches[0]; - var out = cp.execSync(process.execPath + - ' --prof-process --call-graph-size=10 ' + log, - {encoding: 'utf8'}); - assert(pattern.test(out), `${pattern} not matching ${out}`); - fs.unlinkSync(log); + fs.unlinkSync(LOG_FILE); + cb(null); + }); } From feee308de532cc4837eb20d9f0cb018d1bcb9344 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Thu, 15 Sep 2016 01:55:55 +0200 Subject: [PATCH 2/6] fix --- test/parallel/test-tick-processor.js | 34 ++++++++++++++-------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/test/parallel/test-tick-processor.js b/test/parallel/test-tick-processor.js index 367cd12146b9e4..19efa225549455 100644 --- a/test/parallel/test-tick-processor.js +++ b/test/parallel/test-tick-processor.js @@ -42,24 +42,24 @@ if (common.isWindows || common.isFreeBSD) { common.skip('C++ symbols are not mapped for this os.'); } else { -} -tests.push({ - pattern: /RunInDebugContext/, - code: `function f() { - require(\'vm\').runInDebugContext(\'Debug\'); - setImmediate(function() { f(); }); - }; - f();` -}); + tests.push({ + pattern: /RunInDebugContext/, + code: `function f() { + require(\'vm\').runInDebugContext(\'Debug\'); + setImmediate(function() { f(); }); + }; + f();` + }); -tests.push({ - pattern: /Builtin_DateNow/, - code: `function f() { - this.ts = Date.now(); - setImmediate(function() { new f(); }); - }; - f();` -}); + tests.push({ + pattern: /Builtin_DateNow/, + code: `function f() { + this.ts = Date.now(); + setImmediate(function() { new f(); }); + }; + f();` + }); +} runTest(); From 3ccc39cf6fc427878626905f6c156081bb177699 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Thu, 15 Sep 2016 02:07:18 +0200 Subject: [PATCH 3/6] lint --- test/parallel/test-tick-processor.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/parallel/test-tick-processor.js b/test/parallel/test-tick-processor.js index 19efa225549455..2b7a214af72801 100644 --- a/test/parallel/test-tick-processor.js +++ b/test/parallel/test-tick-processor.js @@ -1,7 +1,6 @@ 'use strict'; const common = require('../common'); const fs = require('fs'); -const assert = require('assert'); const cp = require('child_process'); const path = require('path'); From 6142cc4fdd53ed7f3b28b2d761cc7e1cbbf2691e Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Thu, 15 Sep 2016 09:44:34 +0200 Subject: [PATCH 4/6] split --- test/fixtures/tick-processor-base.js | 56 +++++++++ test/parallel/test-tick-processor-builtin.js | 35 ++++++ test/parallel/test-tick-processor-cpp-core.js | 35 ++++++ test/parallel/test-tick-processor-unknown.js | 30 +++++ test/parallel/test-tick-processor.js | 109 ------------------ 5 files changed, 156 insertions(+), 109 deletions(-) create mode 100644 test/fixtures/tick-processor-base.js create mode 100644 test/parallel/test-tick-processor-builtin.js create mode 100644 test/parallel/test-tick-processor-cpp-core.js create mode 100644 test/parallel/test-tick-processor-unknown.js delete mode 100644 test/parallel/test-tick-processor.js diff --git a/test/fixtures/tick-processor-base.js b/test/fixtures/tick-processor-base.js new file mode 100644 index 00000000000000..8e0005ff7d8b99 --- /dev/null +++ b/test/fixtures/tick-processor-base.js @@ -0,0 +1,56 @@ +'use strict'; +const common = require('../common'); +const fs = require('fs'); +const cp = require('child_process'); +const path = require('path'); + +common.refreshTmpDir(); + +const LOG_FILE = path.join(common.tmpDir, 'tick-processor.log'); +const RETRY_TIMEOUT = 750; + +function runTest(test) { + const proc = cp.spawn(process.execPath, [ + '--no_logfile_per_isolate', + '--logfile=-', + '--prof', + '-pe', test.code + ], { + stdio: [ null, 'pipe', 'inherit' ] + }); + + let ticks = ''; + proc.stdout.on('data', chunk => ticks += chunk); + + // Try to match after timeout + setTimeout(() => { + match(test.pattern, proc, () => ticks); + }, RETRY_TIMEOUT); +} + +function match(pattern, parent, ticks) { + // Store current ticks log + fs.writeFileSync(LOG_FILE, ticks()); + + const proc = cp.spawn(process.execPath, [ + '--prof-process', + '--call-graph-size=10', + LOG_FILE + ], { + stdio: [ null, 'pipe', 'inherit' ] + }); + + let out = ''; + proc.stdout.on('data', chunk => out += chunk); + proc.stdout.on('end', () => { + // Retry after timeout + if (!pattern.test(out)) + return setTimeout(() => match(pattern, parent, ticks), RETRY_TIMEOUT); + + parent.kill('SIGTERM'); + + fs.unlinkSync(LOG_FILE); + }); +} + +exports.runTest = runTest; diff --git a/test/parallel/test-tick-processor-builtin.js b/test/parallel/test-tick-processor-builtin.js new file mode 100644 index 00000000000000..7a0e2f7ef9dcb8 --- /dev/null +++ b/test/parallel/test-tick-processor-builtin.js @@ -0,0 +1,35 @@ +'use strict'; +const common = require('../common'); +const fs = require('fs'); +const cp = require('child_process'); +const path = require('path'); + +// TODO(mhdawson) Currently the test-tick-processor functionality in V8 +// depends on addresses being smaller than a full 64 bits. Aix supports +// the full 64 bits and the result is that it does not process the +// addresses correctly and runs out of memory +// Disabling until we get a fix upstreamed into V8 +if (common.isAix) { + common.skip('Aix address range too big for scripts.'); + return; +} + +const base = require(path.join(common.fixturesDir, 'tick-processor-base.js')); + +if (common.isWindows || + common.isSunOS || + common.isAix || + common.isLinuxPPCBE || + common.isFreeBSD) { + common.skip('C++ symbols are not mapped for this os.'); + return; +} + +base.runTest({ + pattern: /Builtin_DateNow/, + code: `function f() { + this.ts = Date.now(); + setImmediate(function() { new f(); }); + }; + f();` +}); diff --git a/test/parallel/test-tick-processor-cpp-core.js b/test/parallel/test-tick-processor-cpp-core.js new file mode 100644 index 00000000000000..f108a3c15679cb --- /dev/null +++ b/test/parallel/test-tick-processor-cpp-core.js @@ -0,0 +1,35 @@ +'use strict'; +const common = require('../common'); +const fs = require('fs'); +const cp = require('child_process'); +const path = require('path'); + +// TODO(mhdawson) Currently the test-tick-processor functionality in V8 +// depends on addresses being smaller than a full 64 bits. Aix supports +// the full 64 bits and the result is that it does not process the +// addresses correctly and runs out of memory +// Disabling until we get a fix upstreamed into V8 +if (common.isAix) { + common.skip('Aix address range too big for scripts.'); + return; +} + +const base = require(path.join(common.fixturesDir, 'tick-processor-base.js')); + +if (common.isWindows || + common.isSunOS || + common.isAix || + common.isLinuxPPCBE || + common.isFreeBSD) { + common.skip('C++ symbols are not mapped for this os.'); + return; +} + +base.runTest({ + pattern: /RunInDebugContext/, + code: `function f() { + require(\'vm\').runInDebugContext(\'Debug\'); + setImmediate(function() { f(); }); + }; + f();` +}); diff --git a/test/parallel/test-tick-processor-unknown.js b/test/parallel/test-tick-processor-unknown.js new file mode 100644 index 00000000000000..05883f18d4be39 --- /dev/null +++ b/test/parallel/test-tick-processor-unknown.js @@ -0,0 +1,30 @@ +'use strict'; +const common = require('../common'); +const fs = require('fs'); +const cp = require('child_process'); +const path = require('path'); + +// TODO(mhdawson) Currently the test-tick-processor functionality in V8 +// depends on addresses being smaller than a full 64 bits. Aix supports +// the full 64 bits and the result is that it does not process the +// addresses correctly and runs out of memory +// Disabling until we get a fix upstreamed into V8 +if (common.isAix) { + common.skip('Aix address range too big for scripts.'); + return; +} + +const base = require(path.join(common.fixturesDir, 'tick-processor-base.js')); + +// Unknown checked for to prevent flakiness, if pattern is not found, +// then a large number of unknown ticks should be present +base.runTest({ + pattern: /LazyCompile.*\[eval\]:1|.*% UNKNOWN/, + code: `function f() { + for (var i = 0; i < 1000000; i++) { + i++; + } + setImmediate(function() { f(); }); + }; + f();` +}); diff --git a/test/parallel/test-tick-processor.js b/test/parallel/test-tick-processor.js deleted file mode 100644 index 2b7a214af72801..00000000000000 --- a/test/parallel/test-tick-processor.js +++ /dev/null @@ -1,109 +0,0 @@ -'use strict'; -const common = require('../common'); -const fs = require('fs'); -const cp = require('child_process'); -const path = require('path'); - -// TODO(mhdawson) Currently the test-tick-processor functionality in V8 -// depends on addresses being smaller than a full 64 bits. Aix supports -// the full 64 bits and the result is that it does not process the -// addresses correctly and runs out of memory -// Disabling until we get a fix upstreamed into V8 -if (common.isAix) { - common.skip('Aix address range too big for scripts.'); - return; -} - -common.refreshTmpDir(); - -const tests = []; - -const LOG_FILE = path.join(common.tmpDir, 'tick-processor.log'); -const RETRY_TIMEOUT = 750; - -// Unknown checked for to prevent flakiness, if pattern is not found, -// then a large number of unknown ticks should be present -tests.push({ - pattern: /LazyCompile.*\[eval\]:1|.*% UNKNOWN/, - code: `function f() { - for (var i = 0; i < 1000000; i++) { - i++; - } - setImmediate(function() { f(); }); - }; - f();` -}); - -if (common.isWindows || - common.isSunOS || - common.isAix || - common.isLinuxPPCBE || - common.isFreeBSD) { - common.skip('C++ symbols are not mapped for this os.'); -} else { - tests.push({ - pattern: /RunInDebugContext/, - code: `function f() { - require(\'vm\').runInDebugContext(\'Debug\'); - setImmediate(function() { f(); }); - }; - f();` - }); - - tests.push({ - pattern: /Builtin_DateNow/, - code: `function f() { - this.ts = Date.now(); - setImmediate(function() { new f(); }); - }; - f();` - }); -} - -runTest(); - -function runTest() { - if (tests.length === 0) - return; - - const test = tests.shift(); - - const proc = cp.spawn(process.execPath, [ - '--no_logfile_per_isolate', - `--logfile=${LOG_FILE}`, - '--prof', - '-pe', test.code - ], { - stdio: [ null, null, 'inherit' ] - }); - - // Try to match after timeout - setTimeout(() => { - match(test.pattern, proc, runTest); - }, RETRY_TIMEOUT); -} - -function match(pattern, parent, cb) { - const proc = cp.spawn(process.execPath, [ - '--prof-process', - '--call-graph-size=10', - LOG_FILE - ], { - stdio: [ null, 'pipe', 'inherit' ] - }); - - let out = ''; - proc.stdout.on('data', (chunk) => { - out += chunk; - }); - proc.stdout.on('end', () => { - // Retry after timeout - if (!pattern.test(out)) - return setTimeout(() => match(pattern, parent, cb), RETRY_TIMEOUT); - - parent.kill('SIGTERM'); - - fs.unlinkSync(LOG_FILE); - cb(null); - }); -} From 22cff8c85928266cca076e71557d6a256a501625 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Thu, 15 Sep 2016 09:53:19 +0200 Subject: [PATCH 5/6] lint --- test/parallel/test-tick-processor-builtin.js | 2 -- test/parallel/test-tick-processor-cpp-core.js | 2 -- test/parallel/test-tick-processor-unknown.js | 2 -- 3 files changed, 6 deletions(-) diff --git a/test/parallel/test-tick-processor-builtin.js b/test/parallel/test-tick-processor-builtin.js index 7a0e2f7ef9dcb8..9097a832fff60f 100644 --- a/test/parallel/test-tick-processor-builtin.js +++ b/test/parallel/test-tick-processor-builtin.js @@ -1,7 +1,5 @@ 'use strict'; const common = require('../common'); -const fs = require('fs'); -const cp = require('child_process'); const path = require('path'); // TODO(mhdawson) Currently the test-tick-processor functionality in V8 diff --git a/test/parallel/test-tick-processor-cpp-core.js b/test/parallel/test-tick-processor-cpp-core.js index f108a3c15679cb..10e3fd35fd60c9 100644 --- a/test/parallel/test-tick-processor-cpp-core.js +++ b/test/parallel/test-tick-processor-cpp-core.js @@ -1,7 +1,5 @@ 'use strict'; const common = require('../common'); -const fs = require('fs'); -const cp = require('child_process'); const path = require('path'); // TODO(mhdawson) Currently the test-tick-processor functionality in V8 diff --git a/test/parallel/test-tick-processor-unknown.js b/test/parallel/test-tick-processor-unknown.js index 05883f18d4be39..15fc2d283f4063 100644 --- a/test/parallel/test-tick-processor-unknown.js +++ b/test/parallel/test-tick-processor-unknown.js @@ -1,7 +1,5 @@ 'use strict'; const common = require('../common'); -const fs = require('fs'); -const cp = require('child_process'); const path = require('path'); // TODO(mhdawson) Currently the test-tick-processor functionality in V8 From 2e785fd7181e0dff3f2b7f32c1fd5126523d55bd Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Thu, 15 Sep 2016 23:23:13 +0200 Subject: [PATCH 6/6] flaky --- test/parallel/parallel.status | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/parallel/parallel.status b/test/parallel/parallel.status index f2536b6b68e376..02fb2983c6669e 100644 --- a/test/parallel/parallel.status +++ b/test/parallel/parallel.status @@ -7,13 +7,17 @@ prefix parallel [true] # This section applies to all platforms [$system==win32] -test-tick-processor : PASS,FLAKY [$system==linux] test-tick-processor : PASS,FLAKY [$system==macos] +[$arch==arm || $arch==arm64] +test-tick-processor-builtin : PASS,FLAKY +test-tick-processor-cpp-core : PASS,FLAKY +test-tick-processor-unknown : PASS,FLAKY + [$system==solaris] # Also applies to SmartOS test-debug-signal-cluster : PASS,FLAKY