Split the object test into smaller tests (get/set/delete/hasOwnProperty)

test/binding.gyp

@@ -9,7 +9,11 @@
         'external.cc',
         'function.cc',
         'name.cc',
-        'object.cc',
+        'object/delete_property.cc',
+        'object/get_property.cc',
+        'object/has_own_property.cc',
+        'object/object.cc',
+        'object/set_property.cc',
         'promise.cc',
         'typedarray.cc',
         'objectwrap.cc',

test/index.js

@@ -5,6 +5,8 @@ process.config.target_defaults.default_configuration =
     .readdirSync(require('path').join(__dirname, 'build'))
     .filter((item) => (item === 'Debug' || item === 'Release'))[0];
 
+// FIXME: We might need a way to load test modules automatically without
+// explicit declaration as follows.
 let testModules = [
   'arraybuffer',
   'asyncworker',
@@ -13,7 +15,11 @@ let testModules = [
   'external',
   'function',
   'name',
-  'object',
+  'object/delete_property',
+  'object/get_property',
+  'object/has_own_property',
+  'object/object',
+  'object/set_property',
   'promise',
   'typedarray',
   'objectwrap'

test/object/delete_property.cc

@@ -0,0 +1,27 @@
+#include "napi.h"
+
+using namespace Napi;
+
+Value DeletePropertyWithNapiValue(const CallbackInfo& info) {
+  Object obj = info[0].As<Object>();
+  Name key = info[1].As<Name>();
+  return Boolean::New(info.Env(), obj.Delete(static_cast<napi_value>(key)));
+}
+
+Value DeletePropertyWithNapiWrapperValue(const CallbackInfo& info) {
+  Object obj = info[0].As<Object>();
+  Name key = info[1].As<Name>();
+  return Boolean::New(info.Env(), obj.Delete(key));
+}
+
+Value DeletePropertyWithCStyleString(const CallbackInfo& info) {
+  Object obj = info[0].As<Object>();
+  String jsKey = info[1].As<String>();
+  return Boolean::New(info.Env(), obj.Delete(jsKey.Utf8Value().c_str()));
+}
+
+Value DeletePropertyWithCppStyleString(const CallbackInfo& info) {
+  Object obj = info[0].As<Object>();
+  String jsKey = info[1].As<String>();
+  return Boolean::New(info.Env(), obj.Delete(jsKey.Utf8Value()));
+}

test/object/delete_property.js

@@ -0,0 +1,37 @@
+'use strict';
+
+const buildType = process.config.target_defaults.default_configuration;
+const assert = require('assert');
+
+test(require(`../build/${buildType}/binding.node`));
+test(require(`../build/${buildType}/binding_noexcept.node`));
+
+function test(binding) {
+  function testDeleteProperty(nativeDeleteProperty) {
+    const obj = { one: 1, two: 2 };
+    Object.defineProperty(obj, "three", {configurable: false, value: 3});
+    assert.strictEqual(nativeDeleteProperty(obj, 'one'), true);
+    assert.strictEqual(nativeDeleteProperty(obj, 'missing'), true);
+
+    /* Returns true for all cases except when the property is an own non-
+       configurable property, in which case, false is returned in non-strict mode. */
+    assert.strictEqual(nativeDeleteProperty(obj, 'three'), false);
+    assert.deepStrictEqual(obj, { two: 2 });
+  }
+
+  function testShouldThrowErrorIfKeyIsInvalid(nativeDeleteProperty) {
+    assert.throws(() => {
+      nativeDeleteProperty(undefined, 'test');
+    }, /object was expected/);
+  }
+
+  testDeleteProperty(binding.object.deletePropertyWithNapiValue);
+  testDeleteProperty(binding.object.deletePropertyWithNapiWrapperValue);
+  testDeleteProperty(binding.object.deletePropertyWithCStyleString);
+  testDeleteProperty(binding.object.deletePropertyWithCppStyleString);
+
+  testShouldThrowErrorIfKeyIsInvalid(binding.object.deletePropertyWithNapiValue);
+  testShouldThrowErrorIfKeyIsInvalid(binding.object.deletePropertyWithNapiWrapperValue);
+  testShouldThrowErrorIfKeyIsInvalid(binding.object.deletePropertyWithCStyleString);
+  testShouldThrowErrorIfKeyIsInvalid(binding.object.deletePropertyWithCppStyleString);
+}

test/object/get_property.cc

@@ -0,0 +1,27 @@
+#include "napi.h"
+
+using namespace Napi;
+
+Value GetPropertyWithNapiValue(const CallbackInfo& info) {
+  Object obj = info[0].As<Object>();
+  Name key = info[1].As<Name>();
+  return obj.Get(static_cast<napi_value>(key));
+}
+
+Value GetPropertyWithNapiWrapperValue(const CallbackInfo& info) {
+  Object obj = info[0].As<Object>();
+  Name key = info[1].As<Name>();
+  return obj.Get(key);
+}
+
+Value GetPropertyWithCStyleString(const CallbackInfo& info) {
+  Object obj = info[0].As<Object>();
+  String jsKey = info[1].As<String>();
+  return obj.Get(jsKey.Utf8Value().c_str());
+}
+
+Value GetPropertyWithCppStyleString(const CallbackInfo& info) {
+  Object obj = info[0].As<Object>();
+  String jsKey = info[1].As<String>();
+  return obj.Get(jsKey.Utf8Value());
+}

test/object/get_property.js

@@ -0,0 +1,30 @@
+'use strict';
+
+const buildType = process.config.target_defaults.default_configuration;
+const assert = require('assert');
+
+test(require(`../build/${buildType}/binding.node`));
+test(require(`../build/${buildType}/binding_noexcept.node`));
+
+function test(binding) {
+  function testGetProperty(nativeGetProperty) {
+    const obj = { test: 1 };
+    assert.strictEqual(nativeGetProperty(obj, 'test'), 1);
+  }
+
+  function testShouldThrowErrorIfKeyIsInvalid(nativeGetProperty) {
+    assert.throws(() => {
+      nativeGetProperty(undefined, 'test');
+    }, /object was expected/);
+  }
+
+  testGetProperty(binding.object.getPropertyWithNapiValue);
+  testGetProperty(binding.object.getPropertyWithNapiWrapperValue);
+  testGetProperty(binding.object.getPropertyWithCStyleString);
+  testGetProperty(binding.object.getPropertyWithCppStyleString);
+
+  testShouldThrowErrorIfKeyIsInvalid(binding.object.getPropertyWithNapiValue);
+  testShouldThrowErrorIfKeyIsInvalid(binding.object.getPropertyWithNapiWrapperValue);
+  testShouldThrowErrorIfKeyIsInvalid(binding.object.getPropertyWithCStyleString);
+  testShouldThrowErrorIfKeyIsInvalid(binding.object.getPropertyWithCppStyleString);
+}

test/object/has_own_property.cc

@@ -0,0 +1,27 @@
+#include "napi.h"
+
+using namespace Napi;
+
+Value HasOwnPropertyWithNapiValue(const CallbackInfo& info) {
+  Object obj = info[0].As<Object>();
+  Name key = info[1].As<Name>();
+  return Boolean::New(info.Env(), obj.HasOwnProperty(static_cast<napi_value>(key)));
+}
+
+Value HasOwnPropertyWithNapiWrapperValue(const CallbackInfo& info) {
+  Object obj = info[0].As<Object>();
+  Name key = info[1].As<Name>();
+  return Boolean::New(info.Env(), obj.HasOwnProperty(key));
+}
+
+Value HasOwnPropertyWithCStyleString(const CallbackInfo& info) {
+  Object obj = info[0].As<Object>();
+  String jsKey = info[1].As<String>();
+  return Boolean::New(info.Env(), obj.HasOwnProperty(jsKey.Utf8Value().c_str()));
+}
+
+Value HasOwnPropertyWithCppStyleString(const CallbackInfo& info) {
+  Object obj = info[0].As<Object>();
+  String jsKey = info[1].As<String>();
+  return Boolean::New(info.Env(), obj.HasOwnProperty(jsKey.Utf8Value()));
+}

test/object/has_own_property.js

@@ -0,0 +1,36 @@
+'use strict';
+
+const buildType = process.config.target_defaults.default_configuration;
+const assert = require('assert');
+
+test(require(`../build/${buildType}/binding.node`));
+test(require(`../build/${buildType}/binding_noexcept.node`));
+
+function test(binding) {
+  function testHasOwnProperty(nativeHasOwnProperty) {
+    const obj = { one: 1 };
+
+    Object.defineProperty(obj, 'two', { value: 2 });
+
+    assert.strictEqual(nativeHasOwnProperty(obj, 'one'), true);
+    assert.strictEqual(nativeHasOwnProperty(obj, 'two'), true);
+    assert.strictEqual('toString' in obj, true);
+    assert.strictEqual(nativeHasOwnProperty(obj, 'toString'), false);
+  }
+
+  function testShouldThrowErrorIfKeyIsInvalid(nativeHasOwnProperty) {
+    assert.throws(() => {
+      nativeHasOwnProperty(undefined, 'test');
+    }, /object was expected/);
+  }
+
+  testHasOwnProperty(binding.object.hasOwnPropertyWithNapiValue);
+  testHasOwnProperty(binding.object.hasOwnPropertyWithNapiWrapperValue);
+  testHasOwnProperty(binding.object.hasOwnPropertyWithCStyleString);
+  testHasOwnProperty(binding.object.hasOwnPropertyWithCppStyleString);
+
+  testShouldThrowErrorIfKeyIsInvalid(binding.object.hasOwnPropertyWithNapiValue);
+  testShouldThrowErrorIfKeyIsInvalid(binding.object.hasOwnPropertyWithNapiWrapperValue);
+  testShouldThrowErrorIfKeyIsInvalid(binding.object.hasOwnPropertyWithCStyleString);
+  testShouldThrowErrorIfKeyIsInvalid(binding.object.hasOwnPropertyWithCppStyleString);
+}

test/object.cc → test/object/object.cc

@@ -2,6 +2,30 @@
 
 using namespace Napi;
 
+// Native wrappers for testing Object::Get()
+Value GetPropertyWithNapiValue(const CallbackInfo& info);
+Value GetPropertyWithNapiWrapperValue(const CallbackInfo& info);
+Value GetPropertyWithCStyleString(const CallbackInfo& info);
+Value GetPropertyWithCppStyleString(const CallbackInfo& info);
+
+// Native wrappers for testing Object::Set()
+void SetPropertyWithNapiValue(const CallbackInfo& info);
+void SetPropertyWithNapiWrapperValue(const CallbackInfo& info);
+void SetPropertyWithCStyleString(const CallbackInfo& info);
+void SetPropertyWithCppStyleString(const CallbackInfo& info);
+
+// Native wrappers for testing Object::Delete()
+Value DeletePropertyWithNapiValue(const CallbackInfo& info);
+Value DeletePropertyWithNapiWrapperValue(const CallbackInfo& info);
+Value DeletePropertyWithCStyleString(const CallbackInfo& info);
+Value DeletePropertyWithCppStyleString(const CallbackInfo& info);
+
+// Native wrappers for testing Object::HasOwnProperty()
+Value HasOwnPropertyWithNapiValue(const CallbackInfo& info);
+Value HasOwnPropertyWithNapiWrapperValue(const CallbackInfo& info);
+Value HasOwnPropertyWithCStyleString(const CallbackInfo& info);
+Value HasOwnPropertyWithCppStyleString(const CallbackInfo& info);
+
 static bool testValue = true;
 
 Value TestGetter(const CallbackInfo& info) {
@@ -88,32 +112,6 @@ void DefineValueProperty(const CallbackInfo& info) {
   obj.DefineProperty(PropertyDescriptor::Value(name, value));
 }
 
-Value GetProperty(const CallbackInfo& info) {
-  Object obj = info[0].As<Object>();
-  Name name = info[1].As<Name>();
-  Value value = obj.Get(name);
-  return value;
-}
-
-void SetProperty(const CallbackInfo& info) {
-  Object obj = info[0].As<Object>();
-  Name name = info[1].As<Name>();
-  Value value = info[2];
-  obj.Set(name, value);
-}
-
-Value DeleteProperty(const CallbackInfo& info) {
-  Object obj = info[0].As<Object>();
-  Name name = info[1].As<Name>();
-  return Boolean::New(info.Env(), obj.Delete(name));
-}
-
-Value HasOwnProperty(const CallbackInfo& info) {
-  Object obj = info[0].As<Object>();
-  Name name = info[1].As<Name>();
-  return Boolean::New(info.Env(), obj.HasOwnProperty(name));
-}
-
 Value CreateObjectUsingMagic(const CallbackInfo& info) {
   Env env = info.Env();
   Object obj = Object::New(env);
@@ -142,10 +140,27 @@ Object InitObject(Env env) {
   exports["GetPropertyNames"] = Function::New(env, GetPropertyNames);
   exports["defineProperties"] = Function::New(env, DefineProperties);
   exports["defineValueProperty"] = Function::New(env, DefineValueProperty);
-  exports["getProperty"] = Function::New(env, GetProperty);
-  exports["setProperty"] = Function::New(env, SetProperty);
-  exports["deleteProperty"] = Function::New(env, DeleteProperty);
-  exports["hasOwnPropertyFromNative"] = Function::New(env, HasOwnProperty);
+
+  exports["getPropertyWithNapiValue"] = Function::New(env, GetPropertyWithNapiValue);
+  exports["getPropertyWithNapiWrapperValue"] = Function::New(env, GetPropertyWithNapiWrapperValue);
+  exports["getPropertyWithCStyleString"] = Function::New(env, GetPropertyWithCStyleString);
+  exports["getPropertyWithCppStyleString"] = Function::New(env, GetPropertyWithCppStyleString);
+
+  exports["setPropertyWithNapiValue"] = Function::New(env, SetPropertyWithNapiValue);
+  exports["setPropertyWithNapiWrapperValue"] = Function::New(env, SetPropertyWithNapiWrapperValue);
+  exports["setPropertyWithCStyleString"] = Function::New(env, SetPropertyWithCStyleString);
+  exports["setPropertyWithCppStyleString"] = Function::New(env, SetPropertyWithCppStyleString);
+
+  exports["deletePropertyWithNapiValue"] = Function::New(env, DeletePropertyWithNapiValue);
+  exports["deletePropertyWithNapiWrapperValue"] = Function::New(env, DeletePropertyWithNapiWrapperValue);
+  exports["deletePropertyWithCStyleString"] = Function::New(env, DeletePropertyWithCStyleString);
+  exports["deletePropertyWithCppStyleString"] = Function::New(env, DeletePropertyWithCppStyleString);
+
+  exports["hasOwnPropertyWithNapiValue"] = Function::New(env, HasOwnPropertyWithNapiValue);
+  exports["hasOwnPropertyWithNapiWrapperValue"] = Function::New(env, HasOwnPropertyWithNapiWrapperValue);
+  exports["hasOwnPropertyWithCStyleString"] = Function::New(env, HasOwnPropertyWithCStyleString);
+  exports["hasOwnPropertyWithCppStyleString"] = Function::New(env, HasOwnPropertyWithCppStyleString);
+
   exports["createObjectUsingMagic"] = Function::New(env, CreateObjectUsingMagic);
 
   return exports;

test/object.js → test/object/object.js

@@ -2,8 +2,8 @@
 const buildType = process.config.target_defaults.default_configuration;
 const assert = require('assert');
 
-test(require(`./build/${buildType}/binding.node`));
-test(require(`./build/${buildType}/binding_noexcept.node`));
+test(require(`../build/${buildType}/binding.node`));
+test(require(`../build/${buildType}/binding_noexcept.node`));
 
 function test(binding) {
   function assertPropertyIs(obj, key, attribute) {
@@ -71,57 +71,12 @@ function test(binding) {
     assert.strictEqual(obj[testSym], 1);
   }
 
-  {
-    const obj = { test: 1 };
-    assert.strictEqual(binding.object.getProperty(obj, 'test'), 1);
-  }
-
-  {
-    const obj = {};
-    binding.object.setProperty(obj, 'test', 1);
-    assert.strictEqual(obj.test, 1);
-  }
-
-  {
-    const obj = { one: 1, two: 2 };
-    Object.defineProperty(obj, "three", {configurable: false, value: 3});
-    assert.strictEqual(binding.object.deleteProperty(obj, 'one'), true);
-    assert.strictEqual(binding.object.deleteProperty(obj, 'missing'), true);
-
-    /* Returns true for all cases except when the property is an own non-
-       configurable property, in which case, false is returned in non-strict mode. */
-    assert.strictEqual(binding.object.deleteProperty(obj, 'three'), false);
-    assert.deepStrictEqual(obj, { two: 2 });
-  }
-
-  {
-    const obj = { one: 1 };
-    Object.defineProperty(obj, 'two', { value: 2 });
-    assert.strictEqual(binding.object.hasOwnPropertyFromNative(obj, 'one'), true);
-    assert.strictEqual(binding.object.hasOwnPropertyFromNative(obj, 'two'), true);
-    assert.strictEqual('toString' in obj, true);
-    assert.strictEqual(binding.object.hasOwnPropertyFromNative(obj, 'toString'), false);
-  }
-
   {
     const obj = {'one': 1, 'two': 2, 'three': 3};
     var arr = binding.object.GetPropertyNames(obj);
     assert.deepStrictEqual(arr, ['one', 'two', 'three'])
   }
 
-  assert.throws(() => {
-    binding.object.getProperty(undefined, 'test');
-  }, /object was expected/);
-  assert.throws(() => {
-    binding.object.setProperty(undefined, 'test', 1);
-  }, /object was expected/);
-  assert.throws(() => {
-    binding.object.deleteProperty(undefined, 'test');
-  }, /object was expected/);
-  assert.throws(() => {
-    binding.object.hasOwnPropertyFromNative(undefined, 'test');
-  }, /object was expected/);
-
   {
     const magicObject = binding.object.createObjectUsingMagic();
     assert.deepStrictEqual(magicObject, {