New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support `{a:1,b:2}.should.include({a:1})` #230

Merged
merged 3 commits into from Jan 27, 2014

Conversation

Projects
None yet
6 participants
@jkroso
Contributor

jkroso commented Jan 5, 2014

I also removed some type checking within the assert interface because it was getting in the way a bit. I hope thats ok.

@coveralls

This comment has been minimized.

coveralls commented Jan 5, 2014

Coverage Status

Coverage remained the same when pulling 2715e95 on jkroso:include into f3ebb09 on chaijs:master.

@jkroso

This comment has been minimized.

Contributor

jkroso commented Jan 5, 2014

looks like I need to clone the assertion to fix that test case. Anyone know how to do that properly?

@logicalparadox

This comment has been minimized.

logicalparadox commented on lib/chai/core/assertions.js in 2715e95 Jan 5, 2014

You probably want to use util.type here as typeof [] === 'object' which I don't think you want.

@jkroso

This comment has been minimized.

Contributor

jkroso commented Jan 5, 2014

I just realized I'm not handling negations properly too so I'll need to refactor anyway

@logicalparadox

This comment has been minimized.

Member

logicalparadox commented Jan 5, 2014

To handle negation properly you will need to use: https://github.com/chaijs/chai/blob/master/lib/chai/utils/transferFlags.js

@coveralls

This comment has been minimized.

coveralls commented Jan 5, 2014

Coverage Status

Coverage remained the same when pulling acbf04d on jkroso:include into f3ebb09 on chaijs:master.

@vesln

This comment has been minimized.

Member

vesln commented Jan 5, 2014

karma...

@jkroso

This comment has been minimized.

Contributor

jkroso commented Jan 5, 2014

works but needs karma-phantomjs-launcher added to the dependencies list

@vesln

This comment has been minimized.

Member

vesln commented Jan 5, 2014

Yeah, we just shouldn't be using canary because these things happen all of the time lately @logicalparadox

@logicalparadox

This comment has been minimized.

Member

logicalparadox commented Jan 5, 2014

I had it using canary because karma-sauce wouldn't run alongside the karma in npm. I haven't checked recently to see if this works.

If it still doesn't work with the karma from npm, perhaps we could lock to a commit or tag from gh://karma...

@lbdremy

This comment has been minimized.

lbdremy commented Jan 15, 2014

+1 Exactly what I was looking for in chai.

@vbardales

This comment has been minimized.

vbardales commented Jan 16, 2014

is it possible to add multi-level objects support ?
like here: https://github.com/vbardales/chai-properties (no negation support here)

            subject = {
              a: 'a',
              b: {
                b1: 'b1',
                b2: {
                  b21: 'b21',
                  b22: 'b22',
                },
                b3: {
                  b31: 'b31',
                  b32: 'b32',
                },
              },
            };

subject.should.include({ b: { b2: { b22: 'b22' }}});
subject.should.include({ b: { b2: { b22: 'b22' }, b3: { b32: 'b32' }}});
subject.should.not.include({ b: { b4: 'b4' }});
subject.should.not.include({ b: { b2: { b22: 'x' }}});
@lbdremy

This comment has been minimized.

lbdremy commented Jan 16, 2014

What is missing to merge this in master?

@jkroso

This comment has been minimized.

Contributor

jkroso commented Jan 27, 2014

@vbardales would definitely be nice to be able to do deep includes. I've wanted it twice now since making this PR

vesln added a commit that referenced this pull request Jan 27, 2014

Merge pull request #230 from jkroso/include
support `{a:1,b:2}.should.include({a:1})`

@vesln vesln merged commit a01e40b into chaijs:master Jan 27, 2014

1 check failed

default The Travis CI build could not complete due to an error
Details

wbyoung added a commit to wbyoung/chai that referenced this pull request Feb 8, 2016

Update include assertion docs for object properties
Originally added in chaijs#230, asserting that an object contains a subset of
properties and values was never documented. Questions like chaijs#193 have
popped up asking for this feature even though it's been present since
1.9.0.

The discussion in chaijs#193 focused mostly around getting something like
this to work via the `property` assertion, but that discussion should
be considered moot by the existing functionality already present in
`include` that was simply overlooked.

Resolves: chaijs#193

lucasfcosta added a commit to lucasfcosta/chai that referenced this pull request Mar 14, 2016

Update include assertion docs for object properties
Originally added in chaijs#230, asserting that an object contains a subset of
properties and values was never documented. Questions like chaijs#193 have
popped up asking for this feature even though it's been present since
1.9.0.

The discussion in chaijs#193 focused mostly around getting something like
this to work via the `property` assertion, but that discussion should
be considered moot by the existing functionality already present in
`include` that was simply overlooked.

Resolves: chaijs#193
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment