From 7ec7fe3427513eb6a0096b5b6cc97e762fadb641 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Wed, 1 Dec 2021 09:00:27 +0100 Subject: [PATCH] Hermes: Fixing a couple of breaking tests (#4120) * Relaxing error message asserting tests * Fixing testListSubscriptSetters --- src/js_object_accessor.hpp | 26 ++++++++++++-------------- src/jsi/jsi_class.hpp | 5 ++++- tests/js/dictionary-tests.js | 26 ++++++++++++-------------- tests/js/mixed-tests.js | 9 ++++----- tests/js/object-tests.js | 4 ++-- tests/js/realm-tests.js | 10 +++++----- 6 files changed, 39 insertions(+), 41 deletions(-) diff --git a/src/js_object_accessor.hpp b/src/js_object_accessor.hpp index 460c0cb066..7a41246c25 100644 --- a/src/js_object_accessor.hpp +++ b/src/js_object_accessor.hpp @@ -554,23 +554,21 @@ struct Unbox { auto current_realm = native_accessor->m_realm; auto js_object = Value::validated_to_object(native_accessor->m_ctx, value); - auto realm_object = get_internal>(native_accessor->m_ctx, js_object); - auto is_ros_instance = + auto is_realm_object = Object::template is_instance>(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>(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) { diff --git a/src/jsi/jsi_class.hpp b/src/jsi/jsi_class.hpp index 9de70254d2..dab474dfe0 100644 --- a/src/jsi/jsi_class.hpp +++ b/src/jsi/jsi_class.hpp @@ -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); @@ -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 { diff --git a/tests/js/dictionary-tests.js b/tests/js/dictionary-tests.js index 70ca4a31ca..b4d906c134 100644 --- a/tests/js/dictionary-tests.js +++ b/tests/js/dictionary-tests.js @@ -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(); @@ -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() { @@ -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"); diff --git a/tests/js/mixed-tests.js b/tests/js/mixed-tests.js index c392e066a3..edc51f51b4 100644 --- a/tests/js/mixed-tests.js +++ b/tests/js/mixed-tests.js @@ -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.", ); }, @@ -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); diff --git a/tests/js/object-tests.js b/tests/js/object-tests.js index 5d93f9e6e8..2c68ee8537 100644 --- a/tests/js/object-tests.js +++ b/tests/js/object-tests.js @@ -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"); }, }; diff --git a/tests/js/realm-tests.js b/tests/js/realm-tests.js index 532ba61905..cc7db32a78 100644 --- a/tests/js/realm-tests.js +++ b/tests/js/realm-tests.js @@ -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 () {