From a0bce8031e5d7e8f2c6efcc53061d1ac17152406 Mon Sep 17 00:00:00 2001 From: Ben Christel Date: Tue, 19 Jul 2016 14:31:58 -0700 Subject: [PATCH 1/3] Test comparison of objects from different frames --- spec/core/matchers/matchersUtilSpec.js | 18 ++++++++++++++++++ src/core/matchers/matchersUtil.js | 12 ++++++++++-- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/spec/core/matchers/matchersUtilSpec.js b/spec/core/matchers/matchersUtilSpec.js index d6ab504e..7fd98ad4 100644 --- a/spec/core/matchers/matchersUtilSpec.js +++ b/spec/core/matchers/matchersUtilSpec.js @@ -93,6 +93,24 @@ describe("matchersUtil", function() { expect(jasmineUnderTest.matchersUtil.equals(new Error("foo"), new Error("bar"))).toBe(false); }); + it("fails for objects with different constructors", function() { + function One() {} + function Two() {} + + expect(jasmineUnderTest.matchersUtil.equals(new One(), new Two())).toBe(false); + }); + + if (typeof document === 'object') { + it("passes for equivalent objects from different frames", function() { + var iframe = document.createElement('iframe'); + document.body.appendChild(iframe); + console.log(iframe); + iframe.contentWindow.eval('window.testObject = {}'); + expect(jasmineUnderTest.matchersUtil.equals({}, iframe.contentWindow.testObject)).toBe(true); + document.body.removeChild(iframe); + }); + } + it("passes for Objects that are equivalent (simple case)", function() { expect(jasmineUnderTest.matchersUtil.equals({a: "foo"}, {a: "foo"})).toBe(true); }); diff --git a/src/core/matchers/matchersUtil.js b/src/core/matchers/matchersUtil.js index ef92c1d4..900c0950 100644 --- a/src/core/matchers/matchersUtil.js +++ b/src/core/matchers/matchersUtil.js @@ -178,8 +178,8 @@ getJasmineRequireObj().matchersUtil = function(j$) { // Objects with different constructors are not equivalent, but `Object`s // or `Array`s from different frames are. var aCtor = a.constructor, bCtor = b.constructor; - if (aCtor !== bCtor && !(isFunction(aCtor) && aCtor instanceof aCtor && - isFunction(bCtor) && bCtor instanceof bCtor)) { + if (aCtor !== bCtor && !(isObjectConstructor(aCtor) && + isObjectConstructor(bCtor))) { return false; } } @@ -239,5 +239,13 @@ getJasmineRequireObj().matchersUtil = function(j$) { function isFunction(obj) { return typeof obj === 'function'; } + + function isObjectConstructor(ctor) { + // aCtor instanceof aCtor is true for the Object and Function + // constructors (since a constructor is-a Function and a function is-a + // Object). We don't just compare ctor === Object because the constructor + // might come from a different frame with different globals. + return isFunction(ctor) && ctor instanceof ctor; + } } }; From d9ded15c45d9e50bbafaf8c9fea0b240aa632472 Mon Sep 17 00:00:00 2001 From: Ben Christel Date: Tue, 19 Jul 2016 15:02:08 -0700 Subject: [PATCH 2/3] Move functions in to a higher scope - Prevents them from accessing eq's local vars, which could cause bugs. --- src/core/matchers/matchersUtil.js | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/core/matchers/matchersUtil.js b/src/core/matchers/matchersUtil.js index 900c0950..db1545be 100644 --- a/src/core/matchers/matchersUtil.js +++ b/src/core/matchers/matchersUtil.js @@ -231,21 +231,21 @@ getJasmineRequireObj().matchersUtil = function(j$) { return extraKeys; } + } + + function has(obj, key) { + return Object.prototype.hasOwnProperty.call(obj, key); + } - function has(obj, key) { - return Object.prototype.hasOwnProperty.call(obj, key); - } + function isFunction(obj) { + return typeof obj === 'function'; + } - function isFunction(obj) { - return typeof obj === 'function'; - } - - function isObjectConstructor(ctor) { - // aCtor instanceof aCtor is true for the Object and Function - // constructors (since a constructor is-a Function and a function is-a - // Object). We don't just compare ctor === Object because the constructor - // might come from a different frame with different globals. - return isFunction(ctor) && ctor instanceof ctor; - } + function isObjectConstructor(ctor) { + // aCtor instanceof aCtor is true for the Object and Function + // constructors (since a constructor is-a Function and a function is-a + // Object). We don't just compare ctor === Object because the constructor + // might come from a different frame with different globals. + return isFunction(ctor) && ctor instanceof ctor; } }; From cb1001e6c279b488005d05606ea2f27b635ea346 Mon Sep 17 00:00:00 2001 From: Ben Christel Date: Thu, 21 Jul 2016 12:26:43 -0700 Subject: [PATCH 3/3] De-dup test setup - Move environment-specific tests for toEquals into describe blocks so the check for the environment can be shared --- spec/core/matchers/matchersUtilSpec.js | 134 ++++++++++++------------- 1 file changed, 67 insertions(+), 67 deletions(-) diff --git a/spec/core/matchers/matchersUtilSpec.js b/spec/core/matchers/matchersUtilSpec.js index 7fd98ad4..9cabd1a2 100644 --- a/spec/core/matchers/matchersUtilSpec.js +++ b/spec/core/matchers/matchersUtilSpec.js @@ -100,17 +100,6 @@ describe("matchersUtil", function() { expect(jasmineUnderTest.matchersUtil.equals(new One(), new Two())).toBe(false); }); - if (typeof document === 'object') { - it("passes for equivalent objects from different frames", function() { - var iframe = document.createElement('iframe'); - document.body.appendChild(iframe); - console.log(iframe); - iframe.contentWindow.eval('window.testObject = {}'); - expect(jasmineUnderTest.matchersUtil.equals({}, iframe.contentWindow.testObject)).toBe(true); - document.body.removeChild(iframe); - }); - } - it("passes for Objects that are equivalent (simple case)", function() { expect(jasmineUnderTest.matchersUtil.equals({a: "foo"}, {a: "foo"})).toBe(true); }); @@ -167,70 +156,81 @@ describe("matchersUtil", function() { expect(jasmineUnderTest.matchersUtil.equals(a,b)).toBe(true); }); - it("passes for equivalent DOM nodes", function() { - if (typeof document === 'undefined') { - return; - } - var a = document.createElement("div"); - a.setAttribute("test-attr", "attr-value") - a.appendChild(document.createTextNode('test')); + describe("when running in a browser", function() { + beforeEach(function() { + if (typeof document === 'undefined') { + pending(); + } + }); - var b = document.createElement("div"); - b.setAttribute("test-attr", "attr-value") - b.appendChild(document.createTextNode('test')); + it("passes for equivalent DOM nodes", function() { + var a = document.createElement("div"); + a.setAttribute("test-attr", "attr-value"); + a.appendChild(document.createTextNode('test')); - expect(jasmineUnderTest.matchersUtil.equals(a,b)).toBe(true); + var b = document.createElement("div"); + b.setAttribute("test-attr", "attr-value"); + b.appendChild(document.createTextNode('test')); + + expect(jasmineUnderTest.matchersUtil.equals(a,b)).toBe(true); + }); + + it("passes for equivalent objects from different frames", function() { + var iframe = document.createElement('iframe'); + document.body.appendChild(iframe); + iframe.contentWindow.eval('window.testObject = {}'); + expect(jasmineUnderTest.matchersUtil.equals({}, iframe.contentWindow.testObject)).toBe(true); + document.body.removeChild(iframe); + }); + + it("fails for DOM nodes with different attributes or child nodes", function() { + var a = document.createElement("div"); + a.setAttribute("test-attr", "attr-value") + a.appendChild(document.createTextNode('test')); + + var b = document.createElement("div"); + b.setAttribute("test-attr", "attr-value2") + b.appendChild(document.createTextNode('test')); + + expect(jasmineUnderTest.matchersUtil.equals(a,b)).toBe(false); + + b.setAttribute("test-attr", "attr-value"); + expect(jasmineUnderTest.matchersUtil.equals(a,b)).toBe(true); + + b.appendChild(document.createTextNode('2')); + expect(jasmineUnderTest.matchersUtil.equals(a,b)).toBe(false); + + a.appendChild(document.createTextNode('2')); + expect(jasmineUnderTest.matchersUtil.equals(a,b)).toBe(true); + }); }); - it("fails for DOM nodes with different attributes or child nodes", function() { - if (typeof document === 'undefined') { - return; - } - var a = document.createElement("div"); - a.setAttribute("test-attr", "attr-value") - a.appendChild(document.createTextNode('test')); + describe("when running in Node", function() { + beforeEach(function() { + if (typeof require !== 'function') { + pending(); + } + }); - var b = document.createElement("div"); - b.setAttribute("test-attr", "attr-value2") - b.appendChild(document.createTextNode('test')); + it("passes for equivalent objects from different vm contexts", function() { + var vm = require('vm'); + var sandbox = { + obj: null + }; + vm.runInNewContext('obj = {a: 1, b: 2}', sandbox); - expect(jasmineUnderTest.matchersUtil.equals(a,b)).toBe(false); + expect(jasmineUnderTest.matchersUtil.equals(sandbox.obj, {a: 1, b: 2})).toBe(true); + }); - b.setAttribute("test-attr", "attr-value"); - expect(jasmineUnderTest.matchersUtil.equals(a,b)).toBe(true); + it("passes for equivalent arrays from different vm contexts", function() { + var vm = require('vm'); + var sandbox = { + arr: null + }; + vm.runInNewContext('arr = [1, 2]', sandbox); - b.appendChild(document.createTextNode('2')); - expect(jasmineUnderTest.matchersUtil.equals(a,b)).toBe(false); - - a.appendChild(document.createTextNode('2')); - expect(jasmineUnderTest.matchersUtil.equals(a,b)).toBe(true); - }); - - it("passes for equivalent objects from different vm contexts", function() { - if (typeof require !== 'function') { - return; - } - var vm = require('vm'); - var sandbox = { - obj: null - }; - vm.runInNewContext('obj = {a: 1, b: 2}', sandbox); - - expect(jasmineUnderTest.matchersUtil.equals(sandbox.obj, {a: 1, b: 2})).toBe(true); - }); - - it("passes for equivalent arrays from different vm contexts", function() { - if (typeof require !== 'function') { - return; - } - - var vm = require('vm'); - var sandbox = { - arr: null - }; - vm.runInNewContext('arr = [1, 2]', sandbox); - - expect(jasmineUnderTest.matchersUtil.equals(sandbox.arr, [1, 2])).toBe(true); + expect(jasmineUnderTest.matchersUtil.equals(sandbox.arr, [1, 2])).toBe(true); + }); }); it("passes when Any is used", function() {