From 3be797c8d8b86cf37f116573bc55b487cf9fcb82 Mon Sep 17 00:00:00 2001 From: Steve Gravrock Date: Tue, 11 Feb 2020 16:25:53 -0800 Subject: [PATCH] Fixed diffs involving jasmine.objectContaining --- lib/jasmine-core/jasmine.js | 51 +++++++++++-------- .../ObjectContainingSpec.js | 3 +- spec/core/matchers/ObjectPathSpec.js | 8 --- spec/core/matchers/toEqualSpec.js | 4 +- src/core/base.js | 4 ++ src/core/matchers/DiffBuilder.js | 23 ++++++++- src/core/matchers/ObjectPath.js | 10 ---- src/core/matchers/matchersUtil.js | 14 ++--- 8 files changed, 62 insertions(+), 55 deletions(-) diff --git a/lib/jasmine-core/jasmine.js b/lib/jasmine-core/jasmine.js index fc466dbe..65146cf8 100644 --- a/lib/jasmine-core/jasmine.js +++ b/lib/jasmine-core/jasmine.js @@ -278,6 +278,10 @@ getJasmineRequireObj().base = function(j$, jasmineGlobal) { return false; }; + j$.isAsymmetricEqualityTester_ = function(obj) { + return obj && j$.isA_('Function', obj.asymmetricMatch); + }; + j$.getType_ = function(value) { return Object.prototype.toString.apply(value); }; @@ -4329,8 +4333,9 @@ getJasmineRequireObj().DiffBuilder = function (j$) { mismatches.traverse(function (path, isLeaf, formatter) { var actualCustom, expectedCustom, useCustom, - actual = path.dereference(actualRoot), - expected = path.dereference(expectedRoot); + derefResult = dereferencePath(path, actualRoot, expectedRoot, prettyPrinter), + actual = derefResult.actual, + expected = derefResult.expected; if (formatter) { messages.push(formatter(actual, expected, path, prettyPrinter)); @@ -4377,6 +4382,24 @@ getJasmineRequireObj().DiffBuilder = function (j$) { '.'; } }; + + function dereferencePath(objectPath, actual, expected, pp) { + var i, asymmetricResult + + for (i = 0; i < objectPath.components.length; i++) { + actual = actual[objectPath.components[i]]; + expected = expected[objectPath.components[i]]; + + if (j$.isAsymmetricEqualityTester_(expected) && j$.isFunction_(expected.valuesForDiff_)) { + var asymmetricResult = expected.valuesForDiff_(actual, pp); + expected = asymmetricResult.self; + actual = asymmetricResult.other; + } + } + + return {actual: actual, expected: expected}; + } + }; getJasmineRequireObj().MatchersUtil = function(j$) { @@ -4459,13 +4482,9 @@ getJasmineRequireObj().MatchersUtil = function(j$) { return message + '.'; }; - function isAsymmetric(obj) { - return obj && j$.isA_('Function', obj.asymmetricMatch); - } - MatchersUtil.prototype.asymmetricDiff_ = function(a, b, aStack, bStack, customTesters, diffBuilder) { if (j$.isFunction_(b.valuesForDiff_)) { - var values = b.valuesForDiff_(a); + var values = b.valuesForDiff_(a, this.pp); this.eq_(values.other, values.self, aStack, bStack, customTesters, diffBuilder); } else { diffBuilder.recordMismatch(); @@ -4473,8 +4492,8 @@ getJasmineRequireObj().MatchersUtil = function(j$) { }; MatchersUtil.prototype.asymmetricMatch_ = function(a, b, aStack, bStack, customTesters, diffBuilder) { - var asymmetricA = isAsymmetric(a), - asymmetricB = isAsymmetric(b), + var asymmetricA = j$.isAsymmetricEqualityTester_(a), + asymmetricB = j$.isAsymmetricEqualityTester_(b), shim = j$.asymmetricEqualityTesterArgCompatShim(this, customTesters), result; @@ -4485,8 +4504,6 @@ getJasmineRequireObj().MatchersUtil = function(j$) { if (asymmetricA) { result = a.asymmetricMatch(b, shim); if (!result) { - // TODO: Do we want to build an asymmetric diff when the actual was an - // asymmeteric equality tester? Might be confusing. diffBuilder.recordMismatch(); } return result; @@ -4709,7 +4726,7 @@ getJasmineRequireObj().MatchersUtil = function(j$) { // Only use the cmpKey when one of the keys is asymmetric and the corresponding key matches, // otherwise explicitly look up the mapKey in the other Map since we want keys with unique // obj identity (that are otherwise equal) to not match. - if (isAsymmetric(mapKey) || isAsymmetric(cmpKey) && + if (j$.isAsymmetricEqualityTester_(mapKey) || j$.isAsymmetricEqualityTester_(cmpKey) && this.eq_(mapKey, cmpKey, aStack, bStack, customTesters, j$.NullDiffBuilder())) { mapValueB = b.get(cmpKey); } else { @@ -5024,16 +5041,6 @@ getJasmineRequireObj().ObjectPath = function(j$) { } }; - ObjectPath.prototype.dereference = function(obj) { - var i; - - for (i = 0; i < this.components.length; i++) { - obj = obj[this.components[i]]; - } - - return obj; - }; - ObjectPath.prototype.add = function(component) { return new ObjectPath(this.components.concat([component])); }; diff --git a/spec/core/asymmetric_equality/ObjectContainingSpec.js b/spec/core/asymmetric_equality/ObjectContainingSpec.js index bec44fe2..8cad948d 100644 --- a/spec/core/asymmetric_equality/ObjectContainingSpec.js +++ b/spec/core/asymmetric_equality/ObjectContainingSpec.js @@ -125,7 +125,8 @@ describe("ObjectContaining", function() { describe("when other is not an object", function() { it("sets self to jasmineToString()", function () { var containing = new jasmineUnderTest.ObjectContaining({}), - result = containing.valuesForDiff_('a'); + pp = jasmineUnderTest.makePrettyPrinter(), + result = containing.valuesForDiff_('a', pp); expect(result).toEqual({ self: '', diff --git a/spec/core/matchers/ObjectPathSpec.js b/spec/core/matchers/ObjectPathSpec.js index 9750272e..4e0d99fa 100644 --- a/spec/core/matchers/ObjectPathSpec.js +++ b/spec/core/matchers/ObjectPathSpec.js @@ -40,12 +40,4 @@ describe('ObjectPath', function() { expect(path.toString()).toEqual('$.foo'); expect(root.toString()).toEqual(''); }); - - describe('#dereference', function() { - it('returns the value corresponding to the path', function () { - var path = new ObjectPath().add('foo').add(1).add('bar'); - var obj = {foo: ['', {bar: 42}]}; - expect(path.dereference(obj)).toEqual(42); - }); - }); }); diff --git a/spec/core/matchers/toEqualSpec.js b/spec/core/matchers/toEqualSpec.js index 3a1ce985..3a249364 100644 --- a/spec/core/matchers/toEqualSpec.js +++ b/spec/core/matchers/toEqualSpec.js @@ -429,10 +429,10 @@ describe("toEqual", function() { expect(compareEquals(actual, expected).message).toEqual(message); }); - it("reports mismatches involving objectContaining", function() { + it("reports mismatches involving objectContaining and an object", function() { var actual = {x: {a: 1, b: 4, c: 3, extra: 'ignored'}}; var expected = {x: jasmineUnderTest.objectContaining({a: 1, b: 2, c: 3})}; - expect(compareEquals(actual, expected).message).toEqual('Expected $.x.b = 4 to equal 2.') + expect(compareEquals(actual, expected).message).toEqual('Expected $.x.b = 4 to equal 2.'); }); it("reports mismatches between a non-object and objectContaining", function() { diff --git a/src/core/base.js b/src/core/base.js index 3a2a097d..37aa692e 100644 --- a/src/core/base.js +++ b/src/core/base.js @@ -111,6 +111,10 @@ getJasmineRequireObj().base = function(j$, jasmineGlobal) { return false; }; + j$.isAsymmetricEqualityTester_ = function(obj) { + return obj && j$.isA_('Function', obj.asymmetricMatch); + }; + j$.getType_ = function(value) { return Object.prototype.toString.apply(value); }; diff --git a/src/core/matchers/DiffBuilder.js b/src/core/matchers/DiffBuilder.js index 197e4c71..4e43aeaa 100644 --- a/src/core/matchers/DiffBuilder.js +++ b/src/core/matchers/DiffBuilder.js @@ -21,8 +21,9 @@ getJasmineRequireObj().DiffBuilder = function (j$) { mismatches.traverse(function (path, isLeaf, formatter) { var actualCustom, expectedCustom, useCustom, - actual = path.dereference(actualRoot), - expected = path.dereference(expectedRoot); + derefResult = dereferencePath(path, actualRoot, expectedRoot, prettyPrinter), + actual = derefResult.actual, + expected = derefResult.expected; if (formatter) { messages.push(formatter(actual, expected, path, prettyPrinter)); @@ -69,4 +70,22 @@ getJasmineRequireObj().DiffBuilder = function (j$) { '.'; } }; + + function dereferencePath(objectPath, actual, expected, pp) { + var i, asymmetricResult + + for (i = 0; i < objectPath.components.length; i++) { + actual = actual[objectPath.components[i]]; + expected = expected[objectPath.components[i]]; + + if (j$.isAsymmetricEqualityTester_(expected) && j$.isFunction_(expected.valuesForDiff_)) { + var asymmetricResult = expected.valuesForDiff_(actual, pp); + expected = asymmetricResult.self; + actual = asymmetricResult.other; + } + } + + return {actual: actual, expected: expected}; + } + }; diff --git a/src/core/matchers/ObjectPath.js b/src/core/matchers/ObjectPath.js index 266e8e62..a9acfdb2 100644 --- a/src/core/matchers/ObjectPath.js +++ b/src/core/matchers/ObjectPath.js @@ -11,16 +11,6 @@ getJasmineRequireObj().ObjectPath = function(j$) { } }; - ObjectPath.prototype.dereference = function(obj) { - var i; - - for (i = 0; i < this.components.length; i++) { - obj = obj[this.components[i]]; - } - - return obj; - }; - ObjectPath.prototype.add = function(component) { return new ObjectPath(this.components.concat([component])); }; diff --git a/src/core/matchers/matchersUtil.js b/src/core/matchers/matchersUtil.js index e79dd075..6fa812bf 100644 --- a/src/core/matchers/matchersUtil.js +++ b/src/core/matchers/matchersUtil.js @@ -78,13 +78,9 @@ getJasmineRequireObj().MatchersUtil = function(j$) { return message + '.'; }; - function isAsymmetric(obj) { - return obj && j$.isA_('Function', obj.asymmetricMatch); - } - MatchersUtil.prototype.asymmetricDiff_ = function(a, b, aStack, bStack, customTesters, diffBuilder) { if (j$.isFunction_(b.valuesForDiff_)) { - var values = b.valuesForDiff_(a); + var values = b.valuesForDiff_(a, this.pp); this.eq_(values.other, values.self, aStack, bStack, customTesters, diffBuilder); } else { diffBuilder.recordMismatch(); @@ -92,8 +88,8 @@ getJasmineRequireObj().MatchersUtil = function(j$) { }; MatchersUtil.prototype.asymmetricMatch_ = function(a, b, aStack, bStack, customTesters, diffBuilder) { - var asymmetricA = isAsymmetric(a), - asymmetricB = isAsymmetric(b), + var asymmetricA = j$.isAsymmetricEqualityTester_(a), + asymmetricB = j$.isAsymmetricEqualityTester_(b), shim = j$.asymmetricEqualityTesterArgCompatShim(this, customTesters), result; @@ -104,8 +100,6 @@ getJasmineRequireObj().MatchersUtil = function(j$) { if (asymmetricA) { result = a.asymmetricMatch(b, shim); if (!result) { - // TODO: Do we want to build an asymmetric diff when the actual was an - // asymmeteric equality tester? Might be confusing. diffBuilder.recordMismatch(); } return result; @@ -328,7 +322,7 @@ getJasmineRequireObj().MatchersUtil = function(j$) { // Only use the cmpKey when one of the keys is asymmetric and the corresponding key matches, // otherwise explicitly look up the mapKey in the other Map since we want keys with unique // obj identity (that are otherwise equal) to not match. - if (isAsymmetric(mapKey) || isAsymmetric(cmpKey) && + if (j$.isAsymmetricEqualityTester_(mapKey) || j$.isAsymmetricEqualityTester_(cmpKey) && this.eq_(mapKey, cmpKey, aStack, bStack, customTesters, j$.NullDiffBuilder())) { mapValueB = b.get(cmpKey); } else {