Skip to content
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

Chain .property() assertions on objects #193

Closed
bodokaiser opened this issue Sep 5, 2013 · 21 comments
Closed

Chain .property() assertions on objects #193

bodokaiser opened this issue Sep 5, 2013 · 21 comments

Comments

@bodokaiser
Copy link

Hello,

is there a workaround to allow something like this:

chai.expect({ foo: 'bar', bar: 'foo' })
  .to.have.property('foo', 'bar')
  .and.to.have.property('bar', 'foo')
  ;
@jeffjo
Copy link

jeffjo commented Dec 7, 2013

+1

Did you ever find a solution for this? Basically, we're looking for a way to reset the chain to the original subject within expect(...)

@bodokaiser
Copy link
Author

No, I didn’t.

should.js is missing this feature too…

Am 07.12.2013 um 18:18 schrieb Jeff Jo notifications@github.com:

+1

Did you ever find a solution for this? Basically, we're looking for a way to reset the chain to the original subject within expect(...)


Reply to this email directly or view it on GitHub.

@logicalparadox
Copy link
Member

Was thinking about this earlier. Two ways:

  • If a second argument is NOT provided then don't "indent" the topic.
  • Store a reference to the "parent" outdent and add a helper called .pop() that resets the topic back to the original.

Second would be better for backwards compatibility but the first makes more logical sense.

/cc @vesln @domenic for additional opinions

edit: logical typo

@bodokaiser
Copy link
Author

What do you think about:

obj.should.have.property('foo').and.should.have.properyt('bar').which.equals('foobar');

Means intention depends on the used word chain?

Am 07.12.2013 um 18:39 schrieb Jake Luer notifications@github.com:

Was thinking about this earlier. Two ways:

If a second argument is provided then don't "indent" the topic.
Store a reference to the "parent" outdent and add a helper called .pop() that resets the topic back to the original.
Second would be better for backwards compatibility but the first makes more logical sense.

/cc @vesln @domenic for additional opinions


Reply to this email directly or view it on GitHub.

@wbyoung
Copy link
Contributor

wbyoung commented Jun 7, 2015

I find that this would be especially nice when used in conjunction with @domenic's chai-as-promised to avoid repetitive .should.eventually. Currently you have to:

var profile = getProfile();
profile.should.eventually.have.property('id', 1);
profile.should.eventually.have.property('username', 'wbyoung');

But I think this would be a bit nicer chained:

getProfile().should.eventually.have.property('id', 1)
  .and.have.property('username', 'wbyoung');

In some ways, this could be solved via a properties assertion (which was discussed & understandably rejected in #72).

I like the outdent idea proposed by @logicalparadox best. Perhaps also could serve as this word:

getProfile().should.eventually.have.property('id', 1)
  .and.also.have.property('username', 'wbyoung');

@wbyoung
Copy link
Contributor

wbyoung commented Jun 11, 2015

I made a small plugin, called chai-also for this & just pushed it to npm. No browser support, but that wouldn't be hard to add if anyone is interested.

@keithamus
Copy link
Member

I quite like the idea of also... but it needs some more discussion to fill gaps; for example I can see people expecting to traverse deep objects like so:

var multiLayerObject = {
    foo: {
        bar: 1,
        baz: 2,
    },
    bing: {
        bong: 3
        bash: 4
    }
};

// Does it just go up a level?
expect(multiLayerObject).to.have.property('foo')
    .and.have.property('bar', 1)
    .and.also.have.property('baz', 2)
  .and.also.also.have.property('bing') // ???
    .and.have.property('bong', 3)
    .and.also.have.property('bash', 4);

// Or does it reset to the originally object within expect()?
expect(multiLayerObject).to.have.deep.property('foo.bar', 1)
    .and.also.have.deep.property('foo.baz', 2)
    .and.also.have.deep.property('bing.bong', 3)
    .and.also.have.deep.property('bing.bash', 4);

At the risk of sounding cynical - I can see either implementation getting a few issues springing up discussing semantics and trying to alter them to the users use-case.

Also worth pointing out that .property() isn't the only thing that sets the object to a new one; .throw() and .ownPropertyDescriptor() do this too - and potentially more will. We'll need a solution that works for all of these I believe.

@wbyoung
Copy link
Contributor

wbyoung commented Jun 12, 2015

@keithamus I agree that it could be confusing. After making this little plugin, I was thinking the same thing — it certainly could read either way. I chose reset to original in my plugin, but feel that having more options could work better. Perhaps also goes back to the previous subject, parent goes one level up, and subject returns to the original:

var obj = {
  foo: {
    bar: 'baz'
  }
};

expect(obj).to.have.deep.property('foo.bar.baz')
  .and.parent.has.property('baz'); // just return to `obj.foo.bar`

expect(obj).to.have.deep.property('foo.bar.baz')
  .and.subject.has.property('baz'); // just return to `obj`

expect(obj).to.have.deep.property('foo.bar')
  .and.also.have.deep.property('foo.bar.baz'); // return to subject before last change, `obj`

expect(obj).to.have.property('foo')
  .and.have.property('bar')
  .and.also.have.deep.property('bar.baz') // return to subject before last change, `obj.foo`
  .and.subject.has.deep.property('foo.bar.baz'); // just return to `obj`

And I don't think it sounded cynical at all, but I also had the same thoughts lingering in the back of my head. ;)

@reggi
Copy link

reggi commented Jul 2, 2015

I was pointed to this issue from my stack-overflow post here, so it seems there's no way to currently do this?

@wbyoung
Copy link
Contributor

wbyoung commented Jul 2, 2015

@reggi as of right now, not really. You can simply create multiple assertions as you suggested in your question on Stack Overflow. I agree with @keithamus that my suggestion of also can be confusing, and it probably makes sense to wait for decisions to be made here & something to be incorporated into the main library, but if you're adventurous (and using node), you can use chai-also.

@keithamus
Copy link
Member

I just remembered that we have .include which can test subset bodies:

chai.expect({ foo: 'bar', bar: 'foo', baz: 'bing' }).to.include({ foo: 'bar', bar: 'foo' });

This should be suitable for testing many properties. You can even combine it with property for complex nesting:

var multiLayerObject = {
    foo: {
        bar: 1,
        baz: 2,
    },
    bing: 3,
    bong: 4,
};
chai.expect(multiLayerObject)
  .to.include({ bing: 3, bong: 4 })
  .and.have.property('foo').include({ bar: 1 });

@wbyoung
Copy link
Contributor

wbyoung commented Feb 3, 2016

@keithamus awesome. The docs should probably be updated to show that this is possible!

@keithamus
Copy link
Member

Hey @wbyoung you're totally right. It looks the docs were missed in the original PR. Luckily it would be simple enough to add in - here is the comment which the docs are generated from - fancy making a PR?

@keithamus
Copy link
Member

FYI looks like this has been a feature since 1.9.0! A hidden feature tucked away, I only just realised it existed the other day myself!

@wbyoung
Copy link
Contributor

wbyoung commented Feb 3, 2016

Just curious — is it one of those things that works by happenstance or is it deliberate (i.e. test coverage)?

I'll try to get a PR up when I have a few min.

I'm excited about this discovery! Thanks for sharing.

@wbyoung
Copy link
Contributor

wbyoung commented Feb 3, 2016

Nevermind… looked at the PR you linked to & see that there are tests. Definitely deliberate.

@wbyoung
Copy link
Contributor

wbyoung commented Feb 8, 2016

@keithamus I just opened #610.

lucasfcosta pushed a commit to lucasfcosta/chai that referenced this issue Mar 14, 2016
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
@CodeTroopers
Copy link

I searched this feature for a while! Why the official documentation is not up-to-date one year after?

@fineline
Copy link

There's one case in the documentation where it does seem that the context goes back up to the top level, and I can't see an explanation for why this would be:

expect({a: 1}).to.have.property('b').but.not.own.property('b'); 

I am curious if anyone can tell whether this does indeed work as expected or just by coincidence?

@meeber
Copy link
Contributor

meeber commented Jan 25, 2018

@fineline Good catch. That's my mistake. It should be split into two assertions:

expect({a: 1}).to.have.property('b');
expect({a: 1}).to.not.have.own.property('b'); 

@fineline
Copy link

@meeber Thanks for clarifying that for me and others.

This was referenced Jun 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants