From b84a145bb9e126f79bca5f5f987dd455fb62b0c4 Mon Sep 17 00:00:00 2001 From: Alex Bepple Date: Wed, 10 Dec 2014 23:50:32 +0100 Subject: [PATCH 1/7] IsObjectWithProperties: provide naive diffs --- lib/matchers/IsObjectWithProperties.js | 6 ++++++ test/matchers/IsObjectWithPropertiesSpec.js | 10 ++++++++++ 2 files changed, 16 insertions(+) diff --git a/lib/matchers/IsObjectWithProperties.js b/lib/matchers/IsObjectWithProperties.js index 7431bb0..15735f6 100644 --- a/lib/matchers/IsObjectWithProperties.js +++ b/lib/matchers/IsObjectWithProperties.js @@ -56,6 +56,12 @@ function IsObjectWithProperties(properties) { .append(' '); return propertyMatchers[key].describeMismatch(actual[key], description); }); + }, + getExpectedForDiff: function () { + return properties; + }, + formatActualForDiff: function (actual) { + return actual; } }); } diff --git a/test/matchers/IsObjectWithPropertiesSpec.js b/test/matchers/IsObjectWithPropertiesSpec.js index 22504ec..df54d0d 100644 --- a/test/matchers/IsObjectWithPropertiesSpec.js +++ b/test/matchers/IsObjectWithPropertiesSpec.js @@ -80,6 +80,16 @@ describe('IsObjectWithProperties', function () { }); }); + it('should format actual for diff', function () { + var simple = IsObjectWithProperties.hasProperties(); + __.assertThat(simple.formatActualForDiff({a: 1}), __.equalTo({a: 1})); + }); + + it('should provide expected for diff', function () { + var simple = IsObjectWithProperties.hasProperties({a: 1}); + __.assertThat(simple.getExpectedForDiff(), __.equalTo({a: 1})); + }); + describe('with a promising matcher', function () { beforeEach(function () { sut = hasProperties({ From ac702b2d01d5d2fc4c2d800c8d953e20820df2b5 Mon Sep 17 00:00:00 2001 From: Alex Bepple Date: Thu, 11 Dec 2014 15:11:22 +0100 Subject: [PATCH 2/7] omit irrelevant properties in diff Properties that match do not contribute to understanding the test failure. --- lib/assertThat.js | 4 ++++ lib/matchers/IsObjectWithProperties.js | 15 ++++++++----- test/assertThatSpec.js | 24 ++++++++++++++++++++- test/matchers/IsObjectWithPropertiesSpec.js | 23 ++++++++++++++------ 4 files changed, 53 insertions(+), 13 deletions(-) diff --git a/lib/assertThat.js b/lib/assertThat.js index 058a10e..2ab2c1c 100644 --- a/lib/assertThat.js +++ b/lib/assertThat.js @@ -36,6 +36,10 @@ function assertThat(reason, actual, matcher) { actual: matcher.formatActualForDiff(actual) }; } + if (_.isFunction(matcher.getDiffItems)) { + errorProperties = matcher.getDiffItems(actual); + errorProperties.showDiff = true; + } throw new AssertionError(description.get(), errorProperties, assertThat); } diff --git a/lib/matchers/IsObjectWithProperties.js b/lib/matchers/IsObjectWithProperties.js index 15735f6..bf94e8d 100644 --- a/lib/matchers/IsObjectWithProperties.js +++ b/lib/matchers/IsObjectWithProperties.js @@ -57,11 +57,16 @@ function IsObjectWithProperties(properties) { return propertyMatchers[key].describeMismatch(actual[key], description); }); }, - getExpectedForDiff: function () { - return properties; - }, - formatActualForDiff: function (actual) { - return actual; + + getDiffItems: function (actual) { + var expectedForDiff = _.pick(propertyMatchers, function (matcher, key) { + return !matcher.matches(actual[key]); + }); + var actualForDiff = _.pick(actual, _.keys(expectedForDiff)); + return { + expected: expectedForDiff, + actual: actualForDiff + }; } }); } diff --git a/test/assertThatSpec.js b/test/assertThatSpec.js index 492dd18..ac2fe62 100644 --- a/test/assertThatSpec.js +++ b/test/assertThatSpec.js @@ -59,7 +59,7 @@ describe('assertThat', function () { assertEquals(thrown.message , 'Assertion message\nExpected: Matcher description\n but: was "real value"'); }); - it('should pass diff representations to AssertionError', function () { + it('should pass old-style diff representations to AssertionError', function () { var thrown; var testMatcher = new TestMatcher(function () { return false; }); @@ -81,6 +81,28 @@ describe('assertThat', function () { assertEquals(thrown.actual, 'actual for diff: actual value'); }); + it('should pass new-style diff representations to AssertionError', function () { + var thrown; + + var testMatcher = new TestMatcher(function () { return false; }); + testMatcher.getDiffItems = function (actual) { + return { + expected: 'expected for diff', + actual: 'actual for diff: ' + actual + }; + }; + + try { + assertThat('actual value', testMatcher); + } + catch (e) { + thrown = e; + } + + assertEquals(thrown.expected, 'expected for diff'); + assertEquals(thrown.actual, 'actual for diff: actual value'); + }); + it('should throw if matcher returns a promise', function () { var thrown; diff --git a/test/matchers/IsObjectWithPropertiesSpec.js b/test/matchers/IsObjectWithPropertiesSpec.js index df54d0d..e70570e 100644 --- a/test/matchers/IsObjectWithPropertiesSpec.js +++ b/test/matchers/IsObjectWithPropertiesSpec.js @@ -80,14 +80,23 @@ describe('IsObjectWithProperties', function () { }); }); - it('should format actual for diff', function () { - var simple = IsObjectWithProperties.hasProperties(); - __.assertThat(simple.formatActualForDiff({a: 1}), __.equalTo({a: 1})); - }); + describe('diff', function() { + it('should exist', function () { + __.assertThat(sut.getDiffItems({}), __.hasProperties({ + expected: __.defined(), + actual: __.defined() + })); + }); + + it('should contain mismatched properties', function () { + var diffObjects = sut.getDiffItems(new Person('Jim', 2)); + __.assertThat(diffObjects, __.everyItem(__.hasProperty('name'))); + }); - it('should provide expected for diff', function () { - var simple = IsObjectWithProperties.hasProperties({a: 1}); - __.assertThat(simple.getExpectedForDiff(), __.equalTo({a: 1})); + it('should omit matched properties', function () { + var diffObjects = sut.getDiffItems({name: 'Joe', children: 0}); + __.assertThat(diffObjects, __.everyItem(__.not(__.hasProperty('name')))); + }); }); describe('with a promising matcher', function () { From 34d5751c5a5270493a971c6ae1046833690f527b Mon Sep 17 00:00:00 2001 From: Alex Bepple Date: Thu, 11 Dec 2014 15:17:04 +0100 Subject: [PATCH 3/7] omit extra properties on the actual in diff --- test/matchers/IsObjectWithPropertiesSpec.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/matchers/IsObjectWithPropertiesSpec.js b/test/matchers/IsObjectWithPropertiesSpec.js index e70570e..4161039 100644 --- a/test/matchers/IsObjectWithPropertiesSpec.js +++ b/test/matchers/IsObjectWithPropertiesSpec.js @@ -97,6 +97,11 @@ describe('IsObjectWithProperties', function () { var diffObjects = sut.getDiffItems({name: 'Joe', children: 0}); __.assertThat(diffObjects, __.everyItem(__.not(__.hasProperty('name')))); }); + + it('should omit extra properties', function () { + var diffObjects = sut.getDiffItems({foo: 'bar'}); + __.assertThat(diffObjects, __.everyItem(__.not(__.hasProperty('foo')))); + }); }); describe('with a promising matcher', function () { From edd9204b5ab4e5ce3ac67b65fe9fe02998c044cb Mon Sep 17 00:00:00 2001 From: Alex Bepple Date: Thu, 11 Dec 2014 18:27:10 +0100 Subject: [PATCH 4/7] when matching properties with matchers, refer to them --- lib/matchers/IsObjectWithProperties.js | 23 ++++++++++ test/matchers/IsObjectWithPropertiesSpec.js | 49 +++++++++++++++++++++ 2 files changed, 72 insertions(+) diff --git a/lib/matchers/IsObjectWithProperties.js b/lib/matchers/IsObjectWithProperties.js index bf94e8d..6326dbf 100644 --- a/lib/matchers/IsObjectWithProperties.js +++ b/lib/matchers/IsObjectWithProperties.js @@ -5,6 +5,7 @@ var _ = require('lodash') , asMatcher = require('./IsEqual').asMatcher , defined = require('./IsDefined').defined ; +var Description = require('../Description'); var promiseAgnostic = require('./promiseAgnostic'); function IsObjectWithProperties(properties) { @@ -62,7 +63,29 @@ function IsObjectWithProperties(properties) { var expectedForDiff = _.pick(propertyMatchers, function (matcher, key) { return !matcher.matches(actual[key]); }); + expectedForDiff = _.mapValues(expectedForDiff, function (matcher, key) { + if (_.isFunction(matcher.getDiffItems)) { + return matcher.getDiffItems(actual[key]).expected; + } + if (_.isFunction(matcher.getExpectedForDiff)) { + return matcher.getExpectedForDiff(); + } + var description = new Description(); + matcher.describeTo(description); + return description.get(); + }); + var actualForDiff = _.pick(actual, _.keys(expectedForDiff)); + actualForDiff = _.mapValues(actualForDiff, function (value, key) { + var matcher = propertyMatchers[key]; + if (_.isFunction(matcher.getDiffItems)) { + return matcher.getDiffItems(actual[key]).actual; + } + if (_.isFunction(matcher.formatActualForDiff)) { + return matcher.formatActualForDiff(actual[key]); + } + return actual[key]; + }); return { expected: expectedForDiff, actual: actualForDiff diff --git a/test/matchers/IsObjectWithPropertiesSpec.js b/test/matchers/IsObjectWithPropertiesSpec.js index 4161039..d3991de 100644 --- a/test/matchers/IsObjectWithPropertiesSpec.js +++ b/test/matchers/IsObjectWithPropertiesSpec.js @@ -5,6 +5,7 @@ var IsObjectWithProperties = require('../../lib/matchers/IsObjectWithProperties' , __ = require('../../lib/hamjest') ; var deferMatcher = require('../deferMatcher'); +var TestMatcher = require('../TestMatcher'); describe('IsObjectWithProperties', function () { @@ -102,6 +103,54 @@ describe('IsObjectWithProperties', function () { var diffObjects = sut.getDiffItems({foo: 'bar'}); __.assertThat(diffObjects, __.everyItem(__.not(__.hasProperty('foo')))); }); + + it('should use diff from new-style diff-capable nested matcher', function () { + var testMatcher = new TestMatcher(function () { return false; }); + testMatcher.getDiffItems = function (actual) { + return { + expected: 'expected from nested', + actual: 'actual from nested: ' + actual + }; + }; + var sutWithNestedMatcher = hasProperties({a: testMatcher}); + var diffObjects = sutWithNestedMatcher.getDiffItems({a: 1}); + __.assertThat(diffObjects, __.hasProperties({ + expected: {a: 'expected from nested'}, + actual: {a: 'actual from nested: 1'} + })); + }); + + it('should use diff from old-style diff-capable nested matcher', function () { + var testMatcher = new TestMatcher(function () { return false; }); + testMatcher.getExpectedForDiff = function () { + return 'expected from nested'; + }; + testMatcher.formatActualForDiff = function (actual) { + return 'actual from nested: ' + actual; + }; + + var sutWithNestedMatcher = hasProperties({a: testMatcher}); + var diffObjects = sutWithNestedMatcher.getDiffItems({a: 1}); + __.assertThat(diffObjects, __.hasProperties({ + expected: {a: 'expected from nested'}, + actual: {a: 'actual from nested: 1'} + })); + }); + + it('should use actual and mismatch description from non-diff-capable nested matcher', function () { + var testMatcher = new TestMatcher(function () { return false; }); + + var description = new Description(); + testMatcher.describeTo(description); + + var sutWithNestedMatcher = hasProperties({a: testMatcher}); + var diffObjects = sutWithNestedMatcher.getDiffItems({a: 1}); + + __.assertThat(diffObjects, __.hasProperties({ + expected: {a: description.get()}, + actual: {a: 1} + })); + }); }); describe('with a promising matcher', function () { From 783de07c10494dcc570f5e34547850cfd8975ad6 Mon Sep 17 00:00:00 2001 From: Alex Bepple Date: Thu, 11 Dec 2014 19:34:56 +0100 Subject: [PATCH 5/7] increase cohesion --- lib/matchers/IsObjectWithProperties.js | 39 +++++++++++++------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/lib/matchers/IsObjectWithProperties.js b/lib/matchers/IsObjectWithProperties.js index 6326dbf..4715b33 100644 --- a/lib/matchers/IsObjectWithProperties.js +++ b/lib/matchers/IsObjectWithProperties.js @@ -60,35 +60,36 @@ function IsObjectWithProperties(properties) { }, getDiffItems: function (actual) { - var expectedForDiff = _.pick(propertyMatchers, function (matcher, key) { + var relevantMatchers = _.pick(propertyMatchers, function (matcher, key) { return !matcher.matches(actual[key]); }); - expectedForDiff = _.mapValues(expectedForDiff, function (matcher, key) { - if (_.isFunction(matcher.getDiffItems)) { - return matcher.getDiffItems(actual[key]).expected; - } - if (_.isFunction(matcher.getExpectedForDiff)) { - return matcher.getExpectedForDiff(); - } + + var guessDiffItems = function (matcher, actual) { var description = new Description(); matcher.describeTo(description); - return description.get(); - }); + return { + expected: description.get(), + actual: actual + }; + }; - var actualForDiff = _.pick(actual, _.keys(expectedForDiff)); - actualForDiff = _.mapValues(actualForDiff, function (value, key) { - var matcher = propertyMatchers[key]; + var diffItemsIndividually = _.mapValues(relevantMatchers, function (matcher, key) { if (_.isFunction(matcher.getDiffItems)) { - return matcher.getDiffItems(actual[key]).actual; + return matcher.getDiffItems(actual[key]); } - if (_.isFunction(matcher.formatActualForDiff)) { - return matcher.formatActualForDiff(actual[key]); + if (_.isFunction(matcher.getExpectedForDiff) && + _.isFunction(matcher.formatActualForDiff)) { + return { + expected: matcher.getExpectedForDiff(), + actual: matcher.formatActualForDiff(actual[key]) + }; } - return actual[key]; + return guessDiffItems(matcher, actual[key]); }); + return { - expected: expectedForDiff, - actual: actualForDiff + expected: _.mapValues(diffItemsIndividually, 'expected'), + actual: _.mapValues(diffItemsIndividually, 'actual') }; } }); From 678be02321d5f93792cbcafd3dfebf6d5719af0d Mon Sep 17 00:00:00 2001 From: Alex Bepple Date: Thu, 11 Dec 2014 19:57:30 +0100 Subject: [PATCH 6/7] migrate SubstringMatcher to new diff API --- lib/matchers/SubstringMatcher.js | 8 ++++++-- test/matchers/SubstringMatcherSpec.js | 12 +++++------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/lib/matchers/SubstringMatcher.js b/lib/matchers/SubstringMatcher.js index 446477f..e2f3b0f 100644 --- a/lib/matchers/SubstringMatcher.js +++ b/lib/matchers/SubstringMatcher.js @@ -26,8 +26,12 @@ function SubstringMatcher(substring, relation, matchesString) { .append('was ') .appendValue(actual); }, - getExpectedForDiff: function () { return substring; }, - formatActualForDiff: function (actual) { return actual; } + getDiffItems: function (actual) { + return { + expected: substring, + actual: actual + }; + } }); } diff --git a/test/matchers/SubstringMatcherSpec.js b/test/matchers/SubstringMatcherSpec.js index fe3a9d7..28ed8b1 100644 --- a/test/matchers/SubstringMatcherSpec.js +++ b/test/matchers/SubstringMatcherSpec.js @@ -6,7 +6,6 @@ var AssertionError = require('assertion-error') , __ = require('../../lib/hamjest') , assertTrue = require('../asserts').assertTrue , assertFalse = require('../asserts').assertFalse - , assertEquals = require('../asserts').assertEquals ; describe('SubstringMatcher', function () { @@ -41,12 +40,11 @@ describe('SubstringMatcher', function () { assertFalse(sut.matches(5)); }); - it('should provide expected for diff', function () { - assertEquals('a value', sut.getExpectedForDiff()); - }); - - it('should format actual for diff', function() { - assertEquals('foo', sut.formatActualForDiff('foo')); + it('should provide diff items', function () { + __.assertThat(sut.getDiffItems('foo'), __.equalTo({ + expected: 'a value', + actual: 'foo' + })); }); describe('description', function () { From 1f1aa50a002dcd93f2c97d232b27f249ac3ee7b9 Mon Sep 17 00:00:00 2001 From: Alex Bepple Date: Thu, 11 Dec 2014 20:07:46 +0100 Subject: [PATCH 7/7] use new diff API everywhere & remove old diff API --- lib/assertThat.js | 8 ------- lib/matchers/Is.js | 3 +-- lib/matchers/IsEqual.js | 8 +++++-- lib/matchers/IsObjectWithProperties.js | 7 ------ lib/promiseThat.js | 8 +++---- test/assertThatSpec.js | 24 +-------------------- test/matchers/IsObjectWithPropertiesSpec.js | 19 +--------------- test/promiseThatSpec.js | 10 ++++----- 8 files changed, 18 insertions(+), 69 deletions(-) diff --git a/lib/assertThat.js b/lib/assertThat.js index 2ab2c1c..9d00985 100644 --- a/lib/assertThat.js +++ b/lib/assertThat.js @@ -28,14 +28,6 @@ function assertThat(reason, actual, matcher) { matcher.describeMismatch(actual, description); var errorProperties = {}; - if (_.isFunction(matcher.getExpectedForDiff) && - _.isFunction(matcher.formatActualForDiff)) { - errorProperties = { - showDiff: true, - expected: matcher.getExpectedForDiff(), - actual: matcher.formatActualForDiff(actual) - }; - } if (_.isFunction(matcher.getDiffItems)) { errorProperties = matcher.getDiffItems(actual); errorProperties.showDiff = true; diff --git a/lib/matchers/Is.js b/lib/matchers/Is.js index 4abd5ae..46b57e1 100644 --- a/lib/matchers/Is.js +++ b/lib/matchers/Is.js @@ -18,8 +18,7 @@ var Is = acceptingMatcher(function Is(innerMatcher) { describeMismatch: function (value, description) { return innerMatcher.describeMismatch(value, description); }, - getExpectedForDiff: innerMatcher.getExpectedForDiff, - formatActualForDiff: innerMatcher.formatActualForDiff + getDiffItems: innerMatcher.getDiffItems }); }); diff --git a/lib/matchers/IsEqual.js b/lib/matchers/IsEqual.js index cf08416..4f9ce29 100644 --- a/lib/matchers/IsEqual.js +++ b/lib/matchers/IsEqual.js @@ -12,8 +12,12 @@ function IsEqual(expectedValue) { describeTo: function (description) { description.appendValue(expectedValue); }, - getExpectedForDiff: function () { return expectedValue; }, - formatActualForDiff: function (actual) { return actual; } + getDiffItems: function (actual) { + return { + expected: expectedValue, + actual: actual + }; + } }); } diff --git a/lib/matchers/IsObjectWithProperties.js b/lib/matchers/IsObjectWithProperties.js index 4715b33..765f0cc 100644 --- a/lib/matchers/IsObjectWithProperties.js +++ b/lib/matchers/IsObjectWithProperties.js @@ -77,13 +77,6 @@ function IsObjectWithProperties(properties) { if (_.isFunction(matcher.getDiffItems)) { return matcher.getDiffItems(actual[key]); } - if (_.isFunction(matcher.getExpectedForDiff) && - _.isFunction(matcher.formatActualForDiff)) { - return { - expected: matcher.getExpectedForDiff(), - actual: matcher.formatActualForDiff(actual[key]) - }; - } return guessDiffItems(matcher, actual[key]); }); diff --git a/lib/promiseThat.js b/lib/promiseThat.js index a65a279..d2f20aa 100644 --- a/lib/promiseThat.js +++ b/lib/promiseThat.js @@ -20,14 +20,14 @@ function promiseThat(reason, actual, matcher) { .appendDescriptionOf(matcher) .append('\n but: '); return q(matcher.describeMismatch(actual, description)).then(function () { - if (!_.isFunction(matcher.getExpectedForDiff) || - !_.isFunction(matcher.formatActualForDiff)) { + if (!_.isFunction(matcher.getDiffItems)) { return {}; } + var diffItems = matcher.getDiffItems(actual); return q.all([ - matcher.getExpectedForDiff(), - matcher.formatActualForDiff(actual) + diffItems.expected, + diffItems.actual ]).spread(function (expected, actual) { return { showDiff: true, diff --git a/test/assertThatSpec.js b/test/assertThatSpec.js index ac2fe62..fc57851 100644 --- a/test/assertThatSpec.js +++ b/test/assertThatSpec.js @@ -59,29 +59,7 @@ describe('assertThat', function () { assertEquals(thrown.message , 'Assertion message\nExpected: Matcher description\n but: was "real value"'); }); - it('should pass old-style diff representations to AssertionError', function () { - var thrown; - - var testMatcher = new TestMatcher(function () { return false; }); - testMatcher.getExpectedForDiff = function () { - return 'expected for diff'; - }; - testMatcher.formatActualForDiff = function (actual) { - return 'actual for diff: ' + actual; - }; - - try { - assertThat('actual value', testMatcher); - } - catch (e) { - thrown = e; - } - - assertEquals(thrown.expected, 'expected for diff'); - assertEquals(thrown.actual, 'actual for diff: actual value'); - }); - - it('should pass new-style diff representations to AssertionError', function () { + it('should pass diff representations to AssertionError', function () { var thrown; var testMatcher = new TestMatcher(function () { return false; }); diff --git a/test/matchers/IsObjectWithPropertiesSpec.js b/test/matchers/IsObjectWithPropertiesSpec.js index d3991de..46f970b 100644 --- a/test/matchers/IsObjectWithPropertiesSpec.js +++ b/test/matchers/IsObjectWithPropertiesSpec.js @@ -104,7 +104,7 @@ describe('IsObjectWithProperties', function () { __.assertThat(diffObjects, __.everyItem(__.not(__.hasProperty('foo')))); }); - it('should use diff from new-style diff-capable nested matcher', function () { + it('should use diff from diff-capable nested matcher', function () { var testMatcher = new TestMatcher(function () { return false; }); testMatcher.getDiffItems = function (actual) { return { @@ -120,23 +120,6 @@ describe('IsObjectWithProperties', function () { })); }); - it('should use diff from old-style diff-capable nested matcher', function () { - var testMatcher = new TestMatcher(function () { return false; }); - testMatcher.getExpectedForDiff = function () { - return 'expected from nested'; - }; - testMatcher.formatActualForDiff = function (actual) { - return 'actual from nested: ' + actual; - }; - - var sutWithNestedMatcher = hasProperties({a: testMatcher}); - var diffObjects = sutWithNestedMatcher.getDiffItems({a: 1}); - __.assertThat(diffObjects, __.hasProperties({ - expected: {a: 'expected from nested'}, - actual: {a: 'actual from nested: 1'} - })); - }); - it('should use actual and mismatch description from non-diff-capable nested matcher', function () { var testMatcher = new TestMatcher(function () { return false; }); diff --git a/test/promiseThatSpec.js b/test/promiseThatSpec.js index 85b2ad5..b62f924 100644 --- a/test/promiseThatSpec.js +++ b/test/promiseThatSpec.js @@ -160,11 +160,11 @@ describe('promiseThat', function () { it('should pass diff representations to AssertionError', function (done) { var testMatcher = new TestMatcher(function () { return false; }); - testMatcher.getExpectedForDiff = function () { - return 'expected for diff'; - }; - testMatcher.formatActualForDiff = function (actual) { - return q('actual for diff: ' + actual); + testMatcher.getDiffItems = function (actual) { + return { + expected: 'expected for diff', + actual: q('actual for diff: ' + actual) + }; }; promiseThat('actual value', testMatcher).done(