include improvement to support arrays better

[lobodpav]

When testing database code, I am getting an array of objects as a result of DB query. With MongoDB, one usually get an additional _id property for every object in the array.

Now, in unit tests, I need to check whether the array contains a sub-object. To do so, I need to write down a bunch of code like the one below. Generally, it searches for the first match using should.include().

var should = require('should');

var arr = [{_id: 1, a: 'a1', b: 'b1'}, {_id: 2, a: 'a2', b: 'b2'}];
var obj = {a: 'a1', b: 'b1'};

function shouldInclude(array, object) {
    array.some(function(val) {
        try {
            val.should.include(object);
            return true;
        }
        catch(err) {
            return false;
        }
    }).should.be.true;
}

shouldInclude(arr, obj);

It would be great to have the shouldInclude() and shouldNotInclude() functions as part of should module.

Actually, I would replace the ~this.obj.indexOf(obj) line 787 of lib/should.js by logic similar to mine.

In my view, this would be more consistent than the current behaviour because when object is provided, include searches for a sub-object. In array case, is search for an equal object. However, for arrays there already is includeEql function available.

Does it make sense to you?

[btd]

Yes, agree with your position. Actual problem is that originally .include worked like .indexOf for strings and arrays. I cannot use the same name and do not break any code. I think i will deprecate .include and split it to .indexOf - for strings/arrays and .contains for objects/arrays (that cover your case). What do you think?

[lobodpav]

What you suggest makes sense.

Moreover, I would expand your idea a bit by implementing these:

• contain - would work as you suggested, plus would also work for strings and simple arrays
  • 'hello boy'.should.contain('boy') would pass
  • [1,2,3].should.contain(3) would pass
  • [[1],[2],[3]].should.contain([3]) would pass
  • [[1],[2],[3, 4]].should.contain([3]) would pass
  • [{a: 'a'}, {b: 'b', c: 'c'}].should.contain({a: 'a'}) would pass
  • [{a: 'a'}, {b: 'b', c: 'c'}].should.contain({b: 'b'}) would pass
• containEql - would work for arrays/objects only and rather to search for sub-objects/arrays it would work the same way as eql but iterating over all keys/elements to find the match
  • [1,2,3].should.contain(3) would pass
  • [[1],[2],[3]].should.containEql([3]) would pass
  • [[1],[2],[3, 4]].should.contain([3]) would fail
  • [{a: 'a'}, {b: 'b', c: 'c'}].should.containEql({a: 'a'}) would pass
  • [{a: 'a'}, {b: 'b', c: 'c'}].should.containEql({b: 'b'}) would fail

These I would deprecate:

• include
• includeEql

And, no need to have indexOf. This way seems to me the most logical. Not 100% sure of [[1],[2],[3, 4]].should.contain([3]) would pass though.

[alsotang]

We should find a balance between less code and less confusion. Does the code of this suggestion occurs frequantly?? Or we can manually use Array.some or Array.forEach to do those assertions?

If [{a: 'a'}, {b: 'b', c: 'c'}].should.contain({b: 'b'}) would pass, I think should.js would be too confused.

[lobodpav]

Quite often, I use existing feature {a: 'a', b: 'b'}.should.contain({b: 'b'}) to equal to true when searching for data in DB query results. The data being returned may contain many properties but I am interested to check specific ones. In these cases, it is easier to define an object I am expecting the DB result object shall contain and then use result.should.contain(). Also, DB queries often return array of results. There comes the idea for array contain support.

You are right I can achieve the same by doing Array.some(). However, when I found out that {}.should.contain() searches for sub-objects, I tried to use it on arrays expecting it should work the same way. It did not work so I took a look into doc finding out it behaves differently. I.e. I was more confused by contain not working consistently when comparing behaviour on objects and on arrays.

If the doc clearly describes the difference between contain and containEql, e.g. contain to search for sub-objects/sub-strings/sub-arrays, and containEql to search for objects/arrays, it shall not cause confusion.

[btd]

Added in master as .containEql and .containDeep. If you can please, take a look if they cover your cases (you can look on tests). Reopen if it will be need.

[lobodpav]

Just to triple check if I understood your code and tests well:

1. .contain() implementation was unchanged
2. .containEql() searches for [object | array | string] which must be an exact match
3. .condainDeep() searches for sub-[object | array | string]

Am I correct?

[btd]

1. Yes i rest old names as is.
2. .containEql search for sub object and compare properties using .eql. It works for object, array, strings
3. .containDeep search for subobject in depth, not sure that i could explain whole, it is better to see examples.

[lobodpav]

I've checked the examples in docs and have a few commments:

1. .contain() is missing in the doc completely
2. .containEql() makes sense and works as one would expected. By the way, the [1, 2, 3].should.containEql(1); example in the doc is redundant as there is the same check on the second row [1,2,3].should.containEql(3);.
3. .containDeep() implements the check I was after but I think the naming is confusing. The name does not suggest:
   • whether it searches deeply for items using .eql() or .equal()
   • whether it searches for partial objects by using .eql. I.e. whether [1, {a: 1, b: {c: 2, d: 3}}].should.containDeep({b: {d: 3}}) is true or false

[btd]

1. .contain and .include are missing because i deprecate them (it is written in changelog). In replace with containEql
2. Ok, i removed it, will push later.
3. Agree about confusing name. It also uses .eql to compare. I assume that .contain* always check sub part of asserted object. But i agree that need better explanation.

[lobodpav]

Thanks for explanation.

To the point 3., .containEql() currently does not check for sub parts because as per doc, [{a: 'a'}, {b: 'b', c: 'c'}].should.not.containEql({b: 'b'}); is true while [{a: 'a'}, {b: 'b', c: 'c'}].should.not.containDeep({b: 'b'}); would be false. This means that just Deep version of contain checks for sub parts.

[btd]

[{a: 'a'}, {b: 'b', c: 'c'}] top object is an array, so .containEql search in sub part of top object, while .containDeep try to search in each enumerable property also.

[btd]

To compare:

> [[1], [2]].should.containEql([1]);
ok
> [[1], [2]].should.containEql(1);
AssertionError: expected [ [ 1 ], [ 2 ] ] to contain 1
> [[1], [2]].should.containDeep(1);
ok
> // end even
> [[1, 2, 3], [2]].should.containDeep(1);
ok
> [[1, 2, 3], [2]].should.containDeep([1, 2]);
ok
> [[1, 2, 3], [2]].should.containDeep([1, 4]);
AssertionError: expected [ [ 1, 2, 3 ], [ 2 ] ] to contain [ 1, 4 ]
>> [[1, [2, 3], 3], [2]].should.containDeep([1, 2]);
ok (because first and second contain)
> [[1, [2, 3], 3], [2]].should.containDeep([1, 3]);
ok
> [[1, [2, 3], 3], [2]].should.containDeep([1, 4]);
AssertionError: expected [ [ 1, [ 2, 3 ], 3 ], [ 2 ] ] to contain [ 1, 4 ]
> [[1, [2, 3], 3], [2]].should.containDeep([1, [2]]);
ok

[lobodpav]

Thanks for great details. It works a bit differently than I thought, these examples would be great in doc, especially the later complex ones.

However, I'm still not confident this shall pass [[1, [2, 3], 3], [2]].should.containDeep([1, 2]);. I would assume the function searches for [1, 2] array inside the parts of [[1, [2, 3], 3], [2]] but instead it searches for 1 and 2 separately. I.e. I would understand that [[1, [2, 3], 3], [2]].containDeep([2, 3]) will pass but not [1, 2].

[btd]

Yes, I also begin think the same. I will treat it inside one property. Also i think it would be ok, rename containDeep, to something like .propertyContain, or .inPropertiesContain ?

[lobodpav]

Great. As per the name, man, I got used to it so it is hard to think something up :)

I would keep the prefix as .contain so that it is clear the functionality is belonging to .contain 'group'. How about .containPropertySubset or maybe shorter .containSubset? It is about finding a subset in the set of each property from mathematical point of view.