From 48ee92539993d17906004fa65341926b5ceb3762 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 25 Aug 2022 13:25:25 +0800 Subject: [PATCH 1/2] 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. --- 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 | 41 ++++++ ...test-report-fatalerror-oomerror-not-set.js | 26 ++++ .../test-report-fatalerror-oomerror-set.js | 38 ++++++ .../report/test-report-fatalerror-oomerror.js | 123 ------------------ 7 files changed, 189 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..b60136549c4f8b --- /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..7d18defc1c6cbe --- /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..59292f5fb02b0b --- /dev/null +++ b/test/report/test-report-fatalerror-oomerror-filename.js @@ -0,0 +1,41 @@ +'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. + // 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..242bc9db64983e --- /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 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-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..d7b3994fcbd8e6 --- /dev/null +++ b/test/report/test-report-fatalerror-oomerror-set.js @@ -0,0 +1,38 @@ +'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-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); -} From 8bf17a0a31f5f9bfefa959bf84dad018423aa5cb Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 25 Aug 2022 16:40:28 +0800 Subject: [PATCH 2/2] fixup! test: split report OOM tests --- test/report/test-report-fatalerror-oomerror-compact.js | 2 +- test/report/test-report-fatalerror-oomerror-directory.js | 2 +- test/report/test-report-fatalerror-oomerror-filename.js | 3 +-- test/report/test-report-fatalerror-oomerror-not-set.js | 4 ++-- test/report/test-report-fatalerror-oomerror-set.js | 3 +-- 5 files changed, 6 insertions(+), 8 deletions(-) diff --git a/test/report/test-report-fatalerror-oomerror-compact.js b/test/report/test-report-fatalerror-oomerror-compact.js index b60136549c4f8b..c8ed75e3ede21f 100644 --- a/test/report/test-report-fatalerror-oomerror-compact.js +++ b/test/report/test-report-fatalerror-oomerror-compact.js @@ -13,7 +13,7 @@ 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') + fixtures.path('report-oom'), ]; { diff --git a/test/report/test-report-fatalerror-oomerror-directory.js b/test/report/test-report-fatalerror-oomerror-directory.js index 7d18defc1c6cbe..5135760d7db9df 100644 --- a/test/report/test-report-fatalerror-oomerror-directory.js +++ b/test/report/test-report-fatalerror-oomerror-directory.js @@ -13,7 +13,7 @@ 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') + fixtures.path('report-oom'), ]; { diff --git a/test/report/test-report-fatalerror-oomerror-filename.js b/test/report/test-report-fatalerror-oomerror-filename.js index 59292f5fb02b0b..b0995f8d5aeb27 100644 --- a/test/report/test-report-fatalerror-oomerror-filename.js +++ b/test/report/test-report-fatalerror-oomerror-filename.js @@ -4,7 +4,6 @@ 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'); @@ -13,7 +12,7 @@ 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') + fixtures.path('report-oom'), ]; { diff --git a/test/report/test-report-fatalerror-oomerror-not-set.js b/test/report/test-report-fatalerror-oomerror-not-set.js index 242bc9db64983e..31bb8f1f3848ba 100644 --- a/test/report/test-report-fatalerror-oomerror-not-set.js +++ b/test/report/test-report-fatalerror-oomerror-not-set.js @@ -4,7 +4,6 @@ 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'); @@ -13,10 +12,11 @@ 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') + 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 }); diff --git a/test/report/test-report-fatalerror-oomerror-set.js b/test/report/test-report-fatalerror-oomerror-set.js index d7b3994fcbd8e6..ce2a7869f7e6c0 100644 --- a/test/report/test-report-fatalerror-oomerror-set.js +++ b/test/report/test-report-fatalerror-oomerror-set.js @@ -4,7 +4,6 @@ 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'); @@ -13,7 +12,7 @@ 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') + fixtures.path('report-oom'), ]; {