Skip to content

Commit e90a80b

Browse files
romandevmhdawson
romandev
authored and
mhdawson
committed
Split the object test into smaller tests
Split the object test into smaller tests (get/set/delete/hasOwnProperty) This change breaks the object test into smaller tests for the following reasons: - Smaller tests are better than very large one. (e.g. readability) - Before this patch, we have missed some test cases. For example, there are four overloading versions of the deleteProperty() method, but we haven't tested them all until now. PR-URL: #183 Reviewed-By: Michael Dawson <[email protected]>
1 parent 82656bb commit e90a80b

12 files changed

+305
-79
lines changed

‎test/binding.gyp‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,11 @@
99
'external.cc',
1010
'function.cc',
1111
'name.cc',
12-
'object.cc',
12+
'object/delete_property.cc',
13+
'object/get_property.cc',
14+
'object/has_own_property.cc',
15+
'object/object.cc',
16+
'object/set_property.cc',
1317
'promise.cc',
1418
'typedarray.cc',
1519
'objectwrap.cc',

‎test/index.js‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ process.config.target_defaults.default_configuration =
55
.readdirSync(require('path').join(__dirname, 'build'))
66
.filter((item) => (item === 'Debug' || item === 'Release'))[0];
77

8+
// FIXME: We might need a way to load test modules automatically without
9+
// explicit declaration as follows.
810
let testModules = [
911
'arraybuffer',
1012
'asyncworker',
@@ -13,7 +15,11 @@ let testModules = [
1315
'external',
1416
'function',
1517
'name',
16-
'object',
18+
'object/delete_property',
19+
'object/get_property',
20+
'object/has_own_property',
21+
'object/object',
22+
'object/set_property',
1723
'promise',
1824
'typedarray',
1925
'objectwrap'

‎test/object/delete_property.cc‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
#include "napi.h"
2+
3+
using namespace Napi;
4+
5+
Value DeletePropertyWithNapiValue(const CallbackInfo& info) {
6+
Object obj = info[0].As<Object>();
7+
Name key = info[1].As<Name>();
8+
return Boolean::New(info.Env(), obj.Delete(static_cast<napi_value>(key)));
9+
}
10+
11+
Value DeletePropertyWithNapiWrapperValue(const CallbackInfo& info) {
12+
Object obj = info[0].As<Object>();
13+
Name key = info[1].As<Name>();
14+
return Boolean::New(info.Env(), obj.Delete(key));
15+
}
16+
17+
Value DeletePropertyWithCStyleString(const CallbackInfo& info) {
18+
Object obj = info[0].As<Object>();
19+
String jsKey = info[1].As<String>();
20+
return Boolean::New(info.Env(), obj.Delete(jsKey.Utf8Value().c_str()));
21+
}
22+
23+
Value DeletePropertyWithCppStyleString(const CallbackInfo& info) {
24+
Object obj = info[0].As<Object>();
25+
String jsKey = info[1].As<String>();
26+
return Boolean::New(info.Env(), obj.Delete(jsKey.Utf8Value()));
27+
}

‎test/object/delete_property.js‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
'use strict';
2+
3+
const buildType = process.config.target_defaults.default_configuration;
4+
const assert = require('assert');
5+
6+
test(require(`../build/${buildType}/binding.node`));
7+
test(require(`../build/${buildType}/binding_noexcept.node`));
8+
9+
function test(binding) {
10+
function testDeleteProperty(nativeDeleteProperty) {
11+
const obj = { one: 1, two: 2 };
12+
Object.defineProperty(obj, "three", {configurable: false, value: 3});
13+
assert.strictEqual(nativeDeleteProperty(obj, 'one'), true);
14+
assert.strictEqual(nativeDeleteProperty(obj, 'missing'), true);
15+
16+
/* Returns true for all cases except when the property is an own non-
17+
configurable property, in which case, false is returned in non-strict mode. */
18+
assert.strictEqual(nativeDeleteProperty(obj, 'three'), false);
19+
assert.deepStrictEqual(obj, { two: 2 });
20+
}
21+
22+
function testShouldThrowErrorIfKeyIsInvalid(nativeDeleteProperty) {
23+
assert.throws(() => {
24+
nativeDeleteProperty(undefined, 'test');
25+
}, /object was expected/);
26+
}
27+
28+
testDeleteProperty(binding.object.deletePropertyWithNapiValue);
29+
testDeleteProperty(binding.object.deletePropertyWithNapiWrapperValue);
30+
testDeleteProperty(binding.object.deletePropertyWithCStyleString);
31+
testDeleteProperty(binding.object.deletePropertyWithCppStyleString);
32+
33+
testShouldThrowErrorIfKeyIsInvalid(binding.object.deletePropertyWithNapiValue);
34+
testShouldThrowErrorIfKeyIsInvalid(binding.object.deletePropertyWithNapiWrapperValue);
35+
testShouldThrowErrorIfKeyIsInvalid(binding.object.deletePropertyWithCStyleString);
36+
testShouldThrowErrorIfKeyIsInvalid(binding.object.deletePropertyWithCppStyleString);
37+
}

‎test/object/get_property.cc‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
#include "napi.h"
2+
3+
using namespace Napi;
4+
5+
Value GetPropertyWithNapiValue(const CallbackInfo& info) {
6+
Object obj = info[0].As<Object>();
7+
Name key = info[1].As<Name>();
8+
return obj.Get(static_cast<napi_value>(key));
9+
}
10+
11+
Value GetPropertyWithNapiWrapperValue(const CallbackInfo& info) {
12+
Object obj = info[0].As<Object>();
13+
Name key = info[1].As<Name>();
14+
return obj.Get(key);
15+
}
16+
17+
Value GetPropertyWithCStyleString(const CallbackInfo& info) {
18+
Object obj = info[0].As<Object>();
19+
String jsKey = info[1].As<String>();
20+
return obj.Get(jsKey.Utf8Value().c_str());
21+
}
22+
23+
Value GetPropertyWithCppStyleString(const CallbackInfo& info) {
24+
Object obj = info[0].As<Object>();
25+
String jsKey = info[1].As<String>();
26+
return obj.Get(jsKey.Utf8Value());
27+
}

‎test/object/get_property.js‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
'use strict';
2+
3+
const buildType = process.config.target_defaults.default_configuration;
4+
const assert = require('assert');
5+
6+
test(require(`../build/${buildType}/binding.node`));
7+
test(require(`../build/${buildType}/binding_noexcept.node`));
8+
9+
function test(binding) {
10+
function testGetProperty(nativeGetProperty) {
11+
const obj = { test: 1 };
12+
assert.strictEqual(nativeGetProperty(obj, 'test'), 1);
13+
}
14+
15+
function testShouldThrowErrorIfKeyIsInvalid(nativeGetProperty) {
16+
assert.throws(() => {
17+
nativeGetProperty(undefined, 'test');
18+
}, /object was expected/);
19+
}
20+
21+
testGetProperty(binding.object.getPropertyWithNapiValue);
22+
testGetProperty(binding.object.getPropertyWithNapiWrapperValue);
23+
testGetProperty(binding.object.getPropertyWithCStyleString);
24+
testGetProperty(binding.object.getPropertyWithCppStyleString);
25+
26+
testShouldThrowErrorIfKeyIsInvalid(binding.object.getPropertyWithNapiValue);
27+
testShouldThrowErrorIfKeyIsInvalid(binding.object.getPropertyWithNapiWrapperValue);
28+
testShouldThrowErrorIfKeyIsInvalid(binding.object.getPropertyWithCStyleString);
29+
testShouldThrowErrorIfKeyIsInvalid(binding.object.getPropertyWithCppStyleString);
30+
}

‎test/object/has_own_property.cc‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
#include "napi.h"
2+
3+
using namespace Napi;
4+
5+
Value HasOwnPropertyWithNapiValue(const CallbackInfo& info) {
6+
Object obj = info[0].As<Object>();
7+
Name key = info[1].As<Name>();
8+
return Boolean::New(info.Env(), obj.HasOwnProperty(static_cast<napi_value>(key)));
9+
}
10+
11+
Value HasOwnPropertyWithNapiWrapperValue(const CallbackInfo& info) {
12+
Object obj = info[0].As<Object>();
13+
Name key = info[1].As<Name>();
14+
return Boolean::New(info.Env(), obj.HasOwnProperty(key));
15+
}
16+
17+
Value HasOwnPropertyWithCStyleString(const CallbackInfo& info) {
18+
Object obj = info[0].As<Object>();
19+
String jsKey = info[1].As<String>();
20+
return Boolean::New(info.Env(), obj.HasOwnProperty(jsKey.Utf8Value().c_str()));
21+
}
22+
23+
Value HasOwnPropertyWithCppStyleString(const CallbackInfo& info) {
24+
Object obj = info[0].As<Object>();
25+
String jsKey = info[1].As<String>();
26+
return Boolean::New(info.Env(), obj.HasOwnProperty(jsKey.Utf8Value()));
27+
}

‎test/object/has_own_property.js‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
'use strict';
2+
3+
const buildType = process.config.target_defaults.default_configuration;
4+
const assert = require('assert');
5+
6+
test(require(`../build/${buildType}/binding.node`));
7+
test(require(`../build/${buildType}/binding_noexcept.node`));
8+
9+
function test(binding) {
10+
function testHasOwnProperty(nativeHasOwnProperty) {
11+
const obj = { one: 1 };
12+
13+
Object.defineProperty(obj, 'two', { value: 2 });
14+
15+
assert.strictEqual(nativeHasOwnProperty(obj, 'one'), true);
16+
assert.strictEqual(nativeHasOwnProperty(obj, 'two'), true);
17+
assert.strictEqual('toString' in obj, true);
18+
assert.strictEqual(nativeHasOwnProperty(obj, 'toString'), false);
19+
}
20+
21+
function testShouldThrowErrorIfKeyIsInvalid(nativeHasOwnProperty) {
22+
assert.throws(() => {
23+
nativeHasOwnProperty(undefined, 'test');
24+
}, /object was expected/);
25+
}
26+
27+
testHasOwnProperty(binding.object.hasOwnPropertyWithNapiValue);
28+
testHasOwnProperty(binding.object.hasOwnPropertyWithNapiWrapperValue);
29+
testHasOwnProperty(binding.object.hasOwnPropertyWithCStyleString);
30+
testHasOwnProperty(binding.object.hasOwnPropertyWithCppStyleString);
31+
32+
testShouldThrowErrorIfKeyIsInvalid(binding.object.hasOwnPropertyWithNapiValue);
33+
testShouldThrowErrorIfKeyIsInvalid(binding.object.hasOwnPropertyWithNapiWrapperValue);
34+
testShouldThrowErrorIfKeyIsInvalid(binding.object.hasOwnPropertyWithCStyleString);
35+
testShouldThrowErrorIfKeyIsInvalid(binding.object.hasOwnPropertyWithCppStyleString);
36+
}

‎test/object.cc‎ renamed to ‎test/object/object.cc‎

Lines changed: 45 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,30 @@
22

33
using namespace Napi;
44

5+
// Native wrappers for testing Object::Get()
6+
Value GetPropertyWithNapiValue(const CallbackInfo& info);
7+
Value GetPropertyWithNapiWrapperValue(const CallbackInfo& info);
8+
Value GetPropertyWithCStyleString(const CallbackInfo& info);
9+
Value GetPropertyWithCppStyleString(const CallbackInfo& info);
10+
11+
// Native wrappers for testing Object::Set()
12+
void SetPropertyWithNapiValue(const CallbackInfo& info);
13+
void SetPropertyWithNapiWrapperValue(const CallbackInfo& info);
14+
void SetPropertyWithCStyleString(const CallbackInfo& info);
15+
void SetPropertyWithCppStyleString(const CallbackInfo& info);
16+
17+
// Native wrappers for testing Object::Delete()
18+
Value DeletePropertyWithNapiValue(const CallbackInfo& info);
19+
Value DeletePropertyWithNapiWrapperValue(const CallbackInfo& info);
20+
Value DeletePropertyWithCStyleString(const CallbackInfo& info);
21+
Value DeletePropertyWithCppStyleString(const CallbackInfo& info);
22+
23+
// Native wrappers for testing Object::HasOwnProperty()
24+
Value HasOwnPropertyWithNapiValue(const CallbackInfo& info);
25+
Value HasOwnPropertyWithNapiWrapperValue(const CallbackInfo& info);
26+
Value HasOwnPropertyWithCStyleString(const CallbackInfo& info);
27+
Value HasOwnPropertyWithCppStyleString(const CallbackInfo& info);
28+
529
static bool testValue = true;
630

731
Value TestGetter(const CallbackInfo& info) {
@@ -88,32 +112,6 @@ void DefineValueProperty(const CallbackInfo& info) {
88112
obj.DefineProperty(PropertyDescriptor::Value(name, value));
89113
}
90114

91-
Value GetProperty(const CallbackInfo& info) {
92-
Object obj = info[0].As<Object>();
93-
Name name = info[1].As<Name>();
94-
Value value = obj.Get(name);
95-
return value;
96-
}
97-
98-
void SetProperty(const CallbackInfo& info) {
99-
Object obj = info[0].As<Object>();
100-
Name name = info[1].As<Name>();
101-
Value value = info[2];
102-
obj.Set(name, value);
103-
}
104-
105-
Value DeleteProperty(const CallbackInfo& info) {
106-
Object obj = info[0].As<Object>();
107-
Name name = info[1].As<Name>();
108-
return Boolean::New(info.Env(), obj.Delete(name));
109-
}
110-
111-
Value HasOwnProperty(const CallbackInfo& info) {
112-
Object obj = info[0].As<Object>();
113-
Name name = info[1].As<Name>();
114-
return Boolean::New(info.Env(), obj.HasOwnProperty(name));
115-
}
116-
117115
Value CreateObjectUsingMagic(const CallbackInfo& info) {
118116
Env env = info.Env();
119117
Object obj = Object::New(env);
@@ -142,10 +140,27 @@ Object InitObject(Env env) {
142140
exports["GetPropertyNames"] = Function::New(env, GetPropertyNames);
143141
exports["defineProperties"] = Function::New(env, DefineProperties);
144142
exports["defineValueProperty"] = Function::New(env, DefineValueProperty);
145-
exports["getProperty"] = Function::New(env, GetProperty);
146-
exports["setProperty"] = Function::New(env, SetProperty);
147-
exports["deleteProperty"] = Function::New(env, DeleteProperty);
148-
exports["hasOwnPropertyFromNative"] = Function::New(env, HasOwnProperty);
143+
144+
exports["getPropertyWithNapiValue"] = Function::New(env, GetPropertyWithNapiValue);
145+
exports["getPropertyWithNapiWrapperValue"] = Function::New(env, GetPropertyWithNapiWrapperValue);
146+
exports["getPropertyWithCStyleString"] = Function::New(env, GetPropertyWithCStyleString);
147+
exports["getPropertyWithCppStyleString"] = Function::New(env, GetPropertyWithCppStyleString);
148+
149+
exports["setPropertyWithNapiValue"] = Function::New(env, SetPropertyWithNapiValue);
150+
exports["setPropertyWithNapiWrapperValue"] = Function::New(env, SetPropertyWithNapiWrapperValue);
151+
exports["setPropertyWithCStyleString"] = Function::New(env, SetPropertyWithCStyleString);
152+
exports["setPropertyWithCppStyleString"] = Function::New(env, SetPropertyWithCppStyleString);
153+
154+
exports["deletePropertyWithNapiValue"] = Function::New(env, DeletePropertyWithNapiValue);
155+
exports["deletePropertyWithNapiWrapperValue"] = Function::New(env, DeletePropertyWithNapiWrapperValue);
156+
exports["deletePropertyWithCStyleString"] = Function::New(env, DeletePropertyWithCStyleString);
157+
exports["deletePropertyWithCppStyleString"] = Function::New(env, DeletePropertyWithCppStyleString);
158+
159+
exports["hasOwnPropertyWithNapiValue"] = Function::New(env, HasOwnPropertyWithNapiValue);
160+
exports["hasOwnPropertyWithNapiWrapperValue"] = Function::New(env, HasOwnPropertyWithNapiWrapperValue);
161+
exports["hasOwnPropertyWithCStyleString"] = Function::New(env, HasOwnPropertyWithCStyleString);
162+
exports["hasOwnPropertyWithCppStyleString"] = Function::New(env, HasOwnPropertyWithCppStyleString);
163+
149164
exports["createObjectUsingMagic"] = Function::New(env, CreateObjectUsingMagic);
150165

151166
return exports;

‎test/object.js‎ renamed to ‎test/object/object.js‎

Lines changed: 2 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@
22
const buildType = process.config.target_defaults.default_configuration;
33
const assert = require('assert');
44

5-
test(require(`./build/${buildType}/binding.node`));
6-
test(require(`./build/${buildType}/binding_noexcept.node`));
5+
test(require(`../build/${buildType}/binding.node`));
6+
test(require(`../build/${buildType}/binding_noexcept.node`));
77

88
function test(binding) {
99
function assertPropertyIs(obj, key, attribute) {
@@ -71,57 +71,12 @@ function test(binding) {
7171
assert.strictEqual(obj[testSym], 1);
7272
}
7373

74-
{
75-
const obj = { test: 1 };
76-
assert.strictEqual(binding.object.getProperty(obj, 'test'), 1);
77-
}
78-
79-
{
80-
const obj = {};
81-
binding.object.setProperty(obj, 'test', 1);
82-
assert.strictEqual(obj.test, 1);
83-
}
84-
85-
{
86-
const obj = { one: 1, two: 2 };
87-
Object.defineProperty(obj, "three", {configurable: false, value: 3});
88-
assert.strictEqual(binding.object.deleteProperty(obj, 'one'), true);
89-
assert.strictEqual(binding.object.deleteProperty(obj, 'missing'), true);
90-
91-
/* Returns true for all cases except when the property is an own non-
92-
configurable property, in which case, false is returned in non-strict mode. */
93-
assert.strictEqual(binding.object.deleteProperty(obj, 'three'), false);
94-
assert.deepStrictEqual(obj, { two: 2 });
95-
}
96-
97-
{
98-
const obj = { one: 1 };
99-
Object.defineProperty(obj, 'two', { value: 2 });
100-
assert.strictEqual(binding.object.hasOwnPropertyFromNative(obj, 'one'), true);
101-
assert.strictEqual(binding.object.hasOwnPropertyFromNative(obj, 'two'), true);
102-
assert.strictEqual('toString' in obj, true);
103-
assert.strictEqual(binding.object.hasOwnPropertyFromNative(obj, 'toString'), false);
104-
}
105-
10674
{
10775
const obj = {'one': 1, 'two': 2, 'three': 3};
10876
var arr = binding.object.GetPropertyNames(obj);
10977
assert.deepStrictEqual(arr, ['one', 'two', 'three'])
11078
}
11179

112-
assert.throws(() => {
113-
binding.object.getProperty(undefined, 'test');
114-
}, /object was expected/);
115-
assert.throws(() => {
116-
binding.object.setProperty(undefined, 'test', 1);
117-
}, /object was expected/);
118-
assert.throws(() => {
119-
binding.object.deleteProperty(undefined, 'test');
120-
}, /object was expected/);
121-
assert.throws(() => {
122-
binding.object.hasOwnPropertyFromNative(undefined, 'test');
123-
}, /object was expected/);
124-
12580
{
12681
const magicObject = binding.object.createObjectUsingMagic();
12782
assert.deepStrictEqual(magicObject, {

0 commit comments

Comments
 (0)