Skip to content

Commit

Permalink
test: account for OOM risks in heapsnapshot-near-heap-limit tests
Browse files Browse the repository at this point in the history
PR-URL: nodejs#37761
Fixes: nodejs#36961
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information
joyeecheung authored and foxxyz committed Oct 18, 2021
1 parent 0586567 commit e47a985
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 17 deletions.
17 changes: 10 additions & 7 deletions test/parallel/test-heapsnapshot-near-heap-limit-worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,14 @@ const env = {
console.log(child.stdout.toString());
const stderr = child.stderr.toString();
console.log(stderr);
// There should be one snapshot taken and then after the
// snapshot heap limit callback is popped, the OOM callback
// becomes effective.
assert(stderr.includes('ERR_WORKER_OUT_OF_MEMORY'));
const list = fs.readdirSync(tmpdir.path)
.filter((file) => file.endsWith('.heapsnapshot'));
assert.strictEqual(list.length, 1);
const risky = /Not generating snapshots because it's too risky/.test(stderr);
if (!risky) {
// There should be one snapshot taken and then after the
// snapshot heap limit callback is popped, the OOM callback
// becomes effective.
assert(stderr.includes('ERR_WORKER_OUT_OF_MEMORY'));
const list = fs.readdirSync(tmpdir.path)
.filter((file) => file.endsWith('.heapsnapshot'));
assert.strictEqual(list.length, 1);
}
}
18 changes: 14 additions & 4 deletions test/parallel/test-heapsnapshot-near-heap-limit.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,17 @@ const env = {
env,
});
console.log(child.stdout.toString());
console.log(child.stderr.toString());
const stderr = child.stderr.toString();
console.log(stderr);
assert(common.nodeProcessAborted(child.status, child.signal),
'process should have aborted, but did not');
const list = fs.readdirSync(tmpdir.path)
.filter((file) => file.endsWith('.heapsnapshot'));
assert.strictEqual(list.length, 1);
const risky = [...stderr.matchAll(
/Not generating snapshots because it's too risky/g)].length;
assert(list.length + risky > 0 && list.length <= 3,
`Generated ${list.length} snapshots ` +
`and ${risky} was too risky`);
}

{
Expand All @@ -79,10 +84,15 @@ const env = {
env,
});
console.log(child.stdout.toString());
console.log(child.stderr.toString());
const stderr = child.stderr.toString();
console.log(stderr);
assert(common.nodeProcessAborted(child.status, child.signal),
'process should have aborted, but did not');
const list = fs.readdirSync(tmpdir.path)
.filter((file) => file.endsWith('.heapsnapshot'));
assert(list.length > 0 && list.length <= 3);
const risky = [...stderr.matchAll(
/Not generating snapshots because it's too risky/g)].length;
assert(list.length + risky > 0 && list.length <= 3,
`Generated ${list.length} snapshots ` +
`and ${risky} was too risky`);
}
11 changes: 5 additions & 6 deletions test/pummel/test-heapsnapshot-near-heap-limit-big.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,9 @@ if (!common.enoughTestMem)
'process should have aborted, but did not');
const list = fs.readdirSync(tmpdir.path)
.filter((file) => file.endsWith('.heapsnapshot'));
if (list.length === 0) {
assert(stderr.includes(
'Not generating snapshots because it\'s too risky'));
} else {
assert(list.length > 0 && list.length <= 3);
}
const risky = [...stderr.matchAll(
/Not generating snapshots because it's too risky/g)].length;
assert(list.length + risky > 0 && list.length <= 3,
`Generated ${list.length} snapshots ` +
`and ${risky} was too risky`);
}

0 comments on commit e47a985

Please sign in to comment.