Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hermes: Fixing a couple of breaking tests #4120

Merged
merged 2 commits into from
Dec 1, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 12 additions & 14 deletions src/js_object_accessor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -554,23 +554,21 @@ struct Unbox<JSEngine, Obj> {

auto current_realm = native_accessor->m_realm;
auto js_object = Value::validated_to_object(native_accessor->m_ctx, value);
auto realm_object = get_internal<JSEngine, RealmObjectClass<JSEngine>>(native_accessor->m_ctx, js_object);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The primary purpose of this change is to move the call to get_internal into the block where we actually know we have an object pointing to a RealmObjectClass instance.


auto is_ros_instance =
auto is_realm_object =
Object::template is_instance<RealmObjectClass<JSEngine>>(native_accessor->m_ctx, js_object);

if (is_ros_instance && realm_object && realm_object->realm() == current_realm) {
return realm_object->obj();
}

if (is_ros_instance && !policy.copy && !policy.update && !policy.create) {
throw std::runtime_error("Realm object is from another Realm");
}

// if our RealmObject isn't in ObjectStore, it's a detached object
// (not in to database), and we can't add it
if (is_ros_instance && !realm_object) {
throw std::runtime_error("Cannot reference a detached instance of Realm.Object");
if (is_realm_object) {
auto realm_object = get_internal<JSEngine, RealmObjectClass<JSEngine>>(native_accessor->m_ctx, js_object);
if (realm_object && realm_object->realm() == current_realm) {
return realm_object->obj();
}
else if (!policy.copy && !policy.update && !policy.create) {
throw std::runtime_error("Realm object is from another Realm");
}
else if (!realm_object) {
throw std::runtime_error("Cannot reference a detached instance of Realm.Object");
}
}

if (!policy.create) {
Expand Down
5 changes: 4 additions & 1 deletion src/jsi/jsi_class.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ class ObjectWrap {
desc.setProperty(env, "value",
globalType(env, "Function")
.call(env, "getter", "setter", R"(
const integerPattern = /^\d+$/;
const integerPattern = /^-?\d+$/;
function getIndex(prop) {
if (typeof prop === "string" && integerPattern.test(prop)) {
return parseInt(prop, 10);
Expand Down Expand Up @@ -372,6 +372,9 @@ class ObjectWrap {
const index = getIndex(prop);
if (Number.isNaN(index)) {
return Reflect.set(...arguments);
} else if (index < 0) {
// This mimics realm::js::validated_positive_index
throw new Error(`Index ${index} cannot be less than zero.`);
} else if (setter) {
return setter(target, index, value);
} else {
Expand Down
26 changes: 12 additions & 14 deletions tests/js/dictionary-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -162,14 +162,13 @@ module.exports = {
TestCase.assertEqual(data.a.y, 2, "Should be an equals to a.y = 2");
TestCase.assertEqual(data.a.z, 4, "Should be an equals to a.z = 4");

let err = new Error("Property must be of type 'number', got (error)");
TestCase.assertThrowsException(
TestCase.assertThrowsContaining(
() => realm.write(() => realm.create(DictIntSchema.name, { a: { c: "error" } })),
err,
"Property must be of type 'number', got (error)",
);
TestCase.assertThrowsException(
TestCase.assertThrowsContaining(
() => realm.write(() => (data.a = "cc")),
new Error("Dictionary.a must be of type 'number{}', got 'string' ('cc')"),
"Dictionary.a must be of type 'number{}', got 'string' ('cc')",
);

realm.close();
Expand All @@ -182,13 +181,9 @@ module.exports = {
a: "wwwww{}",
},
};
let err = new Error(
"Schema validation failed due to the following errors:\n- Property 'Dictionary.a' of type 'dictionary' has unknown object type 'wwwww'",
);
let _defer = () => {
let r = new Realm({ schema: [DictWrongSchema] });
};
TestCase.assertThrowsException(_defer, err);
TestCase.assertThrowsContaining(() => {
new Realm({ schema: [DictWrongSchema] });
}, "Schema validation failed due to the following errors:\n- Property 'Dictionary.a' of type 'dictionary' has unknown object type 'wwwww'");
},

testDictionaryMutability() {
Expand Down Expand Up @@ -746,8 +741,11 @@ module.exports = {

testDictionaryErrorHandling() {
let realm = new Realm({ schema: [DictSchema] });
let err = new Error("Only Realm instances are supported.");
TestCase.assertThrowsException(() => realm.write(() => realm.create(DictSchema.name, { a: { x: {} } })), err);
TestCase.assertThrowsContaining(() => {
realm.write(() => {
realm.create(DictSchema.name, { a: { x: {} } });
});
}, "Only Realm instances are supported.");
realm.write(() => realm.create(DictSchema.name, { a: { x: null } }));
let data = realm.objects(DictSchema.name)[0].a;
TestCase.assertEqual(data.x, null, "Should be an equals to mutable.x = null");
Expand Down
9 changes: 4 additions & 5 deletions tests/js/mixed-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -166,9 +166,9 @@ module.exports = {
testMixedWrongType() {
let realm = new Realm({ schema: [SingleSchema] });

TestCase.assertThrowsException(
TestCase.assertThrowsContaining(
() => realm.write(() => realm.create(SingleSchema.name, { a: Object.create({}) })),
new Error("Only Realm instances are supported."),
"Only Realm instances are supported.",
);
},

Expand Down Expand Up @@ -211,12 +211,11 @@ module.exports = {
TestCase.assertEqual(objectsBefore.length, 0);

// check if the understandable error message is thrown
const error = new Error("A mixed property cannot contain an array of values.");
TestCase.assertThrowsException(() => {
TestCase.assertThrowsContaining(() => {
realm.write(() => {
realm.create("MixedClass", { value: [123, false, "hello"] });
});
}, error);
}, "A mixed property cannot contain an array of values.");

// verify that the transaction has been rolled back
const objectsAfter = realm.objects(MixedSchema.name);
Expand Down
4 changes: 2 additions & 2 deletions tests/js/object-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -773,8 +773,8 @@ module.exports = {
});

// property that does not exist
TestCase.assertThrowsException(() => {
TestCase.assertThrowsContaining(() => {
obj.getPropertyType("foo");
}, new Error("No such property: foo"));
}, "No such property: foo");
},
};
10 changes: 5 additions & 5 deletions tests/js/realm-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -1580,22 +1580,22 @@ module.exports = {
testErrorMessageFromInvalidWrite: function () {
const realm = new Realm({ schema: [schemas.PersonObject] });

TestCase.assertThrowsException(() => {
TestCase.assertThrowsContaining(() => {
realm.write(() => {
const p1 = realm.create("PersonObject", { name: "Ari", age: 10 });
p1.age = "Ten";
});
}, new Error("PersonObject.age must be of type 'number', got 'string' ('Ten')"));
}, "PersonObject.age must be of type 'number', got 'string' ('Ten')");
},

testErrorMessageFromInvalidCreate: function () {
const realm = new Realm({ schema: [schemas.PersonObject] });

TestCase.assertThrowsException(() => {
TestCase.assertThrowsContaining(() => {
realm.write(() => {
const p1 = realm.create("PersonObject", { name: "Ari", age: "Ten" });
realm.create("PersonObject", { name: "Ari", age: "Ten" });
});
}, new Error("PersonObject.age must be of type 'number', got 'string' ('Ten')"));
}, "PersonObject.age must be of type 'number', got 'string' ('Ten')");
},

testValidTypesForListProperties: function () {
Expand Down