Updates for review feedback

 - Make test cases more isolated with block scoping
   and reset finalize count
 - Fix order of actual, expected in asserts

Author: jasongin

test/addons-napi/test_reference/test.js

@@ -9,64 +9,79 @@ const test_reference = require(`./build/${common.buildType}/test_reference`);
 // This test script uses external values with finalizer callbacks
 // in order to track when values get garbage-collected. Each invocation
 // of a finalizer callback increments the finalizeCount property.
-assert.strictEqual(0, test_reference.finalizeCount);
+assert.strictEqual(test_reference.finalizeCount, 0);
 
-// External value without a finalizer
-let value = test_reference.createExternal();
-assert.strictEqual(typeof value, 'object');
-test_reference.checkExternal(value);
-value = null;
-global.gc();
-assert.strictEqual(0, test_reference.finalizeCount);
+{
+  // External value without a finalizer
+  let value = test_reference.createExternal();
+  assert.strictEqual(test_reference.finalizeCount, 0);
+  assert.strictEqual(typeof value, 'object');
+  test_reference.checkExternal(value);
+  value = null;
+  global.gc();
+  assert.strictEqual(test_reference.finalizeCount, 0);
+}
 
-// External value with a finalizer
-value = test_reference.createExternalWithFinalize();
-assert.strictEqual(typeof value, 'object');
-test_reference.checkExternal(value);
-value = null;
-global.gc();
-assert.strictEqual(1, test_reference.finalizeCount);
+{
+  // External value with a finalizer
+  let value = test_reference.createExternalWithFinalize();
+  assert.strictEqual(test_reference.finalizeCount, 0);
+  assert.strictEqual(typeof value, 'object');
+  test_reference.checkExternal(value);
+  value = null;
+  global.gc();
+  assert.strictEqual(test_reference.finalizeCount, 1);
+}
 
-// Weak reference
-value = test_reference.createExternalWithFinalize();
-test_reference.createReference(value, 0);
-assert.strictEqual(test_reference.referenceValue, value);
-value = null;
-global.gc(); // Value should be GC'd because there is only a weak ref
-assert.strictEqual(test_reference.referenceValue, undefined);
-assert.strictEqual(2, test_reference.finalizeCount);
-test_reference.deleteReference();
+{
+  // Weak reference
+  let value = test_reference.createExternalWithFinalize();
+  assert.strictEqual(test_reference.finalizeCount, 0);
+  test_reference.createReference(value, 0);
+  assert.strictEqual(test_reference.referenceValue, value);
+  value = null;
+  global.gc(); // Value should be GC'd because there is only a weak ref
+  assert.strictEqual(test_reference.referenceValue, undefined);
+  assert.strictEqual(test_reference.finalizeCount, 1);
+  test_reference.deleteReference();
+}
 
-// Strong reference
-value = test_reference.createExternalWithFinalize();
-test_reference.createReference(value, 1);
-assert.strictEqual(test_reference.referenceValue, value);
-value = null;
-global.gc(); // Value should NOT be GC'd because there is a strong ref
-assert.strictEqual(2, test_reference.finalizeCount);
-test_reference.deleteReference();
-global.gc(); // Value should be GC'd because the strong ref was deleted
-assert.strictEqual(3, test_reference.finalizeCount);
+{
+  // Strong reference
+  let value = test_reference.createExternalWithFinalize();
+  assert.strictEqual(test_reference.finalizeCount, 0);
+  test_reference.createReference(value, 1);
+  assert.strictEqual(test_reference.referenceValue, value);
+  value = null;
+  global.gc(); // Value should NOT be GC'd because there is a strong ref
+  assert.strictEqual(test_reference.finalizeCount, 0);
+  test_reference.deleteReference();
+  global.gc(); // Value should be GC'd because the strong ref was deleted
+  assert.strictEqual(test_reference.finalizeCount, 1);
+}
 
-// Strong reference, increment then decrement to weak reference
-value = test_reference.createExternalWithFinalize();
-test_reference.createReference(value, 1);
-value = null;
-global.gc(); // Value should NOT be GC'd because there is a strong ref
-assert.strictEqual(3, test_reference.finalizeCount);
+{
+  // Strong reference, increment then decrement to weak reference
+  let value = test_reference.createExternalWithFinalize();
+  assert.strictEqual(test_reference.finalizeCount, 0);
+  test_reference.createReference(value, 1);
+  value = null;
+  global.gc(); // Value should NOT be GC'd because there is a strong ref
+  assert.strictEqual(test_reference.finalizeCount, 0);
 
-assert.strictEqual(test_reference.incrementRefcount(), 2);
-global.gc(); // Value should NOT be GC'd because there is a strong ref
-assert.strictEqual(3, test_reference.finalizeCount);
+  assert.strictEqual(test_reference.incrementRefcount(), 2);
+  global.gc(); // Value should NOT be GC'd because there is a strong ref
+  assert.strictEqual(test_reference.finalizeCount, 0);
 
-assert.strictEqual(test_reference.decrementRefcount(), 1);
-global.gc(); // Value should NOT be GC'd because there is a strong ref
-assert.strictEqual(3, test_reference.finalizeCount);
+  assert.strictEqual(test_reference.decrementRefcount(), 1);
+  global.gc(); // Value should NOT be GC'd because there is a strong ref
+  assert.strictEqual(test_reference.finalizeCount, 0);
 
-assert.strictEqual(test_reference.decrementRefcount(), 0);
-global.gc(); // Value should be GC'd because the ref is now weak!
-assert.strictEqual(4, test_reference.finalizeCount);
+  assert.strictEqual(test_reference.decrementRefcount(), 0);
+  global.gc(); // Value should be GC'd because the ref is now weak!
+  assert.strictEqual(test_reference.finalizeCount, 1);
 
-test_reference.deleteReference();
-global.gc(); // Value was already GC'd
-assert.strictEqual(4, test_reference.finalizeCount);
+  test_reference.deleteReference();
+  global.gc(); // Value was already GC'd
+  assert.strictEqual(test_reference.finalizeCount, 1);
+}

test/addons-napi/test_reference/test_reference.c

@@ -27,6 +27,8 @@ napi_value CreateExternal(napi_env env, napi_callback_info info) {
                          NULL, /* finalize_cb */
                          NULL, /* finalize_hint */
                          &result));
+
+  finalize_count = 0;
   return result;
 }
 
@@ -41,6 +43,8 @@ napi_value CreateExternalWithFinalize(napi_env env, napi_callback_info info) {
                          FinalizeExternal,
                          NULL, /* finalize_hint */
                          &result));
+
+  finalize_count = 0;
   return result;
 }