From e167009904bf24d56caf7910fc5dbb79f42265d9 Mon Sep 17 00:00:00 2001 From: solodynamo Date: Sun, 10 Sep 2017 01:25:23 +0530 Subject: [PATCH 01/10] property assertion should only accept strings if nested, fixes #1043 --- lib/chai/core/assertions.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/chai/core/assertions.js b/lib/chai/core/assertions.js index 069c5241b..2a819ed20 100644 --- a/lib/chai/core/assertions.js +++ b/lib/chai/core/assertions.js @@ -1765,6 +1765,12 @@ module.exports = function (chai, _) { , obj = flag(this, 'object') , ssfi = flag(this, 'ssfi'); + if (typeof name !== 'string' && isNested) { + var msgPrefix = flag(this, 'message') + msgPrefix = msgPrefix ? msgPrefix + ': ' : '' + throw new AssertionError(msgPrefix + 'the argument to `property` must be a string') + } + if (isNested && isOwn) { flagMsg = flagMsg ? flagMsg + ': ' : ''; throw new AssertionError( From 3f2ce293b5633def58ae44857e20159496a16a39 Mon Sep 17 00:00:00 2001 From: solodynamo Date: Sun, 10 Sep 2017 16:35:21 +0530 Subject: [PATCH 02/10] similar logic seperated out --- lib/chai/core/assertions.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/chai/core/assertions.js b/lib/chai/core/assertions.js index 2a819ed20..a2c41ab27 100644 --- a/lib/chai/core/assertions.js +++ b/lib/chai/core/assertions.js @@ -1765,14 +1765,13 @@ module.exports = function (chai, _) { , obj = flag(this, 'object') , ssfi = flag(this, 'ssfi'); + flagMsg = flagMsg ? flagMsg + ': ' : ''; + if (typeof name !== 'string' && isNested) { - var msgPrefix = flag(this, 'message') - msgPrefix = msgPrefix ? msgPrefix + ': ' : '' - throw new AssertionError(msgPrefix + 'the argument to `property` must be a string') + throw new AssertionError(flagMsg + 'the argument to `property` must be a string') } if (isNested && isOwn) { - flagMsg = flagMsg ? flagMsg + ': ' : ''; throw new AssertionError( flagMsg + 'The "nested" and "own" flags cannot be combined.', undefined, @@ -1781,7 +1780,6 @@ module.exports = function (chai, _) { } if (obj === null || obj === undefined) { - flagMsg = flagMsg ? flagMsg + ': ' : ''; throw new AssertionError( flagMsg + 'Target cannot be null or undefined.', undefined, From 335fd33809994b9f96e3f33ba91b992bd6a9cfb1 Mon Sep 17 00:00:00 2001 From: solodynamo Date: Sun, 10 Sep 2017 16:40:28 +0530 Subject: [PATCH 03/10] test cases for fix #1043 --- test/expect.js | 8 ++++++++ test/should.js | 2 ++ 2 files changed, 10 insertions(+) diff --git a/test/expect.js b/test/expect.js index 0b5a0e5b2..392bd3ed7 100644 --- a/test/expect.js +++ b/test/expect.js @@ -1462,6 +1462,14 @@ describe('expect', function () { err(function () { expect(undefined, 'blah').to.have.property("a"); }, "blah: Target cannot be null or undefined."); + + expect(function () { + expect({a:1}).to.have.nested.property('{a:1}'); + }).to.not.throw('the argument to `property` must be a string'); + + expect(function () { + expect({a:1}).to.have.nested.property({'a':'1'}); + }).to.throw('the argument to `property` must be a string'); }); it('property(name, val)', function(){ diff --git a/test/should.js b/test/should.js index 1453512d6..c7274d184 100644 --- a/test/should.js +++ b/test/should.js @@ -1407,6 +1407,8 @@ describe('should', function() { ({ 'foo.bar[]': 'baz'}).should.have.nested.property('foo\\.bar\\[\\]'); + ({a:1}).should.have.nested.property('a'); + err(function(){ ({ 'foo.bar': 'baz' }).should.have.nested.property('foo.bar'); }, "expected { 'foo.bar': 'baz' } to have nested property 'foo.bar'"); From 5fd4fa29eeb97b00846eddac46ca7440284c055e Mon Sep 17 00:00:00 2001 From: solodynamo Date: Sun, 10 Sep 2017 21:25:46 +0530 Subject: [PATCH 04/10] type check if not isNested with tests --- lib/chai/core/assertions.js | 13 ++++++++++--- test/assert.js | 5 +++++ test/expect.js | 4 ++++ 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/lib/chai/core/assertions.js b/lib/chai/core/assertions.js index a2c41ab27..d7940765f 100644 --- a/lib/chai/core/assertions.js +++ b/lib/chai/core/assertions.js @@ -1763,12 +1763,19 @@ module.exports = function (chai, _) { , isOwn = flag(this, 'own') , flagMsg = flag(this, 'message') , obj = flag(this, 'object') - , ssfi = flag(this, 'ssfi'); + , ssfi = flag(this, 'ssfi') + , nameType = typeof name; flagMsg = flagMsg ? flagMsg + ': ' : ''; - if (typeof name !== 'string' && isNested) { - throw new AssertionError(flagMsg + 'the argument to `property` must be a string') + if (isNested) { + if (nameType !== 'string') { + throw new AssertionError(flagMsg + 'the argument to `property` must be a string'); + } + } else { + if (nameType !== 'string' && nameType !== 'number' && nameType !== 'symbol') { + throw new AssertionError(flagMsg + 'the argument to `property` must be either of type string, number or symbol'); + } } if (isNested && isOwn) { diff --git a/test/assert.js b/test/assert.js index e3483ee97..85e966610 100644 --- a/test/assert.js +++ b/test/assert.js @@ -1395,6 +1395,7 @@ describe('assert', function () { var obj = { foo: { bar: 'baz' } }; var simpleObj = { foo: 'bar' }; var undefinedKeyObj = { foo: undefined }; + var dummyObj = { a: '1' }; assert.property(obj, 'foo'); assert.property(obj, 'toString'); assert.propertyVal(obj, 'toString', Object.prototype.toString); @@ -1454,6 +1455,10 @@ describe('assert', function () { err(function () { assert.property(undefined, 'a', 'blah'); }, "blah: Target cannot be null or undefined."); + + err(function () { + assert.propertyVal(dummyObj, 'a', '2', 'blah'); + }, "blah: expected { a: '1' } to have property 'a' of '2', but got '1'"); }); it('deepPropertyVal', function () { diff --git a/test/expect.js b/test/expect.js index 392bd3ed7..44f5f4aff 100644 --- a/test/expect.js +++ b/test/expect.js @@ -1470,6 +1470,10 @@ describe('expect', function () { expect(function () { expect({a:1}).to.have.nested.property({'a':'1'}); }).to.throw('the argument to `property` must be a string'); + + expect(function () { + expect({a:1}).to.have.property(null); + }).to.throw('the argument to `property` must be either of type string, number or symbol'); }); it('property(name, val)', function(){ From 8ff7cb754a5cb6de5a29df67c76ab7acd5895a41 Mon Sep 17 00:00:00 2001 From: solodynamo Date: Thu, 14 Sep 2017 11:15:50 +0530 Subject: [PATCH 05/10] Tests for assert and should inteface --- test/assert.js | 9 +++++++++ test/expect.js | 12 ++++-------- test/should.js | 9 +++++++++ 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/test/assert.js b/test/assert.js index 85e966610..5b2259082 100644 --- a/test/assert.js +++ b/test/assert.js @@ -1,5 +1,6 @@ describe('assert', function () { var assert = chai.assert; + var expect = chai.expect; it('assert', function () { var foo = 'bar'; @@ -1459,6 +1460,14 @@ describe('assert', function () { err(function () { assert.propertyVal(dummyObj, 'a', '2', 'blah'); }, "blah: expected { a: '1' } to have property 'a' of '2', but got '1'"); + + expect(function () { + assert.nestedProperty({a:1}, '{a:1}'); + }).to.not.throw('the argument to `property` must be a string'); + + expect(function () { + assert.nestedProperty({a:1}, {'a':'1'}); + }).to.throw('the argument to `property` must be a string'); }); it('deepPropertyVal', function () { diff --git a/test/expect.js b/test/expect.js index 44f5f4aff..410a85f25 100644 --- a/test/expect.js +++ b/test/expect.js @@ -1463,14 +1463,6 @@ describe('expect', function () { expect(undefined, 'blah').to.have.property("a"); }, "blah: Target cannot be null or undefined."); - expect(function () { - expect({a:1}).to.have.nested.property('{a:1}'); - }).to.not.throw('the argument to `property` must be a string'); - - expect(function () { - expect({a:1}).to.have.nested.property({'a':'1'}); - }).to.throw('the argument to `property` must be a string'); - expect(function () { expect({a:1}).to.have.property(null); }).to.throw('the argument to `property` must be either of type string, number or symbol'); @@ -1767,6 +1759,10 @@ describe('expect', function () { expect({ 'foo.bar': 'baz' }) .to.have.nested.property('foo.bar'); }, "expected { 'foo.bar': 'baz' } to have nested property 'foo.bar'"); + + expect(function () { + expect({a:1}).to.have.nested.property({'a':'1'}); + }).to.throw('the argument to `property` must be a string'); }); it('nested.property(name, val)', function(){ diff --git a/test/should.js b/test/should.js index c7274d184..a121ec734 100644 --- a/test/should.js +++ b/test/should.js @@ -1,5 +1,6 @@ describe('should', function() { var should = chai.Should(); + var expect = chai.expect; it('assertion', function(){ 'test'.should.be.a('string'); @@ -1409,6 +1410,14 @@ describe('should', function() { ({a:1}).should.have.nested.property('a'); + expect(function () { + ({a:1}).should.have.nested.property('{a:1}'); + }).to.not.throw('the argument to `property` must be a string'); + + expect(function () { + ({a:1}).should.have.nested.property({'a':'1'}); + }).to.throw('the argument to `property` must be a string'); + err(function(){ ({ 'foo.bar': 'baz' }).should.have.nested.property('foo.bar'); }, "expected { 'foo.bar': 'baz' } to have nested property 'foo.bar'"); From a078a6db733a5974f8115e5d7add8b59383ff9d1 Mon Sep 17 00:00:00 2001 From: solodynamo Date: Sat, 16 Sep 2017 20:06:48 +0530 Subject: [PATCH 06/10] Error message change --- lib/chai/core/assertions.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/chai/core/assertions.js b/lib/chai/core/assertions.js index d7940765f..2d6676b10 100644 --- a/lib/chai/core/assertions.js +++ b/lib/chai/core/assertions.js @@ -1770,11 +1770,11 @@ module.exports = function (chai, _) { if (isNested) { if (nameType !== 'string') { - throw new AssertionError(flagMsg + 'the argument to `property` must be a string'); + throw new AssertionError(flagMsg + 'the argument to property must be a string when using nested syntax'); } } else { if (nameType !== 'string' && nameType !== 'number' && nameType !== 'symbol') { - throw new AssertionError(flagMsg + 'the argument to `property` must be either of type string, number or symbol'); + throw new AssertionError(flagMsg + ' the argument to property must be a string, number, or symbol'); } } From e1442d0a7bdc0ead98bddb244eed37ed06085d31 Mon Sep 17 00:00:00 2001 From: solodynamo Date: Sat, 16 Sep 2017 20:12:20 +0530 Subject: [PATCH 07/10] changes in should and assert tests as per review --- test/assert.js | 9 ++++----- test/expect.js | 4 ++-- test/should.js | 9 ++++----- 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/test/assert.js b/test/assert.js index 5b2259082..519334654 100644 --- a/test/assert.js +++ b/test/assert.js @@ -1,6 +1,5 @@ describe('assert', function () { var assert = chai.assert; - var expect = chai.expect; it('assert', function () { var foo = 'bar'; @@ -1461,13 +1460,13 @@ describe('assert', function () { assert.propertyVal(dummyObj, 'a', '2', 'blah'); }, "blah: expected { a: '1' } to have property 'a' of '2', but got '1'"); - expect(function () { + chai.expect(function () { assert.nestedProperty({a:1}, '{a:1}'); - }).to.not.throw('the argument to `property` must be a string'); + }).to.not.throw('the argument to property must be a string when using nested syntax'); - expect(function () { + chai.expect(function () { assert.nestedProperty({a:1}, {'a':'1'}); - }).to.throw('the argument to `property` must be a string'); + }).to.throw('the argument to property must be a string when using nested syntax'); }); it('deepPropertyVal', function () { diff --git a/test/expect.js b/test/expect.js index 410a85f25..a02c73d34 100644 --- a/test/expect.js +++ b/test/expect.js @@ -1465,7 +1465,7 @@ describe('expect', function () { expect(function () { expect({a:1}).to.have.property(null); - }).to.throw('the argument to `property` must be either of type string, number or symbol'); + }).to.throw('the argument to property must be a string, number, or symbol'); }); it('property(name, val)', function(){ @@ -1762,7 +1762,7 @@ describe('expect', function () { expect(function () { expect({a:1}).to.have.nested.property({'a':'1'}); - }).to.throw('the argument to `property` must be a string'); + }).to.throw('the argument to property must be a string when using nested syntax'); }); it('nested.property(name, val)', function(){ diff --git a/test/should.js b/test/should.js index a121ec734..2a499f7af 100644 --- a/test/should.js +++ b/test/should.js @@ -1,6 +1,5 @@ describe('should', function() { var should = chai.Should(); - var expect = chai.expect; it('assertion', function(){ 'test'.should.be.a('string'); @@ -1410,13 +1409,13 @@ describe('should', function() { ({a:1}).should.have.nested.property('a'); - expect(function () { + (function () { ({a:1}).should.have.nested.property('{a:1}'); - }).to.not.throw('the argument to `property` must be a string'); + }).should.not.throw('the argument to property must be a string when using nested syntax'); - expect(function () { + (function () { ({a:1}).should.have.nested.property({'a':'1'}); - }).to.throw('the argument to `property` must be a string'); + }).should.throw('the argument to property must be a string when using nested syntax'); err(function(){ ({ 'foo.bar': 'baz' }).should.have.nested.property('foo.bar'); From d58d2bfd1bfa7cb5c90bd7f9f2e93f8975985b04 Mon Sep 17 00:00:00 2001 From: solodynamo Date: Sun, 17 Sep 2017 13:07:08 +0530 Subject: [PATCH 08/10] assert tests modified --- test/assert.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/assert.js b/test/assert.js index 519334654..070953da4 100644 --- a/test/assert.js +++ b/test/assert.js @@ -1460,13 +1460,13 @@ describe('assert', function () { assert.propertyVal(dummyObj, 'a', '2', 'blah'); }, "blah: expected { a: '1' } to have property 'a' of '2', but got '1'"); - chai.expect(function () { + assert.doesNotThrow(function () { assert.nestedProperty({a:1}, '{a:1}'); - }).to.not.throw('the argument to property must be a string when using nested syntax'); + }, Error, 'the argument to property must be a string when using nested syntax'); - chai.expect(function () { + assert.throws(function () { assert.nestedProperty({a:1}, {'a':'1'}); - }).to.throw('the argument to property must be a string when using nested syntax'); + }, Error, 'the argument to property must be a string when using nested syntax'); }); it('deepPropertyVal', function () { From 84062b168020503f43183c04d87999371fe7dfcf Mon Sep 17 00:00:00 2001 From: solodynamo Date: Sun, 24 Sep 2017 18:26:28 +0530 Subject: [PATCH 09/10] Review comments changes --- lib/chai/core/assertions.js | 12 ++++++++++-- test/assert.js | 10 +++------- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/lib/chai/core/assertions.js b/lib/chai/core/assertions.js index 2d6676b10..293d4d678 100644 --- a/lib/chai/core/assertions.js +++ b/lib/chai/core/assertions.js @@ -1770,11 +1770,19 @@ module.exports = function (chai, _) { if (isNested) { if (nameType !== 'string') { - throw new AssertionError(flagMsg + 'the argument to property must be a string when using nested syntax'); + throw new AssertionError( + flagMsg + 'the argument to property must be a string when using nested syntax', + undefined, + ssfi + ); } } else { if (nameType !== 'string' && nameType !== 'number' && nameType !== 'symbol') { - throw new AssertionError(flagMsg + ' the argument to property must be a string, number, or symbol'); + throw new AssertionError( + flagMsg + 'the argument to property must be a string, number, or symbol', + undefined, + ssfi + ); } } diff --git a/test/assert.js b/test/assert.js index 070953da4..489b99e37 100644 --- a/test/assert.js +++ b/test/assert.js @@ -1460,13 +1460,9 @@ describe('assert', function () { assert.propertyVal(dummyObj, 'a', '2', 'blah'); }, "blah: expected { a: '1' } to have property 'a' of '2', but got '1'"); - assert.doesNotThrow(function () { - assert.nestedProperty({a:1}, '{a:1}'); - }, Error, 'the argument to property must be a string when using nested syntax'); - - assert.throws(function () { - assert.nestedProperty({a:1}, {'a':'1'}); - }, Error, 'the argument to property must be a string when using nested syntax'); + err(function () { + assert.nestedProperty({a:1}, {'a':'1'}, 'blah'); + }, 'blah: the argument to property must be a string when using nested syntax'); }); it('deepPropertyVal', function () { From f9e5a6841cdd8ad993a35e1d10583cdb4c21cac8 Mon Sep 17 00:00:00 2001 From: solodynamo Date: Fri, 29 Sep 2017 20:51:08 +0530 Subject: [PATCH 10/10] review comments fixes --- test/assert.js | 4 ++++ test/expect.js | 12 ++++++------ test/should.js | 12 ++++++------ 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/test/assert.js b/test/assert.js index 489b99e37..2bbd0f320 100644 --- a/test/assert.js +++ b/test/assert.js @@ -1456,6 +1456,10 @@ describe('assert', function () { assert.property(undefined, 'a', 'blah'); }, "blah: Target cannot be null or undefined."); + err(function () { + assert.property({a:1}, {'a':'1'}, 'blah'); + }, 'blah: the argument to property must be a string, number, or symbol'); + err(function () { assert.propertyVal(dummyObj, 'a', '2', 'blah'); }, "blah: expected { a: '1' } to have property 'a' of '2', but got '1'"); diff --git a/test/expect.js b/test/expect.js index a02c73d34..bb4fcbfca 100644 --- a/test/expect.js +++ b/test/expect.js @@ -1463,9 +1463,9 @@ describe('expect', function () { expect(undefined, 'blah').to.have.property("a"); }, "blah: Target cannot be null or undefined."); - expect(function () { - expect({a:1}).to.have.property(null); - }).to.throw('the argument to property must be a string, number, or symbol'); + err(function () { + expect({a:1}, 'blah').to.have.property(null) + }, "blah: the argument to property must be a string, number, or symbol"); }); it('property(name, val)', function(){ @@ -1760,9 +1760,9 @@ describe('expect', function () { .to.have.nested.property('foo.bar'); }, "expected { 'foo.bar': 'baz' } to have nested property 'foo.bar'"); - expect(function () { - expect({a:1}).to.have.nested.property({'a':'1'}); - }).to.throw('the argument to property must be a string when using nested syntax'); + err(function () { + expect({a:1}, 'blah').to.have.nested.property({'a':'1'}); + }, "blah: the argument to property must be a string when using nested syntax"); }); it('nested.property(name, val)', function(){ diff --git a/test/should.js b/test/should.js index 2a499f7af..f241e186b 100644 --- a/test/should.js +++ b/test/should.js @@ -1182,6 +1182,10 @@ describe('should', function() { err(function() { ({a: {b: 1}}).should.have.own.nested.property("a.b"); }, "The \"nested\" and \"own\" flags cannot be combined."); + + err(function () { + ({a:1}).should.have.property(undefined); + }, "the argument to property must be a string, number, or symbol"); }); it('property(name, val)', function(){ @@ -1409,13 +1413,9 @@ describe('should', function() { ({a:1}).should.have.nested.property('a'); - (function () { - ({a:1}).should.have.nested.property('{a:1}'); - }).should.not.throw('the argument to property must be a string when using nested syntax'); - - (function () { + err(function(){ ({a:1}).should.have.nested.property({'a':'1'}); - }).should.throw('the argument to property must be a string when using nested syntax'); + }, "the argument to property must be a string when using nested syntax"); err(function(){ ({ 'foo.bar': 'baz' }).should.have.nested.property('foo.bar');