Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 7b60b8f

Browse filesBrowse files
ofrobotsMyles Borins
authored andcommitted
test: fix flakiness of stringbytes-external
The tests used to rely on precise timing of when a JavaScript object would be garbage collected to ensure that there is enough memory available on the system. Switch the test to use a malloc/free pair instead. Ref: #5945 Ref: #6039 Ref: #6073 PR-URL: #6705 Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent cc4c518 commit 7b60b8f
Copy full SHA for 7b60b8f
Expand file treeCollapse file tree

11 files changed

+99
-49
lines changed
Open diff view settings
Collapse file

‎test/README.md‎

Copy file name to clipboardExpand all lines: test/README.md
+3-1Lines changed: 3 additions & 1 deletion
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,9 @@ Tests for when the `--abort-on-uncaught-exception` flag is used.
1010

1111
### addons
1212

13-
Tests for [addon](https://nodejs.org/api/addons.html) functionality.
13+
Tests for [addon](https://nodejs.org/api/addons.html) functionality along with
14+
some tests that require an addon to function properly.
15+
1416

1517
| Runs on CI |
1618
|:----------:|
Collapse file
+24Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
#include <stdlib.h>
2+
#include <node.h>
3+
#include <v8.h>
4+
5+
void EnsureAllocation(const v8::FunctionCallbackInfo<v8::Value> &args) {
6+
v8::Isolate* isolate = args.GetIsolate();
7+
uintptr_t size = args[0]->IntegerValue();
8+
v8::Local<v8::Boolean> success;
9+
10+
void* buffer = malloc(size);
11+
if (buffer) {
12+
success = v8::Boolean::New(isolate, true);
13+
free(buffer);
14+
} else {
15+
success = v8::Boolean::New(isolate, false);
16+
}
17+
args.GetReturnValue().Set(success);
18+
}
19+
20+
void init(v8::Local<v8::Object> target) {
21+
NODE_SET_METHOD(target, "ensureAllocation", EnsureAllocation);
22+
}
23+
24+
NODE_MODULE(binding, init);
Collapse file
+8Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
'targets': [
3+
{
4+
'target_name': 'binding',
5+
'sources': [ 'binding.cc' ]
6+
}
7+
]
8+
}
Collapse file

‎…llel/test-stringbytes-external-at-max.js‎ ‎…-max/test-stringbytes-external-at-max.js‎test/parallel/test-stringbytes-external-at-max.js renamed to test/addons/stringbytes-external-exceed-max/test-stringbytes-external-at-max.js test/parallel/test-stringbytes-external-at-max.js renamed to test/addons/stringbytes-external-exceed-max/test-stringbytes-external-at-max.js

Copy file name to clipboard
+8-6Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
'use strict';
2-
// Flags: --expose-gc
32

4-
const common = require('../common');
3+
const common = require('../../common');
4+
const binding = require('./build/Release/binding');
55
const assert = require('assert');
66

77
// v8 fails silently if string length > v8::String::kMaxLength
@@ -14,19 +14,21 @@ if (!common.enoughTestMem) {
1414
console.log(skipMessage);
1515
return;
1616
}
17-
assert(typeof gc === 'function', 'Run this test with --expose-gc');
1817

1918
try {
2019
var buf = new Buffer(kStringMaxLength);
21-
// Try to allocate memory first then force gc so future allocations succeed.
22-
new Buffer(2 * kStringMaxLength);
23-
gc();
2420
} catch (e) {
2521
// If the exception is not due to memory confinement then rethrow it.
2622
if (e.message !== 'Invalid array buffer length') throw (e);
2723
console.log(skipMessage);
2824
return;
2925
}
3026

27+
// Ensure we have enough memory available for future allocations to succeed.
28+
if (!binding.ensureAllocation(2 * kStringMaxLength)) {
29+
console.log(skipMessage);
30+
return;
31+
}
32+
3133
const maxString = buf.toString('binary');
3234
assert.equal(maxString.length, kStringMaxLength);
Collapse file

‎…gbytes-external-exceed-max-by-1-ascii.js‎ ‎…gbytes-external-exceed-max-by-1-ascii.js‎test/sequential/test-stringbytes-external-exceed-max-by-1-ascii.js renamed to test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-ascii.js test/sequential/test-stringbytes-external-exceed-max-by-1-ascii.js renamed to test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-ascii.js

Copy file name to clipboard
+8-6Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
'use strict';
2-
// Flags: --expose-gc
32

4-
const common = require('../common');
3+
const common = require('../../common');
4+
const binding = require('./build/Release/binding');
55
const assert = require('assert');
66

77
const skipMessage =
@@ -10,24 +10,26 @@ if (!common.enoughTestMem) {
1010
console.log(skipMessage);
1111
return;
1212
}
13-
assert(typeof gc === 'function', 'Run this test with --expose-gc');
1413

1514
// v8 fails silently if string length > v8::String::kMaxLength
1615
// v8::String::kMaxLength defined in v8.h
1716
const kStringMaxLength = process.binding('buffer').kStringMaxLength;
1817

1918
try {
2019
var buf = new Buffer(kStringMaxLength + 1);
21-
// Try to allocate memory first then force gc so future allocations succeed.
22-
new Buffer(2 * kStringMaxLength);
23-
gc();
2420
} catch (e) {
2521
// If the exception is not due to memory confinement then rethrow it.
2622
if (e.message !== 'Invalid array buffer length') throw (e);
2723
console.log(skipMessage);
2824
return;
2925
}
3026

27+
// Ensure we have enough memory available for future allocations to succeed.
28+
if (!binding.ensureAllocation(2 * kStringMaxLength)) {
29+
console.log(skipMessage);
30+
return;
31+
}
32+
3133
assert.throws(function() {
3234
buf.toString('ascii');
3335
}, /toString failed/);
Collapse file

‎…bytes-external-exceed-max-by-1-base64.js‎ ‎…bytes-external-exceed-max-by-1-base64.js‎test/sequential/test-stringbytes-external-exceed-max-by-1-base64.js renamed to test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-base64.js test/sequential/test-stringbytes-external-exceed-max-by-1-base64.js renamed to test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-base64.js

Copy file name to clipboard
+8-6Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
'use strict';
2-
// Flags: --expose-gc
32

4-
const common = require('../common');
3+
const common = require('../../common');
4+
const binding = require('./build/Release/binding');
55
const assert = require('assert');
66

77
const skipMessage =
@@ -10,24 +10,26 @@ if (!common.enoughTestMem) {
1010
console.log(skipMessage);
1111
return;
1212
}
13-
assert(typeof gc === 'function', 'Run this test with --expose-gc');
1413

1514
// v8 fails silently if string length > v8::String::kMaxLength
1615
// v8::String::kMaxLength defined in v8.h
1716
const kStringMaxLength = process.binding('buffer').kStringMaxLength;
1817

1918
try {
2019
var buf = new Buffer(kStringMaxLength + 1);
21-
// Try to allocate memory first then force gc so future allocations succeed.
22-
new Buffer(2 * kStringMaxLength);
23-
gc();
2420
} catch (e) {
2521
// If the exception is not due to memory confinement then rethrow it.
2622
if (e.message !== 'Invalid array buffer length') throw (e);
2723
console.log(skipMessage);
2824
return;
2925
}
3026

27+
// Ensure we have enough memory available for future allocations to succeed.
28+
if (!binding.ensureAllocation(2 * kStringMaxLength)) {
29+
console.log(skipMessage);
30+
return;
31+
}
32+
3133
assert.throws(function() {
3234
buf.toString('base64');
3335
}, /toString failed/);
Collapse file

‎…bytes-external-exceed-max-by-1-binary.js‎ ‎…bytes-external-exceed-max-by-1-binary.js‎test/sequential/test-stringbytes-external-exceed-max-by-1-binary.js renamed to test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-binary.js test/sequential/test-stringbytes-external-exceed-max-by-1-binary.js renamed to test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-binary.js

Copy file name to clipboardExpand all lines: test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-binary.js
+8-6Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
'use strict';
2-
// Flags: --expose-gc
32

4-
const common = require('../common');
3+
const common = require('../../common');
4+
const binding = require('./build/Release/binding');
55
const assert = require('assert');
66

77
const skipMessage =
@@ -10,24 +10,26 @@ if (!common.enoughTestMem) {
1010
console.log(skipMessage);
1111
return;
1212
}
13-
assert(typeof gc === 'function', 'Run this test with --expose-gc');
1413

1514
// v8 fails silently if string length > v8::String::kMaxLength
1615
// v8::String::kMaxLength defined in v8.h
1716
const kStringMaxLength = process.binding('buffer').kStringMaxLength;
1817

1918
try {
2019
var buf = new Buffer(kStringMaxLength + 1);
21-
// Try to allocate memory first then force gc so future allocations succeed.
22-
new Buffer(2 * kStringMaxLength);
23-
gc();
2420
} catch (e) {
2521
// If the exception is not due to memory confinement then rethrow it.
2622
if (e.message !== 'Invalid array buffer length') throw (e);
2723
console.log(skipMessage);
2824
return;
2925
}
3026

27+
// Ensure we have enough memory available for future allocations to succeed.
28+
if (!binding.ensureAllocation(2 * kStringMaxLength)) {
29+
console.log(skipMessage);
30+
return;
31+
}
32+
3133
assert.throws(function() {
3234
buf.toString('binary');
3335
}, /toString failed/);
Collapse file

‎…ingbytes-external-exceed-max-by-1-hex.js‎ ‎…ingbytes-external-exceed-max-by-1-hex.js‎test/sequential/test-stringbytes-external-exceed-max-by-1-hex.js renamed to test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-hex.js test/sequential/test-stringbytes-external-exceed-max-by-1-hex.js renamed to test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-hex.js

Copy file name to clipboard
+8-6Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
'use strict';
2-
// Flags: --expose-gc
32

4-
const common = require('../common');
3+
const common = require('../../common');
4+
const binding = require('./build/Release/binding');
55
const assert = require('assert');
66

77
const skipMessage =
@@ -10,24 +10,26 @@ if (!common.enoughTestMem) {
1010
console.log(skipMessage);
1111
return;
1212
}
13-
assert(typeof gc === 'function', 'Run this test with --expose-gc');
1413

1514
// v8 fails silently if string length > v8::String::kMaxLength
1615
// v8::String::kMaxLength defined in v8.h
1716
const kStringMaxLength = process.binding('buffer').kStringMaxLength;
1817

1918
try {
2019
var buf = new Buffer(kStringMaxLength + 1);
21-
// Try to allocate memory first then force gc so future allocations succeed.
22-
new Buffer(2 * kStringMaxLength);
23-
gc();
2420
} catch (e) {
2521
// If the exception is not due to memory confinement then rethrow it.
2622
if (e.message !== 'Invalid array buffer length') throw (e);
2723
console.log(skipMessage);
2824
return;
2925
}
3026

27+
// Ensure we have enough memory available for future allocations to succeed.
28+
if (!binding.ensureAllocation(2 * kStringMaxLength)) {
29+
console.log(skipMessage);
30+
return;
31+
}
32+
3133
assert.throws(function() {
3234
buf.toString('hex');
3335
}, /toString failed/);
Collapse file

‎…ngbytes-external-exceed-max-by-1-utf8.js‎ ‎…ngbytes-external-exceed-max-by-1-utf8.js‎test/sequential/test-stringbytes-external-exceed-max-by-1-utf8.js renamed to test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-utf8.js test/sequential/test-stringbytes-external-exceed-max-by-1-utf8.js renamed to test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-utf8.js

Copy file name to clipboardExpand all lines: test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-utf8.js
+8-6Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
'use strict';
2-
// Flags: --expose-gc
32

4-
const common = require('../common');
3+
const common = require('../../common');
4+
const binding = require('./build/Release/binding');
55
const assert = require('assert');
66

77
const skipMessage =
@@ -10,24 +10,26 @@ if (!common.enoughTestMem) {
1010
console.log(skipMessage);
1111
return;
1212
}
13-
assert(typeof gc === 'function', 'Run this test with --expose-gc');
1413

1514
// v8 fails silently if string length > v8::String::kMaxLength
1615
// v8::String::kMaxLength defined in v8.h
1716
const kStringMaxLength = process.binding('buffer').kStringMaxLength;
1817

1918
try {
2019
var buf = new Buffer(kStringMaxLength + 1);
21-
// Try to allocate memory first then force gc so future allocations succeed.
22-
new Buffer(2 * kStringMaxLength);
23-
gc();
2420
} catch (e) {
2521
// If the exception is not due to memory confinement then rethrow it.
2622
if (e.message !== 'Invalid array buffer length') throw (e);
2723
console.log(skipMessage);
2824
return;
2925
}
3026

27+
// Ensure we have enough memory available for future allocations to succeed.
28+
if (!binding.ensureAllocation(2 * kStringMaxLength)) {
29+
console.log(skipMessage);
30+
return;
31+
}
32+
3133
assert.throws(function() {
3234
buf.toString();
3335
}, /toString failed|Invalid array buffer length/);
Collapse file

‎…-stringbytes-external-exceed-max-by-2.js‎ ‎…-stringbytes-external-exceed-max-by-2.js‎test/sequential/test-stringbytes-external-exceed-max-by-2.js renamed to test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-2.js test/sequential/test-stringbytes-external-exceed-max-by-2.js renamed to test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-2.js

Copy file name to clipboard
+8-6Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
'use strict';
2-
// Flags: --expose-gc
32

4-
const common = require('../common');
3+
const common = require('../../common');
4+
const binding = require('./build/Release/binding');
55
const assert = require('assert');
66

77
const skipMessage =
@@ -10,23 +10,25 @@ if (!common.enoughTestMem) {
1010
console.log(skipMessage);
1111
return;
1212
}
13-
assert(typeof gc === 'function', 'Run this test with --expose-gc');
1413

1514
// v8 fails silently if string length > v8::String::kMaxLength
1615
// v8::String::kMaxLength defined in v8.h
1716
const kStringMaxLength = process.binding('buffer').kStringMaxLength;
1817

1918
try {
2019
var buf = new Buffer(kStringMaxLength + 2);
21-
// Try to allocate memory first then force gc so future allocations succeed.
22-
new Buffer(2 * kStringMaxLength);
23-
gc();
2420
} catch (e) {
2521
// If the exception is not due to memory confinement then rethrow it.
2622
if (e.message !== 'Invalid array buffer length') throw (e);
2723
console.log(skipMessage);
2824
return;
2925
}
3026

27+
// Ensure we have enough memory available for future allocations to succeed.
28+
if (!binding.ensureAllocation(2 * kStringMaxLength)) {
29+
console.log(skipMessage);
30+
return;
31+
}
32+
3133
const maxString = buf.toString('utf16le');
3234
assert.equal(maxString.length, (kStringMaxLength + 2) / 2);

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.