From 2a90bb95ccc0f5d852c2611fda0ca7bb131a9a5d Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 25 Aug 2022 13:25:25 +0800 Subject: [PATCH] test: split report OOM tests On some machines the report OOM tests can take too long to complete, resulting in a timeout. This splits the test into several different smaller tests to reduce the flakiness of it. PR-URL: https://github.com/nodejs/node/pull/44389 Refs: https://github.com/nodejs/reliability/issues/356 Reviewed-By: Chengzhong Wu Reviewed-By: James M Snell Reviewed-By: Luigi Pinca --- test/fixtures/report-oom.js | 13 ++ ...test-report-fatalerror-oomerror-compact.js | 35 +++++ ...st-report-fatalerror-oomerror-directory.js | 36 +++++ ...est-report-fatalerror-oomerror-filename.js | 40 ++++++ ...test-report-fatalerror-oomerror-not-set.js | 26 ++++ .../test-report-fatalerror-oomerror-set.js | 37 ++++++ .../report/test-report-fatalerror-oomerror.js | 123 ------------------ 7 files changed, 187 insertions(+), 123 deletions(-) create mode 100644 test/fixtures/report-oom.js create mode 100644 test/report/test-report-fatalerror-oomerror-compact.js create mode 100644 test/report/test-report-fatalerror-oomerror-directory.js create mode 100644 test/report/test-report-fatalerror-oomerror-filename.js create mode 100644 test/report/test-report-fatalerror-oomerror-not-set.js create mode 100644 test/report/test-report-fatalerror-oomerror-set.js delete mode 100644 test/report/test-report-fatalerror-oomerror.js diff --git a/test/fixtures/report-oom.js b/test/fixtures/report-oom.js new file mode 100644 index 00000000000000..1677dc239eb453 --- /dev/null +++ b/test/fixtures/report-oom.js @@ -0,0 +1,13 @@ +'use strict'; + +const list = []; +while (true) { + const record = new MyRecord(); + list.push(record); +} + +function MyRecord() { + this.name = 'foo'; + this.id = 128; + this.account = 98454324; +} diff --git a/test/report/test-report-fatalerror-oomerror-compact.js b/test/report/test-report-fatalerror-oomerror-compact.js new file mode 100644 index 00000000000000..c8ed75e3ede21f --- /dev/null +++ b/test/report/test-report-fatalerror-oomerror-compact.js @@ -0,0 +1,35 @@ +'use strict'; + +// Testcases for situations involving fatal errors, like Javascript heap OOM + +require('../common'); +const assert = require('assert'); +const fs = require('fs'); +const helper = require('../common/report.js'); +const spawnSync = require('child_process').spawnSync; +const tmpdir = require('../common/tmpdir'); +const fixtures = require('../common/fixtures'); + +// Common args that will cause an out-of-memory error for child process. +const ARGS = [ + '--max-old-space-size=20', + fixtures.path('report-oom'), +]; + +{ + // Verify that --report-compact is respected when set. + tmpdir.refresh(); + const args = ['--report-on-fatalerror', '--report-compact', ...ARGS]; + const child = spawnSync(process.execPath, args, { cwd: tmpdir.path }); + assert.notStrictEqual(child.status, 0, 'Process exited unexpectedly'); + + const reports = helper.findReports(child.pid, tmpdir.path); + assert.strictEqual(reports.length, 1); + + const report = reports[0]; + helper.validate(report); + assert.strictEqual(require(report).header.threadId, null); + // Subtract 1 because "xx\n".split("\n") => [ 'xx', '' ]. + const lines = fs.readFileSync(report, 'utf8').split('\n').length - 1; + assert.strictEqual(lines, 1); +} diff --git a/test/report/test-report-fatalerror-oomerror-directory.js b/test/report/test-report-fatalerror-oomerror-directory.js new file mode 100644 index 00000000000000..5135760d7db9df --- /dev/null +++ b/test/report/test-report-fatalerror-oomerror-directory.js @@ -0,0 +1,36 @@ +'use strict'; + +// Testcases for situations involving fatal errors, like Javascript heap OOM + +require('../common'); +const assert = require('assert'); +const fs = require('fs'); +const helper = require('../common/report.js'); +const spawnSync = require('child_process').spawnSync; +const tmpdir = require('../common/tmpdir'); +const fixtures = require('../common/fixtures'); + +// Common args that will cause an out-of-memory error for child process. +const ARGS = [ + '--max-old-space-size=20', + fixtures.path('report-oom'), +]; + +{ + // Verify that --report-directory is respected when set. + // Verify that --report-compact is respected when not set. + tmpdir.refresh(); + const dir = '--report-directory=' + tmpdir.path; + const args = ['--report-on-fatalerror', dir, ...ARGS]; + const child = spawnSync(process.execPath, args, { }); + assert.notStrictEqual(child.status, 0, 'Process exited unexpectedly'); + + const reports = helper.findReports(child.pid, tmpdir.path); + assert.strictEqual(reports.length, 1); + + const report = reports[0]; + helper.validate(report); + assert.strictEqual(require(report).header.threadId, null); + const lines = fs.readFileSync(report, 'utf8').split('\n').length - 1; + assert(lines > 10); +} diff --git a/test/report/test-report-fatalerror-oomerror-filename.js b/test/report/test-report-fatalerror-oomerror-filename.js new file mode 100644 index 00000000000000..b0995f8d5aeb27 --- /dev/null +++ b/test/report/test-report-fatalerror-oomerror-filename.js @@ -0,0 +1,40 @@ +'use strict'; + +// Testcases for situations involving fatal errors, like Javascript heap OOM + +require('../common'); +const assert = require('assert'); +const helper = require('../common/report.js'); +const spawnSync = require('child_process').spawnSync; +const tmpdir = require('../common/tmpdir'); +const fixtures = require('../common/fixtures'); + +// Common args that will cause an out-of-memory error for child process. +const ARGS = [ + '--max-old-space-size=20', + fixtures.path('report-oom'), +]; + +{ + // Verify that --report-compact is respected when set. + // Verify that --report-filename is respected when set. + tmpdir.refresh(); + const args = [ + '--report-on-fatalerror', + '--report-compact', + '--report-filename=stderr', + ...ARGS, + ]; + const child = spawnSync(process.execPath, args, { encoding: 'utf8' }); + assert.notStrictEqual(child.status, 0, 'Process exited unexpectedly'); + + const reports = helper.findReports(child.pid, tmpdir.path); + assert.strictEqual(reports.length, 0); + + const lines = child.stderr.split('\n'); + // Skip over unavoidable free-form output and gc log from V8. + const report = lines.find((i) => i.startsWith('{')); + const json = JSON.parse(report); + + assert.strictEqual(json.header.threadId, null); +} diff --git a/test/report/test-report-fatalerror-oomerror-not-set.js b/test/report/test-report-fatalerror-oomerror-not-set.js new file mode 100644 index 00000000000000..31bb8f1f3848ba --- /dev/null +++ b/test/report/test-report-fatalerror-oomerror-not-set.js @@ -0,0 +1,26 @@ +'use strict'; + +// Testcases for situations involving fatal errors, like Javascript heap OOM + +require('../common'); +const assert = require('assert'); +const helper = require('../common/report.js'); +const spawnSync = require('child_process').spawnSync; +const tmpdir = require('../common/tmpdir'); +const fixtures = require('../common/fixtures'); + +// Common args that will cause an out-of-memory error for child process. +const ARGS = [ + '--max-old-space-size=20', + fixtures.path('report-oom'), +]; + +{ + tmpdir.refresh(); + // Verify that --report-on-fatalerror is respected when not set. + const args = ARGS; + const child = spawnSync(process.execPath, args, { cwd: tmpdir.path }); + assert.notStrictEqual(child.status, 0, 'Process exited unexpectedly'); + const reports = helper.findReports(child.pid, tmpdir.path); + assert.strictEqual(reports.length, 0); +} diff --git a/test/report/test-report-fatalerror-oomerror-set.js b/test/report/test-report-fatalerror-oomerror-set.js new file mode 100644 index 00000000000000..ce2a7869f7e6c0 --- /dev/null +++ b/test/report/test-report-fatalerror-oomerror-set.js @@ -0,0 +1,37 @@ +'use strict'; + +// Testcases for situations involving fatal errors, like Javascript heap OOM + +require('../common'); +const assert = require('assert'); +const helper = require('../common/report.js'); +const spawnSync = require('child_process').spawnSync; +const tmpdir = require('../common/tmpdir'); +const fixtures = require('../common/fixtures'); + +// Common args that will cause an out-of-memory error for child process. +const ARGS = [ + '--max-old-space-size=20', + fixtures.path('report-oom'), +]; + +{ + // Verify that --report-on-fatalerror is respected when set. + tmpdir.refresh(); + const args = ['--report-on-fatalerror', ...ARGS]; + const child = spawnSync(process.execPath, args, { cwd: tmpdir.path }); + assert.notStrictEqual(child.status, 0, 'Process exited unexpectedly'); + + const reports = helper.findReports(child.pid, tmpdir.path); + assert.strictEqual(reports.length, 1); + + const report = reports[0]; + helper.validate(report); + + const content = require(report); + // Errors occur in a context where env is not available, so thread ID is + // unknown. Assert this, to verify that the underlying env-less situation is + // actually reached. + assert.strictEqual(content.header.threadId, null); + assert.strictEqual(content.header.trigger, 'OOMError'); +} diff --git a/test/report/test-report-fatalerror-oomerror.js b/test/report/test-report-fatalerror-oomerror.js deleted file mode 100644 index 5b918d65e62e54..00000000000000 --- a/test/report/test-report-fatalerror-oomerror.js +++ /dev/null @@ -1,123 +0,0 @@ -'use strict'; - -// Testcases for situations involving fatal errors, like Javascript heap OOM - -require('../common'); -const assert = require('assert'); -const fs = require('fs'); -const helper = require('../common/report.js'); -const spawnSync = require('child_process').spawnSync; -const tmpdir = require('../common/tmpdir'); - -if (process.argv[2] === 'child') { - - const list = []; - while (true) { - const record = new MyRecord(); - list.push(record); - } - - function MyRecord() { - this.name = 'foo'; - this.id = 128; - this.account = 98454324; - } -} - -// Common args that will cause an out-of-memory error for child process. -const ARGS = [ - '--max-old-space-size=20', - __filename, - 'child', -]; - -{ - // Verify that --report-on-fatalerror is respected when set. - tmpdir.refresh(); - const args = ['--report-on-fatalerror', ...ARGS]; - const child = spawnSync(process.execPath, args, { cwd: tmpdir.path }); - assert.notStrictEqual(child.status, 0, 'Process exited unexpectedly'); - - const reports = helper.findReports(child.pid, tmpdir.path); - assert.strictEqual(reports.length, 1); - - const report = reports[0]; - helper.validate(report); - - const content = require(report); - // Errors occur in a context where env is not available, so thread ID is - // unknown. Assert this, to verify that the underlying env-less situation is - // actually reached. - assert.strictEqual(content.header.threadId, null); - assert.strictEqual(content.header.trigger, 'OOMError'); -} - -{ - // Verify that --report-on-fatalerror is respected when not set. - const args = ARGS; - const child = spawnSync(process.execPath, args, { cwd: tmpdir.path }); - assert.notStrictEqual(child.status, 0, 'Process exited unexpectedly'); - const reports = helper.findReports(child.pid, tmpdir.path); - assert.strictEqual(reports.length, 0); -} - -{ - // Verify that --report-directory is respected when set. - // Verify that --report-compact is respected when not set. - tmpdir.refresh(); - const dir = '--report-directory=' + tmpdir.path; - const args = ['--report-on-fatalerror', dir, ...ARGS]; - const child = spawnSync(process.execPath, args, { }); - assert.notStrictEqual(child.status, 0, 'Process exited unexpectedly'); - - const reports = helper.findReports(child.pid, tmpdir.path); - assert.strictEqual(reports.length, 1); - - const report = reports[0]; - helper.validate(report); - assert.strictEqual(require(report).header.threadId, null); - const lines = fs.readFileSync(report, 'utf8').split('\n').length - 1; - assert(lines > 10); -} - -{ - // Verify that --report-compact is respected when set. - tmpdir.refresh(); - const args = ['--report-on-fatalerror', '--report-compact', ...ARGS]; - const child = spawnSync(process.execPath, args, { cwd: tmpdir.path }); - assert.notStrictEqual(child.status, 0, 'Process exited unexpectedly'); - - const reports = helper.findReports(child.pid, tmpdir.path); - assert.strictEqual(reports.length, 1); - - const report = reports[0]; - helper.validate(report); - assert.strictEqual(require(report).header.threadId, null); - // Subtract 1 because "xx\n".split("\n") => [ 'xx', '' ]. - const lines = fs.readFileSync(report, 'utf8').split('\n').length - 1; - assert.strictEqual(lines, 1); -} - -{ - // Verify that --report-compact is respected when set. - // Verify that --report-filename is respected when set. - tmpdir.refresh(); - const args = [ - '--report-on-fatalerror', - '--report-compact', - '--report-filename=stderr', - ...ARGS, - ]; - const child = spawnSync(process.execPath, args, { encoding: 'utf8' }); - assert.notStrictEqual(child.status, 0, 'Process exited unexpectedly'); - - const reports = helper.findReports(child.pid, tmpdir.path); - assert.strictEqual(reports.length, 0); - - const lines = child.stderr.split('\n'); - // Skip over unavoidable free-form output and gc log from V8. - const report = lines.find((i) => i.startsWith('{')); - const json = JSON.parse(report); - - assert.strictEqual(json.header.threadId, null); -}