test: improve async-hooks/test-callback-error

PR-URL: #13559
Fixes: #13527
Reviewed-By: Andreas Madsen <[email protected]>

test/async-hooks/async-hooks.status

@@ -0,0 +1,21 @@
+prefix async-hooks
+
+# To mark a test as flaky, list the test name in the appropriate section
+# below, without ".js", followed by ": PASS,FLAKY". Example:
+# sample-test                        : PASS,FLAKY
+
+[true] # This section applies to all platforms
+
+[$system==win32]
+
+[$system==linux]
+test-callback-error                  : PASS,FLAKY
+
+[$system==macos]
+test-callback-error                  : PASS,FLAKY
+
+[$arch==arm || $arch==arm64]
+
+[$system==solaris] # Also applies to SmartOS
+
+[$system==freebsd]

test/async-hooks/test-callback-error.js

@@ -1,56 +1,120 @@
 'use strict';
-
 const common = require('../common');
 const assert = require('assert');
-const spawnSync = require('child_process').spawnSync;
+const { spawnSync, fork } = require('child_process');
 const async_hooks = require('async_hooks');
 const initHooks = require('./init-hooks');
 
-switch (process.argv[2]) {
+const arg = process.argv[2];
+switch (arg) {
   case 'test_init_callback':
     initHooks({
-      oninit: common.mustCall(() => { throw new Error('test_init_callback'); })
+      oninit: common.mustCall(() => { throw new Error(arg); })
     }).enable();
+    async_hooks.emitInit(
+      async_hooks.executionAsyncId(),
+      `${arg}_type`,
+      async_hooks.triggerAsyncId()
+    );
+    return;
 
-    async_hooks.emitInit(async_hooks.executionAsyncId(),
-                         'test_init_callback_type',
-                         async_hooks.triggerAsyncId());
-    break;
   case 'test_callback':
     initHooks({
-      onbefore: common.mustCall(() => { throw new Error('test_callback'); })
+      onbefore: common.mustCall(() => { throw new Error(arg); })
     }).enable();
-
-    async_hooks.emitInit(async_hooks.executionAsyncId(), 'test_callback_type',
-                         async_hooks.triggerAsyncId());
+    async_hooks.emitInit(
+      async_hooks.executionAsyncId(),
+      `${arg}_type`,
+      async_hooks.triggerAsyncId()
+    );
     async_hooks.emitBefore(async_hooks.executionAsyncId());
-    break;
+    return;
+
   case 'test_callback_abort':
     initHooks({
-      oninit: common.mustCall(() => { throw new Error('test_callback_abort'); })
+      oninit: common.mustCall(() => { throw new Error(arg); })
     }).enable();
+    async_hooks.emitInit(
+      async_hooks.executionAsyncId(),
+      `${arg}_type`,
+      async_hooks.triggerAsyncId()
+    );
+    return;
+}
+
+// this part should run only for the master test
+assert.ok(!arg);
+{
+  // console.log should stay until this test's flakiness is solved
+  console.log('start case 1');
+  console.time('end case 1');
+  const child = spawnSync(process.execPath, [__filename, 'test_init_callback']);
+  assert.ifError(child.error);
+  const test_init_first_line = child.stderr.toString().split(/[\r\n]+/g)[0];
+  assert.strictEqual(test_init_first_line, 'Error: test_init_callback');
+  assert.strictEqual(child.status, 1);
+  console.timeEnd('end case 1');
+}
 
-    async_hooks.emitInit(async_hooks.executionAsyncId(), 'test_callback_abort',
-                         async_hooks.triggerAsyncId());
-    break;
+{
+  console.log('start case 2');
+  console.time('end case 2');
+  const child = spawnSync(process.execPath, [__filename, 'test_callback']);
+  assert.ifError(child.error);
+  const test_callback_first_line = child.stderr.toString().split(/[\r\n]+/g)[0];
+  assert.strictEqual(test_callback_first_line, 'Error: test_callback');
+  assert.strictEqual(child.status, 1);
+  console.timeEnd('end case 2');
 }
 
-const c1 = spawnSync(`${process.execPath}`, [__filename, 'test_init_callback']);
-assert.strictEqual(c1.stderr.toString().split('\n')[0],
-                   'Error: test_init_callback');
-assert.strictEqual(c1.status, 1);
-
-const c2 = spawnSync(`${process.execPath}`, [__filename, 'test_callback']);
-assert.strictEqual(c2.stderr.toString().split('\n')[0], 'Error: test_callback');
-assert.strictEqual(c2.status, 1);
-
-const c3 = spawnSync(`${process.execPath}`, ['--abort-on-uncaught-exception',
-                                             __filename,
-                                             'test_callback_abort']);
-assert.strictEqual(c3.stdout.toString(), '');
-
-const stderrOutput = c3.stderr.toString()
-                       .trim()
-                       .split('\n')
-                       .map((s) => s.trim());
-assert.strictEqual(stderrOutput[0], 'Error: test_callback_abort');
+{
+  console.log('start case 3');
+  console.time('end case 3');
+  // Timeout is set because this case is known to be problematic, so stderr is
+  // logged for further analysis.
+  // Ref: https://github.com/nodejs/node/issues/13527
+  // Ref: https://github.com/nodejs/node/pull/13559
+  const opts = {
+    execArgv: ['--abort-on-uncaught-exception'],
+    silent: true
+  };
+  const child = fork(__filename, ['test_callback_abort'], opts);
+
+  let stdout = '';
+  child.stdout.on('data', (data) => {
+    stdout += data;
+  });
+
+  let stderr = '';
+  child.stderr.on('data', (data) => {
+    stderr += data;
+  });
+
+  const tO = setTimeout(() => {
+    console.log(stderr);
+    child.kill('SIGKILL');
+    process.exit(1);
+  }, 15 * 1000);
+  tO.unref();
+
+  child.on('close', (code, signal) => {
+    clearTimeout(tO);
+    if (common.isWindows) {
+      assert.strictEqual(code, 3);
+      assert.strictEqual(signal, null);
+    } else {
+      assert.strictEqual(code, null);
+      // most posix systems will show 'SIGABRT', but alpine34 does not
+      if (signal !== 'SIGABRT') {
+        console.log(`parent recived signal ${signal}\nchild's stderr:`);
+        console.log(stderr);
+        process.exit(1);
+      }
+      assert.strictEqual(signal, 'SIGABRT');
+    }
+    assert.strictEqual(stdout, '');
+    const firstLineStderr = stderr.split(/[\r\n]+/g)[0].trim();
+    assert.strictEqual(firstLineStderr, 'Error: test_callback_abort');
+  });
+  console.timeEnd('end case 3');
+}