assert.include: Added support for needles of 'Object' type.

[paraggupta1993]

"Assert.include: Added support for needles of 'Object' type."

Hey @johan , Currently assert.include(needle, haystack) doesnot support needle of Object type. Particularly, useful for asserting presence of a 'hash' in a array of hashes.

It throws error on

> assert.include
[Function]
> haystack = [{1:2},3]
[ { '1': 2 }, 3 ]
> needle = {1:2}
{ '1': 2 }
> assert.include(needle,haystack)
Error: include expected needle to be found in haystack
- needle: {"1":2}
haystack: [{"1":2},3]
    at error (node_modules/assertive/lib/assertive.js:443:12)
    at Object.assert.include (node_modules/assertive/lib/assertive.js:186:15)
    at repl:1:9
    at REPLServer.self.eval (repl.js:110:21)
    at Interface.<anonymous> (repl.js:239:12)
    at Interface.EventEmitter.emit (events.js:95:17)
    at Interface._onLine (readline.js:202:10)
    at Interface._line (readline.js:531:8)
    at Interface._ttyWrite (readline.js:760:14)
    at ReadStream.onkeypress (readline.js:99:10)

If there is any other way around. Kindly tell. Pardon my ignorance.

Otherwise, I tried to add the functionality myself. This PR is for the same. After adding this PR, assert.include checks presence of needle ( 'Object' type ) in haystack ('Array' type) as well.

> assert.include
[Function]
> haystack = [{1:2},3]
[ { '1': 2 }, 3 ]
> needle = {1:2}
{ '1': 2 }
> assert.include(needle,haystack)
undefined
>

I will add the test-cases once you agree for adding this functionality.

[EndangeredMassa]

This seems reasonable to me. @johan?

[johan]

Assertive already does support object needles, and any type of exotic needle including functions, host objects and whatever strangeness you can throw at it, but the test function is a "strictly equals" check (_.contains), not a "looks like" check (for comparison, same distinction as between equal and deepEqual).

Both modes of behaviour has its own merit, but I find the current behaviour the more useful and widely applicable for assertions about non-basic-types. Do you have an idea for a better verb than include for a duck-type inclusion check? I'm not convinced the value of that is great, but with a really good name, I could be swayed.

Here's how you do use assertive's include to check for objects:

> include = require('assertive').include
[Function]
> needle = {exotics: 'allowed', even_functions: include}
{ exotics: 'allowed',
  even_functions: [Function] }
> haystack = [null, 0, needle]
[ null,
  0,
  { exotics: 'allowed',
    even_functions: [Function] } ]
> include needle, haystack
undefined
> include {exotics: 'allowed', even_functions: include}, haystack
Error: include expected needle to be found in haystack
- needle: {"exotics":"allowed","even_functions":"[object Undefined]"}
haystack: [null,0,{"exotics":"allowed","even_functions":"[object Undefined]"}]

The latter invocation is testing for a different object than the one in the haystack. They may look alike, and have the same semantic content, but they are different independent object at different memory locations, and if you destructively modify one of them, the other one won't change to match, so they are different objects in a very real sense.

[johan]

For further perspective, the suggested patch breaks backwards compatibility by making my example no longer behave correctly, i e neither of the two would raise an exception. That kind of functional change could happen at a major version rev, if we realized our initial implementation or spec was fundamentally broken, but I don't find that to be the case here.

[jkrems]

Proposals inspired by [].some (untested, read as pseudo-code):

var assert = require('assertive'), deepEquals = assert.deepEquals;
some(deepEquals, 'descriptive string', needle, haystack);
// coffee syntax, with values:
// some deepEquals, { a: 10 }, [ null, 'foo', { a: 10 } ]

function some(assertion) {
  var assertArgs = [].slice.call(arguments, 1);
  var haystack = assertArgs.pop();
  var errors = [];

  var testFunction = function(element) {
    try {
      // we could also think about allowing the test function to return true/false/undefined
      // but I don't know how well that would play with existing assertions
      return assert.apply(null, assertArgs.concat([element])), true;
    } catch (err) {
      return errors.push(err), false;
    }
  };

  if (!_.some(haystack, testFunction)) {
    throw new Error("Guess what happens. Aren't I helpful?"); // use errors here
  }
};

[johan]

I like your thinking – having a top-level some (and every) actually does seem kind of neat.

The javascriptier way would be to pass them a function returning a truthy or falsey value rather than passing an assertive assertion that throws an exception on failure and otherwise doesn't, though, but maybe assertive is enough of its own test dsl to be a universe of its own rather than a piece of code embracing standard js conventions?

It'll be on the expensive side generating what could be a large amount of exceptions until we find the first non-exceptional function call, but it is of course not hard to implement. Giving some love to what errors look like would be the next step.

For every, it's probably best to just make the error note something about the length of the array, index of the first fail, and what that exception looks like. For some, where all we know on a failure is that every array member failed the assertion, it's a bit trickier what is the most useful error output (a sea of noise is rarely useful). Also the first error, and a note of the length of the array?

I think I'd call zero-element haystacks an argument format failure condition for both, as it's a meaningless thing to test for and easy enough to guard out in logic above the assertive invocation, rather than making a judgment call like "technically, every number in [] is evenly divisible by four". For some, we would not necessarily need any special casing of that – as long as the fail message is intuitive and understandable.

[jkrems]

I like the reusing of all existing assertions so I think treating "returns false" and "throws" as roughly the same would be nice. I'm not sure if it would be a breaking change to make sure all assertions return true if they pass.

As for generating a lot of exceptions: yes. For large collections that might be an issue. If we support "throws" and "returns false" as equivalent options, than we could add some kind of advisory to the readme ("If you see this taking an awful lot of time, consider writing a small filter/test function instead of using an assertion"). I'd expect <10 elements (or at least <100) in most "normal"/"sane" test suites, so I don't think the creation of error objects would be a bottleneck.

As for the summary: I think mentioning one of the errors would be enough, combined with a includes-like mention of the overall collection. E.g. the deepEquals error will print the needle, the haystack is printed by includes - combined you got the same amount of information that includes provide. Admittedly with 100% more noise (if stack is printed for the inner exception).

Zero-element: Fully agree. Fail fast when empty haystack (or anything _.isEmpty) is passed in.

Open question: arrays only or should object haystacks be allowed? See lodash.some:

_.some({x: 10, y: 12}, function(n) { return n > 11; }); // true
_.some({x: 10, y: 12}, function(n) { return n > 13; }); // false

[johan]

Both object and array types of collections seem fairly fair game to me, I think. Maybe swapping out for lodash while at it, especially if `.some` doesn't (it seems it does, though).

_.throws returns the caught exception object, when it passes, which is useful, but those are guaranteed to be truthy, so if that's the only one with a defined return value, maybe the false convention would be a viable performance saver for custom tests.

While the above isn't perhaps a full spec (some always tends to show up when hacking it up, or in peer review), I would be happy to get these two into an upcoming assertive version. I don't think I'll have the bandwidth to look at it myself soon, but would love it sooner rather than later, if someone else is game. :-)

While some and every are opposites, I don't know if assertive's current negation handling would prove beneficial or mostly in the way to implement it; feel free to bypass, if so. Edit: Not sure what I was thinking here; ignore – they are philosophically inverse quantifiers, but not opposites. :-) A proper some / every implementation should only look at as few elements as necessary to call a yay/nay is another obvious part of the spec worth calling out in advance.

[johan]

Are there standard functional programming names for the negatives of some and every, like someNot and none?

[jkrems]

I'd be in favor or notAll or notEvery instead of someNot. none really sounds better than notSome. But for many cases assert.some assert.notDeepEqual might be enough, not sure we need the inverse functions.

[johan]

Yeah, there is likely no need for the negated ones – but if we do find good names, there is a cognitive elegance and simplicity in none deepEqual ... that every notDeepEqual ... can't quite compete with.

[paraggupta1993]

Thanks @EndangeredMassa , @johan , @jkrems for looking into this. Also thanks for letting me know how we can assert for needles of object type in a haystack.

Reading further into conversation, @jkrems proposal of some, every and none would be of great use. And would keep include the way it was intended for ( which I didn't know ).

Also, allowing custom predicates which return true or false for each element in some, every and none would make things flexible for testers.

@jkrems : Are you planning to work on these ?

Thanks.

[jkrems]

I may get around to work on this next week, but I couldn't promise anything since I'll be on the road (or at least out of the country).

[EndangeredMassa]

What's the status of this?

[EndangeredMassa]

Is this still something we want to do?

[johan]

I'd like none and every meta assertions, but having not had the need for either myself yet, it hasn't felt like a pressing feature. I'd probably implement them myself if/when I encounter the situation, though.

[johan]

This no longer technically being a pull request, but just a holder for a good unrelated-to-the-title feature request, I'll close this now. assert.none, assert.some, assert.notAll and assert.every per above are welcome future additions if anynoe wants to hack at them.