From 82b7894d3b539a02ad115d7287dcc429f6be5a94 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Thu, 15 Sep 2016 01:26:52 +0200 Subject: [PATCH] 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 PR-URL: https://github.com/nodejs/node/pull/8542 Reviewed-By: Rich Trott --- test/fixtures/tick-processor-base.js | 56 ++++++++++++++++ test/parallel/parallel.status | 6 +- test/parallel/test-tick-processor-builtin.js | 33 ++++++++++ test/parallel/test-tick-processor-cpp-core.js | 33 ++++++++++ test/parallel/test-tick-processor-unknown.js | 28 ++++++++ test/parallel/test-tick-processor.js | 66 ------------------- 6 files changed, 155 insertions(+), 67 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/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 diff --git a/test/parallel/test-tick-processor-builtin.js b/test/parallel/test-tick-processor-builtin.js new file mode 100644 index 00000000000000..9097a832fff60f --- /dev/null +++ b/test/parallel/test-tick-processor-builtin.js @@ -0,0 +1,33 @@ +'use strict'; +const common = require('../common'); +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..10e3fd35fd60c9 --- /dev/null +++ b/test/parallel/test-tick-processor-cpp-core.js @@ -0,0 +1,33 @@ +'use strict'; +const common = require('../common'); +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..15fc2d283f4063 --- /dev/null +++ b/test/parallel/test-tick-processor-unknown.js @@ -0,0 +1,28 @@ +'use strict'; +const common = require('../common'); +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 53dabeec638499..00000000000000 --- a/test/parallel/test-tick-processor.js +++ /dev/null @@ -1,66 +0,0 @@ -'use strict'; -const common = require('../common'); -const fs = require('fs'); -const assert = require('assert'); -const cp = require('child_process'); - -// 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(); -process.chdir(common.tmpDir); -// 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() { - for (var i = 0; i < 1000000; i++) { - i++; - } - setImmediate(function() { f(); }); - }; - setTimeout(function() { process.exit(0); }, 2000); - f();`); -if (common.isWindows || - common.isSunOS || - common.isAix || - common.isLinuxPPCBE || - common.isFreeBSD) { - common.skip('C++ symbols are not mapped for this os.'); - return; -} -runTest(/RunInDebugContext/, - `function f() { - require(\'vm\').runInDebugContext(\'Debug\'); - setImmediate(function() { f(); }); - }; - setTimeout(function() { process.exit(0); }, 2000); - f();`); - -runTest(/Builtin_DateNow/, - `function f() { - this.ts = Date.now(); - setImmediate(function() { new f(); }); - }; - setTimeout(function() { process.exit(0); }, 2000); - f();`); - -function runTest(pattern, code) { - cp.execFileSync(process.execPath, ['-prof', '-pe', code]); - var matches = fs.readdirSync(common.tmpDir); - - assert.strictEqual(matches.length, 1, 'There should be a single log file.'); - - 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); -}