From 80dc4172f015ff7c0ad4e48cdd95e39e69ccfd83 Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Wed, 26 Sep 2018 09:29:08 +0200
Subject: [PATCH] test: fix tick-processor tests

This reduces the runtime significantly and fixes the tests to run on
the CI. It is significantly more robust by accepting some fallbacks.
Therefore they are moved out of pummel to sequential.
---
 test/common/tick-processor.js                 | 118 ++++++++++++++++++
 .../sequential/test-tick-processor-builtin.js |  20 +++
 .../test-tick-processor-cpp-core.js           |  16 +++
 ...test-tick-processor-polyfill-brokenfile.js |  31 ++---
 .../test-tick-processor-preprocess-flag.js    |  15 +++
 .../test-tick-processor-builtin.js            |  21 ----
 .../test-tick-processor-cpp-core.js           |  21 ----
 .../test-tick-processor-preprocess-flag.js    |  22 ----
 .../test-tick-processor-unknown.js            |  28 -----
 test/tick-processor/testcfg.py                |   6 -
 test/tick-processor/tick-processor-base.js    |  65 ----------
 test/tick-processor/util.js                   |  18 ---
 12 files changed, 185 insertions(+), 196 deletions(-)
 create mode 100644 test/common/tick-processor.js
 create mode 100644 test/sequential/test-tick-processor-builtin.js
 create mode 100644 test/sequential/test-tick-processor-cpp-core.js
 rename test/{tick-processor => sequential}/test-tick-processor-polyfill-brokenfile.js (77%)
 create mode 100644 test/sequential/test-tick-processor-preprocess-flag.js
 delete mode 100644 test/tick-processor/test-tick-processor-builtin.js
 delete mode 100644 test/tick-processor/test-tick-processor-cpp-core.js
 delete mode 100644 test/tick-processor/test-tick-processor-preprocess-flag.js
 delete mode 100644 test/tick-processor/test-tick-processor-unknown.js
 delete mode 100644 test/tick-processor/testcfg.py
 delete mode 100644 test/tick-processor/tick-processor-base.js
 delete mode 100644 test/tick-processor/util.js

diff --git a/test/common/tick-processor.js b/test/common/tick-processor.js
new file mode 100644
index 00000000000000..3f36379aeef5e9
--- /dev/null
+++ b/test/common/tick-processor.js
@@ -0,0 +1,118 @@
+'use strict';
+
+const {
+  isWindows,
+  isSunOS,
+  isAIX,
+  isLinuxPPCBE,
+  isFreeBSD,
+  skip
+} = require('../common');
+
+const fs = require('fs');
+const cp = require('child_process');
+const path = require('path');
+
+const tmpdir = require('./tmpdir');
+tmpdir.refresh();
+
+const LOG_FILE = path.join(tmpdir.path, 'tick-processor.log');
+const START_PROF_PROCESS_TIMEOUT = 400;
+
+let running = 0;
+let ran = 0;
+
+// The tick processor is not supported on all platforms.
+function isCPPSymbolsNotMapped() {
+  if (isWindows || isSunOS || isAIX || isLinuxPPCBE || isFreeBSD) {
+    skip('C++ symbols are not mapped for this OS.');
+    return true;
+  }
+  return false;
+}
+
+function runTest(test) {
+  if (isCPPSymbolsNotMapped()) {
+    return;
+  }
+
+  ran++;
+  running = 0;
+  const proc = cp.spawn(process.execPath, [
+    '--no_logfile_per_isolate',
+    '--logfile=-',
+    '--prof',
+    '-pe', test.code
+  ], {
+    stdio: [ 'ignore', 'pipe', 'inherit' ]
+  });
+
+  let ticks = '';
+  proc.stdout.on('data', (chunk) => ticks += chunk);
+
+  // Check if the pattern is already there or not.
+  setTimeout(() => {
+    if (running === 0) {
+      match(test, proc, () => ticks);
+    }
+  }, START_PROF_PROCESS_TIMEOUT);
+
+  proc.on('exit', () => {
+    running++;
+    if (running === 1) {
+      match(test, proc, () => ticks);
+    }
+  });
+}
+
+function match(test, parent, ticks) {
+  running++;
+
+  const { pattern, profProcessFlags: flags = [] } = test;
+  // Store current ticks log
+  fs.writeFileSync(LOG_FILE, ticks());
+
+  const proc = cp.spawn(process.execPath, [
+    '--prof-process',
+    '--call-graph-size=10',
+    ...flags,
+    LOG_FILE
+  ], {
+    stdio: [ 'ignore', 'pipe', 'inherit' ]
+  });
+
+  let out = '';
+
+  proc.stdout.on('data', (chunk) => out += chunk);
+  proc.stdout.once('end', () => {
+    proc.once('exit', () => {
+      fs.unlinkSync(LOG_FILE);
+
+      // Retry after timeout
+      if (!pattern.test(out)) {
+        running--;
+        // If the profiling is done without match, try up to ten times again
+        // and then fail.
+        if (running === 1) {
+          if (ran < 10) {
+            return runTest(test);
+          }
+          console.error(out);
+          throw new Error(`${pattern} Failed`);
+        }
+        return;
+      }
+
+      parent.stdout.removeAllListeners();
+      parent.kill();
+    });
+
+    proc.stdout.removeAllListeners();
+    proc.kill();
+  });
+}
+
+module.exports = {
+  runTest,
+  isCPPSymbolsNotMapped
+};
diff --git a/test/sequential/test-tick-processor-builtin.js b/test/sequential/test-tick-processor-builtin.js
new file mode 100644
index 00000000000000..d48a02f79d9538
--- /dev/null
+++ b/test/sequential/test-tick-processor-builtin.js
@@ -0,0 +1,20 @@
+'use strict';
+require('../common');
+
+const base = require('../common/tick-processor');
+
+base.runTest({
+  // If the C++ symbols are not detected by the OS, no JS is going to be
+  // executed.
+  pattern: /Builtin: ObjectKeys| \d    [0-6]\.\d%    [0-6]\.\d%  JavaScript/,
+  code: `let add = 0;
+         const obj = { a: 5 };
+         function f() {
+           for (var i = 0; i < 4e6; i++) {
+             obj.a = i;
+             add += Object.keys(obj).length;
+           }
+           return add;
+         };
+         f();`
+});
diff --git a/test/sequential/test-tick-processor-cpp-core.js b/test/sequential/test-tick-processor-cpp-core.js
new file mode 100644
index 00000000000000..896ef25d41e5ba
--- /dev/null
+++ b/test/sequential/test-tick-processor-cpp-core.js
@@ -0,0 +1,16 @@
+'use strict';
+require('../common');
+
+const base = require('../common/tick-processor');
+
+base.runTest({
+  // If the C++ symbols are not mapped to the OS they can end up as `UNKNOWN` or
+  // no JS is executed at all.
+  pattern: /MakeContext|\d\d\.\d%  UNKNOWN|0    0\.0%    0\.0%  JavaScript|[7-9]\d\.\d%   [7-9]\d\.\d%  write/,
+  code: `function f() {
+           for (var i = 0; i < 5e2; i++) {
+             require('vm').createContext({});
+           }
+         };
+         f();`
+});
diff --git a/test/tick-processor/test-tick-processor-polyfill-brokenfile.js b/test/sequential/test-tick-processor-polyfill-brokenfile.js
similarity index 77%
rename from test/tick-processor/test-tick-processor-polyfill-brokenfile.js
rename to test/sequential/test-tick-processor-polyfill-brokenfile.js
index f61bdbe9a12510..d4d7ff5520c585 100644
--- a/test/tick-processor/test-tick-processor-polyfill-brokenfile.js
+++ b/test/sequential/test-tick-processor-polyfill-brokenfile.js
@@ -1,37 +1,39 @@
 'use strict';
-const common = require('../common');
-const { isCPPSymbolsNotMapped } = require('./util');
-const tmpdir = require('../common/tmpdir');
-tmpdir.refresh();
+const { skip } = require('../common');
 
-if (!common.enoughTestCpu)
-  common.skip('test is CPU-intensive');
+const { isCPPSymbolsNotMapped } = require('../common/tick-processor');
 
 if (isCPPSymbolsNotMapped) {
-  common.skip('C++ symbols are not mapped for this OS.');
+  skip('C++ symbols are not mapped for this OS.');
 }
 
+const tmpdir = require('../common/tmpdir');
+tmpdir.refresh();
+
 // This test will produce a broken profile log.
 // ensure prof-polyfill not stuck in infinite loop
 // and success process
 
-
 const assert = require('assert');
 const { spawn, spawnSync } = require('child_process');
 const path = require('path');
 const { writeFileSync } = require('fs');
 
 const LOG_FILE = path.join(tmpdir.path, 'tick-processor.log');
-const RETRY_TIMEOUT = 150;
+const RETRY_TIMEOUT = 250;
 const BROKEN_PART = 'tick,';
 const WARN_REG_EXP = /\(node:\d+\) \[BROKEN_PROFILE_FILE] Warning: Profile file .* is broken/;
 const WARN_DETAIL_REG_EXP = /".*tick," at the file end is broken/;
 
-const code = `function f() {
-           this.ts = Date.now();
-           setImmediate(function() { new f(); });
-         };
-         f();`;
+const code = `
+  let add = 0;
+  function f() {
+    for (var i = 0; i < 1e3; i++) {
+      add += Date.now();
+    }
+    return add;
+  };
+  f();`;
 
 const proc = spawn(process.execPath, [
   '--no_logfile_per_isolate',
@@ -45,7 +47,6 @@ const proc = spawn(process.execPath, [
 let ticks = '';
 proc.stdout.on('data', (chunk) => ticks += chunk);
 
-
 function runPolyfill(content) {
   proc.kill();
   content += BROKEN_PART;
diff --git a/test/sequential/test-tick-processor-preprocess-flag.js b/test/sequential/test-tick-processor-preprocess-flag.js
new file mode 100644
index 00000000000000..d0a886768f0d8f
--- /dev/null
+++ b/test/sequential/test-tick-processor-preprocess-flag.js
@@ -0,0 +1,15 @@
+'use strict';
+require('../common');
+
+const base = require('../common/tick-processor');
+
+base.runTest({
+  pattern: /^{/,
+  code: `function f() {
+          for (var i = 0; i < 5e2; i++) {
+            require('vm').createContext({});
+          }
+         };
+         f();`,
+  profProcessFlags: ['--preprocess']
+});
diff --git a/test/tick-processor/test-tick-processor-builtin.js b/test/tick-processor/test-tick-processor-builtin.js
deleted file mode 100644
index 1f38abe08c32e2..00000000000000
--- a/test/tick-processor/test-tick-processor-builtin.js
+++ /dev/null
@@ -1,21 +0,0 @@
-'use strict';
-const common = require('../common');
-const { isCPPSymbolsNotMapped } = require('./util');
-
-if (!common.enoughTestCpu)
-  common.skip('test is CPU-intensive');
-
-if (isCPPSymbolsNotMapped) {
-  common.skip('C++ symbols are not mapped for this os.');
-}
-
-const base = require('./tick-processor-base.js');
-
-base.runTest({
-  pattern: /Builtin_DateNow/,
-  code: `function f() {
-           this.ts = Date.now();
-           setImmediate(function() { new f(); });
-         };
-         f();`
-});
diff --git a/test/tick-processor/test-tick-processor-cpp-core.js b/test/tick-processor/test-tick-processor-cpp-core.js
deleted file mode 100644
index e76d99ab09db7b..00000000000000
--- a/test/tick-processor/test-tick-processor-cpp-core.js
+++ /dev/null
@@ -1,21 +0,0 @@
-'use strict';
-const common = require('../common');
-const { isCPPSymbolsNotMapped } = require('./util');
-
-if (!common.enoughTestCpu)
-  common.skip('test is CPU-intensive');
-
-if (isCPPSymbolsNotMapped) {
-  common.skip('C++ symbols are not mapped for this os.');
-}
-
-const base = require('./tick-processor-base.js');
-
-base.runTest({
-  pattern: /MakeContext/,
-  code: `function f() {
-           require('vm').createContext({});
-           setImmediate(function() { f(); });
-         };
-         f();`
-});
diff --git a/test/tick-processor/test-tick-processor-preprocess-flag.js b/test/tick-processor/test-tick-processor-preprocess-flag.js
deleted file mode 100644
index 8b1ec9920f47ed..00000000000000
--- a/test/tick-processor/test-tick-processor-preprocess-flag.js
+++ /dev/null
@@ -1,22 +0,0 @@
-'use strict';
-const common = require('../common');
-const { isCPPSymbolsNotMapped } = require('./util');
-
-if (!common.enoughTestCpu)
-  common.skip('test is CPU-intensive');
-
-if (isCPPSymbolsNotMapped) {
-  common.skip('C++ symbols are not mapped for this os.');
-}
-
-const base = require('./tick-processor-base.js');
-
-base.runTest({
-  pattern: /^{/,
-  code: `function f() {
-           require('vm').createContext({});
-           setImmediate(function() { f(); });
-         };
-         f();`,
-  profProcessFlags: ['--preprocess']
-});
diff --git a/test/tick-processor/test-tick-processor-unknown.js b/test/tick-processor/test-tick-processor-unknown.js
deleted file mode 100644
index 182f8c957c820a..00000000000000
--- a/test/tick-processor/test-tick-processor-unknown.js
+++ /dev/null
@@ -1,28 +0,0 @@
-'use strict';
-const common = require('../common');
-
-// 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.');
-
-if (!common.enoughTestCpu)
-  common.skip('test is CPU-intensive');
-
-const base = require('./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/tick-processor/testcfg.py b/test/tick-processor/testcfg.py
deleted file mode 100644
index 42e98355a5823f..00000000000000
--- a/test/tick-processor/testcfg.py
+++ /dev/null
@@ -1,6 +0,0 @@
-import sys, os
-sys.path.append(os.path.join(os.path.dirname(__file__), '..'))
-import testpy
-
-def GetConfiguration(context, root):
-  return testpy.SimpleTestConfiguration(context, root, 'tick-processor')
diff --git a/test/tick-processor/tick-processor-base.js b/test/tick-processor/tick-processor-base.js
deleted file mode 100644
index 33944655258bef..00000000000000
--- a/test/tick-processor/tick-processor-base.js
+++ /dev/null
@@ -1,65 +0,0 @@
-'use strict';
-require('../common');
-const fs = require('fs');
-const cp = require('child_process');
-const path = require('path');
-
-const tmpdir = require('../common/tmpdir');
-tmpdir.refresh();
-
-const LOG_FILE = path.join(tmpdir.path, 'tick-processor.log');
-const RETRY_TIMEOUT = 150;
-
-function runTest(test) {
-  const proc = cp.spawn(process.execPath, [
-    '--no_logfile_per_isolate',
-    '--logfile=-',
-    '--prof',
-    '-pe', test.code
-  ], {
-    stdio: [ 'ignore', 'pipe', 'inherit' ]
-  });
-
-  let ticks = '';
-  proc.stdout.on('data', (chunk) => ticks += chunk);
-
-  // Try to match after timeout
-  setTimeout(() => {
-    match(test.pattern, proc, () => ticks, test.profProcessFlags);
-  }, RETRY_TIMEOUT);
-}
-
-function match(pattern, parent, ticks, flags = []) {
-  // Store current ticks log
-  fs.writeFileSync(LOG_FILE, ticks());
-
-  const proc = cp.spawn(process.execPath, [
-    '--prof-process',
-    '--call-graph-size=10',
-    ...flags,
-    LOG_FILE
-  ], {
-    stdio: [ 'ignore', 'pipe', 'inherit' ]
-  });
-
-  let out = '';
-
-  proc.stdout.on('data', (chunk) => out += chunk);
-  proc.stdout.once('end', () => {
-    proc.once('exit', () => {
-      fs.unlinkSync(LOG_FILE);
-
-      // Retry after timeout
-      if (!pattern.test(out))
-        return setTimeout(() => match(pattern, parent, ticks), RETRY_TIMEOUT);
-
-      parent.stdout.removeAllListeners();
-      parent.kill();
-    });
-
-    proc.stdout.removeAllListeners();
-    proc.kill();
-  });
-}
-
-exports.runTest = runTest;
diff --git a/test/tick-processor/util.js b/test/tick-processor/util.js
deleted file mode 100644
index d25a2a7dcb132e..00000000000000
--- a/test/tick-processor/util.js
+++ /dev/null
@@ -1,18 +0,0 @@
-'use strict';
-
-// Utilities for the tick-processor tests
-const {
-  isWindows,
-  isSunOS,
-  isAIX,
-  isLinuxPPCBE,
-  isFreeBSD
-} = require('../common');
-
-module.exports = {
-  isCPPSymbolsNotMapped: isWindows ||
-                         isSunOS ||
-                         isAIX ||
-                         isLinuxPPCBE ||
-                         isFreeBSD
-};