Change scope to the property when using should.all.have.property

[Tyderion]

Hi there Thanks for creating this plugin it is quite useful.

One Problem I sometimes run into is the following.

In normal chai you can write something like this:

var a = {a: "a"};
a.should.have.property("a").not.equal("c"); // Test passes

var c = {a: "c"};
c.should.have.property("a").not.equal("c"); // Test fails

Is it possible to implement the same for all-validations?

Something like:

var a = {a: "a"};
var c = {a: "c"};
[a,c].should.all.have.property("a").not.equal("c"); // Test currently passes but should fail

That would be very handy.

[RubenVerborgh]

Thanks for reporting. Will look into this at some point.

[RubenVerborgh]

I added test cases for the issue you mentioned, but a fix turns out to be hard / impossible in general. The way Chai is built, I cannot tell when to switch on or off the all or something flag. At the point the assertion happens, we simply don't know what will follow in the chain.

I'm afraid I'll thus have to close this issue unresolved. Should you know a solution that satisfies the added test cases, don't hesitate to let me know or add a pull request.

[keithamus]

@RubenVerborgh or @Tyderion: just curious - have you filed an issue within chai about this? I'm not exactly sure how we could solve it - but I'd certainly like to at the very least discuss it in chai core.

[RubenVerborgh]

I haven't; I'm afraid it's not trivially solvable with the given chai API.

[keithamus]

I think I might have an idea about fixing this though:

Chai's .property changes the flag(this, 'object') to be the property you're asserting on - meaning if you do expect({ a: 1 }).to.have.property('a') then the chained value is 1 - which means you can do cool stuff like expect({a: 1}).property('a').to.equal(1). I think this is the issue that is gumming up chai-things (please correct me if I'm wrong though). Based on this assumption I think we can solve this particular problem.

If chai-things were to specifically hook off of .property and change the object flag to be an /array/ of the gathered properties, it would change the value of the chained assertion - which could then fall through to the standard plugin behaviour.

In other words:

var arr = [ {a: 1}, {a: 1} ]
expect(arr).to.all.have.property('a'); // this works fine

var values = arr.filter(item => item.a)
expect(values).to.all.equal(1); // This works fine

// As such there is no reason why this shouldnt work:
var assertion = expect(arr).to.all.have.property('a');
flag(assertion, 'object', values);
assertion.to.equal(1);

Thoughts @RubenVerborgh? Do you think it'd work? Or do you think I'm way off base here?

[RubenVerborgh]

I must admit, it has been years since I've looked into this. I'm not even fully sure how I implemented it back then. Your idea seems to make sense indeed… although I'm afraid there are some more complexities hidden somewhere.

Unfortunately, I don't foresee having time to look into this in the near future, with other projects occupying most of my time. If somebody would volunteer, I'd be happy to offer support.

[keithamus]

I'm happy to take a crack at a PR, I just wanted to gauge your opinion on it. So if you're happy I'll get working on it :smile:

[RubenVerborgh]

That would be great, thanks! There are simple tests but they are currently skipped.

Do ensure that it integrates with the existing code though; if I recall correctly, we there hold a nested assertion object in order to be able to step back in the chain. What would need to happen is that, at the moment the first assertion is tested on all elements of the array, the property it set should be remembered for possible chaining later.

I still think it will be quite hard to tackle cases like

[{ a: 1 }, { a: 2 }].should.include.something.with.property('a').equal(2);

because the first assertion would work on the first element, but the second wouldn't.

[keithamus]

Just an update - I've done some work on this, and progress is looking good. I'll put a PR up shortly. My main problem so far is that the something and all flags do not behave like other chai flags - they do not carry onto the next assertions, making the current assertions quite weird:

// this does not work:
[{ a: 1 }, { a: 2 }].should.include.something.with.property('a').equal(2);

// but this does!
[{ a: 1 }, { a: 2 }].should.include.something.with.property('a').something.equal(2);

I'll get that resolved soon (I hope) and I'll get a PR ready for the next session I do on this.

[RubenVerborgh]

@keithamus You mentioned you had a partial pull request for this. I believe that would really change how this library would work.

Just a question, but would you be interested in taking ownership of Chai-Things? I am focusing on other JavaScript projects at the moment and can't give this library the attention it requires.

[keithamus]

@RubenVerborgh I'd be happy to take over the project; I could pull it into the chaijs org, and make you a maintainer. The code would probably change quite significantly to come inline with Chai's code style though, to keep things consistent. Thoughts?

[RubenVerborgh]

Thanks! Can I transfer it to the chaijs organisation then?

I don't mind the code being changed. It's MIT, so go ahead!

[keithamus]

Sweet, yeah simply transfer it to the chaijs org and I'll pick it up when I can

[RubenVerborgh]

I didn't have permission to transfer because I'm not an admin of chaijs, so I'm transferring it to your personal account instead. You should then be able to complete the transfer.

[RubenVerborgh]

Thanks, I see the transfer has been completed!

[keithamus]

:+1:

[TueCN]

Hi @keithamus

Is there any updates to this issue?

I would really like to be able to use syntax like this for checking nested arrays: someArray.should.all.have.property('someOtherArray').that.not.all.have.property('someProp')

Is there a recommended workaround? I am not sure how to interpret your previous comment and make it work with allinstead of something

// this does not work: [{ a: 1 }, { a: 2 }].should.include.something.with.property('a').equal(2);

// but this does! [{ a: 1 }, { a: 2 }].should.include.something.with.property('a').something.equal(2);

[keithamus]

@TueCN I'm working on getting some other things in chai handled first, but when thats done I'll be spending time with Chai Things.

[TueCN]

Great! Thank you for your work :+1:

[auni53]

+1 because this would be great

[NenadP]

+1, it would be nice if I could do

someArray.should.all.have.property('someOtherArray').that.is.an('array')

[golsan]

+1

[truca]

@keithamus Is there any news on the subject? How could i help to solve this issue?

[keithamus]

It's unlikely any of the core maintainers can get to working on chai-things any time soon. We'll likely pick it up after chaijs/chai#585

[iknowkungfoo]

I've just recently been introduced to chai and am working on unit tests related to sorting. Trying to get this to work:

expect(data).should.all.have.property('attributes');

where data looks like

{
    "data": [
        {
            "id": "8",
            "type": "some_type",
            "attributes": {
                "a": 1,
                "b": 2
            }
        },
        {
            "id": "9",
            "type": "some_type",
            "attributes": {
                "a": 3,
                "b": 4
            }
        }
    ]
}

This is where chai-things has been tripping:

// The assertion's object for `all` should be array-like
var arrayObject = utils.flag(this, "object");
expect(arrayObject).to.have.property("length");
var length = arrayObject.length;
expect(length).to.be.a("number", "all object length");

saying that length is not a number. It's actually undefined. If I make this change:

// The assertion's object for `all` should be array-like
var arrayObject = utils.flag(this, "object");
arrayObject = arrayObject.__flags.object; // This is where the data object exists.
expect(arrayObject).to.have.property("length");
var length = arrayObject.length;
expect(length).to.be.a("number", "all object length");

Then all is well with the world.

How can I verify this is the correct fix?

[llopess]

+1