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: nodejs#5945 PR-URL: nodejs#6039 Reviewed-By: jasnell - James M Snell <jasnell@gmail.com> Reviewed-By: evanlucas - Evan Lucas <evanlucas@me.com> Reviewed-By: Trott - Rich Trott <rtrott@gmail.com>
ofrobots · Apr 5, 2016 · f4ebd59 · f4ebd59
1 parent 2d7e316
commit f4ebd59
Show file tree

Hide file tree

Showing 9 changed files with 88 additions and 42 deletions.
diff --git a/test/addons/stringbytes-external-exceed-max/binding.cc b/test/addons/stringbytes-external-exceed-max/binding.cc
@@ -0,0 +1,24 @@
+#include <stdlib.h>
+#include <node.h>
+#include <v8.h>
+
+void EnsureAllocation(const v8::FunctionCallbackInfo<v8::Value> &args) {
+  v8::Isolate* isolate = args.GetIsolate();
+  uintptr_t size = args[0]->IntegerValue();
+  v8::Local<v8::Boolean> success;
+
+  void* buffer = malloc(size);
+  if (buffer) {
+    success = v8::Boolean::New(isolate, true);
+    free(buffer);
+  } else {
+    success = v8::Boolean::New(isolate, false);
+  }
+  args.GetReturnValue().Set(success);
+}
+
+void init(v8::Local<v8::Object> target) {
+  NODE_SET_METHOD(target, "ensureAllocation", EnsureAllocation);
+}
+
+NODE_MODULE(binding, init);
diff --git a/test/addons/stringbytes-external-exceed-max/binding.gyp b/test/addons/stringbytes-external-exceed-max/binding.gyp
@@ -0,0 +1,8 @@
+{
+  'targets': [
+    {
+      'target_name': 'binding',
+      'sources': [ 'binding.cc' ]
+    }
+  ]
+}
diff --git a/...ngbytes-external-exceed-max-by-1-ascii.js → ...ngbytes-external-exceed-max-by-1-ascii.js b/...ngbytes-external-exceed-max-by-1-ascii.js → ...ngbytes-external-exceed-max-by-1-ascii.js
@@ -1,7 +1,7 @@
 'use strict';
-// Flags: --expose-gc
 
-const common = require('../common');
+const common = require('../../common');
+const binding = require('./build/Release/binding');
 const assert = require('assert');
 
 const skipMessage =
@@ -10,24 +10,26 @@ if (!common.enoughTestMem) {
   console.log(skipMessage);
   return;
 }
-assert(typeof gc === 'function', 'Run this test with --expose-gc');
 
 // v8 fails silently if string length > v8::String::kMaxLength
 // v8::String::kMaxLength defined in v8.h
 const kStringMaxLength = process.binding('buffer').kStringMaxLength;
 
 try {
   var buf = Buffer.allocUnsafe(kStringMaxLength + 1);
-  // Try to allocate memory first then force gc so future allocations succeed.
-  Buffer.allocUnsafe(2 * kStringMaxLength);
-  gc();
 } catch (e) {
   // If the exception is not due to memory confinement then rethrow it.
   if (e.message !== 'Array buffer allocation failed') throw (e);
   console.log(skipMessage);
   return;
 }
 
+// Ensure we have enough memory available for future allocations to succeed.
+if (!binding.ensureAllocation(2 * kStringMaxLength)) {
+  console.log(skipMessage);
+  return;
+}
+
 assert.throws(function() {
   buf.toString('ascii');
 }, /"toString\(\)" failed/);
diff --git a/...gbytes-external-exceed-max-by-1-base64.js → ...gbytes-external-exceed-max-by-1-base64.js b/...gbytes-external-exceed-max-by-1-base64.js → ...gbytes-external-exceed-max-by-1-base64.js
@@ -1,7 +1,7 @@
 'use strict';
-// Flags: --expose-gc
 
-const common = require('../common');
+const common = require('../../common');
+const binding = require('./build/Release/binding');
 const assert = require('assert');
 
 const skipMessage =
@@ -10,24 +10,26 @@ if (!common.enoughTestMem) {
   console.log(skipMessage);
   return;
 }
-assert(typeof gc === 'function', 'Run this test with --expose-gc');
 
 // v8 fails silently if string length > v8::String::kMaxLength
 // v8::String::kMaxLength defined in v8.h
 const kStringMaxLength = process.binding('buffer').kStringMaxLength;
 
 try {
   var buf = Buffer.allocUnsafe(kStringMaxLength + 1);
-  // Try to allocate memory first then force gc so future allocations succeed.
-  Buffer.allocUnsafe(2 * kStringMaxLength);
-  gc();
 } catch (e) {
   // If the exception is not due to memory confinement then rethrow it.
   if (e.message !== 'Array buffer allocation failed') throw (e);
   console.log(skipMessage);
   return;
 }
 
+// Ensure we have enough memory available for future allocations to succeed.
+if (!binding.ensureAllocation(2 * kStringMaxLength)) {
+  console.log(skipMessage);
+  return;
+}
+
 assert.throws(function() {
   buf.toString('base64');
 }, /"toString\(\)" failed/);
diff --git a/...gbytes-external-exceed-max-by-1-binary.js → ...gbytes-external-exceed-max-by-1-binary.js b/...gbytes-external-exceed-max-by-1-binary.js → ...gbytes-external-exceed-max-by-1-binary.js
@@ -1,7 +1,7 @@
 'use strict';
-// Flags: --expose-gc
 
-const common = require('../common');
+const common = require('../../common');
+const binding = require('./build/Release/binding');
 const assert = require('assert');
 
 const skipMessage =
@@ -10,24 +10,26 @@ if (!common.enoughTestMem) {
   console.log(skipMessage);
   return;
 }
-assert(typeof gc === 'function', 'Run this test with --expose-gc');
 
 // v8 fails silently if string length > v8::String::kMaxLength
 // v8::String::kMaxLength defined in v8.h
 const kStringMaxLength = process.binding('buffer').kStringMaxLength;
 
 try {
   var buf = Buffer.allocUnsafe(kStringMaxLength + 1);
-  // Try to allocate memory first then force gc so future allocations succeed.
-  Buffer.allocUnsafe(2 * kStringMaxLength);
-  gc();
 } catch (e) {
   // If the exception is not due to memory confinement then rethrow it.
   if (e.message !== 'Array buffer allocation failed') throw (e);
   console.log(skipMessage);
   return;
 }
 
+// Ensure we have enough memory available for future allocations to succeed.
+if (!binding.ensureAllocation(2 * kStringMaxLength)) {
+  console.log(skipMessage);
+  return;
+}
+
 assert.throws(function() {
   buf.toString('binary');
 }, /"toString\(\)" failed/);

diff --git a/...ringbytes-external-exceed-max-by-1-hex.js → ...ringbytes-external-exceed-max-by-1-hex.js b/...ringbytes-external-exceed-max-by-1-hex.js → ...ringbytes-external-exceed-max-by-1-hex.js
@@ -1,7 +1,7 @@
 'use strict';
-// Flags: --expose-gc
 
-const common = require('../common');
+const common = require('../../common');
+const binding = require('./build/Release/binding');
 const assert = require('assert');
 
 const skipMessage =
@@ -10,24 +10,26 @@ if (!common.enoughTestMem) {
   console.log(skipMessage);
   return;
 }
-assert(typeof gc === 'function', 'Run this test with --expose-gc');
 
 // v8 fails silently if string length > v8::String::kMaxLength
 // v8::String::kMaxLength defined in v8.h
 const kStringMaxLength = process.binding('buffer').kStringMaxLength;
 
 try {
   var buf = Buffer.allocUnsafe(kStringMaxLength + 1);
-  // Try to allocate memory first then force gc so future allocations succeed.
-  Buffer.allocUnsafe(2 * kStringMaxLength);
-  gc();
 } catch (e) {
   // If the exception is not due to memory confinement then rethrow it.
   if (e.message !== 'Array buffer allocation failed') throw (e);
   console.log(skipMessage);
   return;
 }
 
+// Ensure we have enough memory available for future allocations to succeed.
+if (!binding.ensureAllocation(2 * kStringMaxLength)) {
+  console.log(skipMessage);
+  return;
+}
+
 assert.throws(function() {
   buf.toString('hex');
 }, /"toString\(\)" failed/);
diff --git a/...ingbytes-external-exceed-max-by-1-utf8.js → ...ingbytes-external-exceed-max-by-1-utf8.js b/...ingbytes-external-exceed-max-by-1-utf8.js → ...ingbytes-external-exceed-max-by-1-utf8.js
@@ -1,7 +1,7 @@
 'use strict';
-// Flags: --expose-gc
 
-const common = require('../common');
+const common = require('../../common');
+const binding = require('./build/Release/binding');
 const assert = require('assert');
 
 const skipMessage =
@@ -10,24 +10,26 @@ if (!common.enoughTestMem) {
   console.log(skipMessage);
   return;
 }
-assert(typeof gc === 'function', 'Run this test with --expose-gc');
 
 // v8 fails silently if string length > v8::String::kMaxLength
 // v8::String::kMaxLength defined in v8.h
 const kStringMaxLength = process.binding('buffer').kStringMaxLength;
 
 try {
   var buf = Buffer.allocUnsafe(kStringMaxLength + 1);
-  // Try to allocate memory first then force gc so future allocations succeed.
-  Buffer.allocUnsafe(2 * kStringMaxLength);
-  gc();
 } catch (e) {
   // If the exception is not due to memory confinement then rethrow it.
   if (e.message !== 'Array buffer allocation failed') throw (e);
   console.log(skipMessage);
   return;
 }
 
+// Ensure we have enough memory available for future allocations to succeed.
+if (!binding.ensureAllocation(2 * kStringMaxLength)) {
+  console.log(skipMessage);
+  return;
+}
+
 assert.throws(function() {
   buf.toString();
 }, /"toString\(\)" failed|Array buffer allocation failed/);

diff --git a/...t-stringbytes-external-exceed-max-by-2.js → ...t-stringbytes-external-exceed-max-by-2.js b/...t-stringbytes-external-exceed-max-by-2.js → ...t-stringbytes-external-exceed-max-by-2.js
@@ -1,7 +1,7 @@
 'use strict';
-// Flags: --expose-gc
 
-const common = require('../common');
+const common = require('../../common');
+const binding = require('./build/Release/binding');
 const assert = require('assert');
 
 const skipMessage =
@@ -10,23 +10,25 @@ if (!common.enoughTestMem) {
   console.log(skipMessage);
   return;
 }
-assert(typeof gc === 'function', 'Run this test with --expose-gc');
 
 // v8 fails silently if string length > v8::String::kMaxLength
 // v8::String::kMaxLength defined in v8.h
 const kStringMaxLength = process.binding('buffer').kStringMaxLength;
 
 try {
   var buf = Buffer.allocUnsafe(kStringMaxLength + 2);
-  // Try to allocate memory first then force gc so future allocations succeed.
-  Buffer.allocUnsafe(2 * kStringMaxLength);
-  gc();
 } catch (e) {
   // If the exception is not due to memory confinement then rethrow it.
   if (e.message !== 'Array buffer allocation failed') throw (e);
   console.log(skipMessage);
   return;
 }
 
+// Ensure we have enough memory available for future allocations to succeed.
+if (!binding.ensureAllocation(2 * kStringMaxLength)) {
+  console.log(skipMessage);
+  return;
+}
+
 const maxString = buf.toString('utf16le');
 assert.equal(maxString.length, (kStringMaxLength + 2) / 2);
diff --git a/...l/test-stringbytes-external-exceed-max.js → ...x/test-stringbytes-external-exceed-max.js b/...l/test-stringbytes-external-exceed-max.js → ...x/test-stringbytes-external-exceed-max.js
@@ -1,7 +1,7 @@
 'use strict';
-// Flags: --expose-gc
 
-const common = require('../common');
+const common = require('../../common');
+const binding = require('./build/Release/binding');
 const assert = require('assert');
 
 const skipMessage =
@@ -10,24 +10,26 @@ if (!common.enoughTestMem) {
   console.log(skipMessage);
   return;
 }
-assert(typeof gc === 'function', 'Run this test with --expose-gc');
 
 // v8 fails silently if string length > v8::String::kMaxLength
 // v8::String::kMaxLength defined in v8.h
 const kStringMaxLength = process.binding('buffer').kStringMaxLength;
 
 try {
   var buf = Buffer.allocUnsafe(kStringMaxLength * 2 + 2);
-  // Try to allocate memory first then force gc so future allocations succeed.
-  Buffer.allocUnsafe(2 * kStringMaxLength);
-  gc();
 } catch (e) {
   // If the exception is not due to memory confinement then rethrow it.
   if (e.message !== 'Array buffer allocation failed') throw (e);
   console.log(skipMessage);
   return;
 }
 
+// Ensure we have enough memory available for future allocations to succeed.
+if (!binding.ensureAllocation(2 * kStringMaxLength)) {
+  console.log(skipMessage);
+  return;
+}
+
 assert.throws(function() {
   buf.toString('utf16le');
 }, /"toString\(\)" failed/);