Try to come up with an alternative to "to satisfy assertion"

[papandreou]

... That works better with the <assertion> type and is more well defined and documented.

I want to avoid this construct in all assertions that end with "satisfying" and accept an assertion string OR another value that will be used with to satisfy, eg.:

https://github.com/unexpectedjs/unexpected-stream/blob/master/lib/unexpectedStream.js#L85

Another example is the to have items satisfying family.

[papandreou]

The old idea of compound assertions came up again today on the gitter channel as a solution to this problem and eliminate the need for the weird (and luckily undocumented) to satisfy assertion hack.

Long story short, the ambiguity problem would be resolved if we change the meaning of <assertion> when it comes right after the assertion string so that:

expect.addAssertion('<any> to have items satisfying <assertion>', ...);

would allow:

expect([1,2,3], 'to have items satisfying to be a number');

whereas:

expect.addAssertion('<function> when called with <array> <assertion>', ...);

would still require the subsequent assertion to be passed as its own string:

expect(Math.max, 'when called with', [1, 2], 'to equal', 3);

Let's discuss implementation ideas here :)

[sunesimonsen]

There will be situations where you have to decide between multiple equally good solutions:

expect.addAssertion('foo bar <assertion>', ...)
expect.addAssertion('foo <assertion>', ...)
expect.addAssertion('baz', ...)
expect.addAssertion('bar baz', ...)
expect(subject, 'foo bar baz', ...)

We need to take these situations into account.

[sunesimonsen]

I don't think this would hurt performance for cases where <assertion> is not in the picture. That will still just be a hash lookup. When the hash lookup fails we would try to break the string into parts the are composed of existing assertions. If that fails we show an error.

I have a feeling that splitting a string into a set of known parts that covers the string perfectly NP complete - I'm not completely sure, but it sounds a lot like the knapsack problem.

I possible algorithm be to index all the assertion in a trie and use the trie to find prefixes of a string that matches an assertion. If you have that it is basically a recursive problem.

[papandreou]

That will still just be a hash lookup

Yeah, exactly. It would just be something to try if we end up in the current "no matching assertion found" case.

I think a simple greedy algorithm would have a shot:

// if lookupAssertionRule(subject, assertionString, args) fails:
var tokens = assertionString.split(' ');
for (var n = tokens.length - 1; n > 0 ; n -= 1) {
    var assertionRule = lookupAssertionRule(subject, tokens.slice(0, n).join(' '), args);
    if (assertionRule) {
        // Great, found the longest prefix of the string that yielded a suitable assertion
        // for the given subject and args
        // Save tokens.slice(n) somewhere and repeat the process when/if the assertion
        // calls expect.shift
        break;
    }
}

Actually, we wouldn't even be able to find the optimum solution without running the candidate assertions, since we don't know the type of the value that is expect.shifted to the next assertion. We simply don't know which assertions will be defined before that value is known.

I think the greedy approach will be the way to go. There will be cases where it'll have to give up, so we'll need to explore whether those are synthetic.

[sunesimonsen]

Great idea to lookup prefixes of word length, that would be fast enough. I think we need to slice the string completely in the first go, as we need it to be covered perfectly; otherwise we would end up failing for strings that can actually be covered.

How many levels of compound assertions are we going to support? As many as you like, right?

If we figure out how to cut up the string so it can be covered in a greedy way, wouldn't we be able to just call expect with all the parts and use the existing mechanism? Then it is just syntax sugar.

[papandreou]

Thinking about it a little more, I think it'll be sufficient to find the longest prefix of the assertion string that yields a supported assertion (the greedy algorithm) AND ends with <assertion> -- and not attempt to cover the rest of the string that time around. Just execute the found assertion. If it turns out that the rest of the assertion string is bogus, it'll cause an error when expect.shifting later.

There will of course be some (hopefully made up) cases where this method will pick an initial assertion that will render it impossible to find matching assertions to cover the rest of the string. But since quite few assertions end in <assertion> and don't tend to be substrings of one another, I think it's pretty safe to assume that won't happen.

[sunesimonsen]

I'm okay with that, this is pretty easy to get to work. But we should document how it works, as it is pretty surprising that it does not always work.

[papandreou]

Cool, let's go for that, then.

I agree that the limits should be communicated somehow. Maybe it would be possible to detect potential ambiguities at addAssertion time? The troublesome case is when an assertion ending with <assertion> is a prefix of another one that also ends with <assertion>?

[papandreou]

A very simple (but breaking!) solution would be to always require expect.it for non-compound assertions:

expect([1, 2], 'to have items satisfying', expect.it('to be a number'));
expect([1, 2], 'to have items satisfying to be a number');

So the ambiguous form would be prohibited:

expect([1, 2], 'to have items satisfying', 'to be a number');

Or rather, it would fail, because 1 and 2 do not satisfy the string to be a number.

[sunesimonsen]

I don't think we should do this. It would break a lot of stuff and its is also annoying you can't chose to break the assertions into there separate parts which makes it clearer what will happen.

[sunesimonsen]

I'm going to pick up the branch that organise the assertions into a tree again. Maybe that implementation will help give us more information on what is assertions and not.

https://github.com/unexpectedjs/unexpected/blob/sunesimonsen/yggdrasil/test/assertionTree.spec.js