Allow more than one assertion string?

[papandreou]

@gertsonderby asked about whether Unexpected 10 would help with making the assertions more grammatically correct by allowing "filler words" in the argument list, and I thought about that for a bit. Early on we ruled out the possiblity of supporting multiple subjects, because we need the assertion string to be passed as the second parameter. But maybe that doesn't rule out the possiblity of allowing some of the other parameters to be assertion strings as well, possiblity even including flags?

expect.addAssertion('<number> to be in range from <number> up to [and including] <number>', function (expect, subject, lower, upper) { ... });

[sunesimonsen]

I think it's possible, but I don't think the benefits outweighs the complexity.

[papandreou]

Fair enough, thanks for taking the time to consider it :)

[sunesimonsen]

Sorry, I didn't mean to just wipe it of the table. I'm not done considering
it, that is just my initial feeling. I'll try to figure out the
consequences of this change. I just don't have the time right now.
On Oct 10, 2015 4:34 PM, "Andreas Lind" notifications@github.com wrote:

Fair enough, thanks for taking the time to consider it :)

—
Reply to this email directly or view it on GitHub
#225 (comment)
.

[papandreou]

No worries, I read your reply as a candid reaction, not as a definitive rejection. I tend to agree that it would add a tad too much complexity, unless it could be made to fit into a generalization of the "pattern matching" code in lookupAssertion that doesn't special case any of the arguments -- if we want that, of course.

[sunesimonsen]

I guess it would require all the fragments to be concatenated to an assertion key. But there are a lot of things that are structured around having one assertion string.

[inyono]

Related to this: http://unexpected.js.org/api/addAssertion/ should have a warning/note that the pattern can only have one single assertion string. From the description alone it is not clear that this is not possible and if you do this nonetheless, you get a confusing error message.

[papandreou]

@inyono Good point. I'm on my phone, so I can't try and see what the error message is, but it looks like we're missing that check.

Does that mean that you have a use case for multiple assertion strings?

[inyono]

@papandreou. You get TypeError: types.some is not a function

Yeah, basically the same as the one described above, i.e. using filler word to make assertions with multiple subjects more readable. But that is definitely just nice to have, especially since you could at least change the shown error message, I guess. For example, my first try on an assertion for Redux reducers:

export const unexpectedReducer = reducer => ({
  installInto: expect => {
    expect.addAssertion('<object|undefined> to be transformed to <object> <object?>',
      (expect, stateBefore, stateAfter, action = {}) => {
        if (stateBefore) {
          deepFreeze(stateBefore);
        }

        expect(reducer(stateBefore, action), 'to equal', stateAfter);
      });
  }

Initially, I wanted to have something like <object|undefined> to be transformed to <object> when handling <object?>.

[papandreou]

@inyono, thanks, I added a note about it to the docs (http://unexpected.js.org/api/addAssertion/) and landed a commit on master so that addAssertion will fail with Only one assertion string is supported (see #225) as of the next release.

[papandreou]

@inyono Thanks for describing your use case, that sounds reasonable. We'll keep it in mind if we end up revisiting this potential enhancement.

[papandreou]

@bruderstein mentioned another use case for this on the gitter channel today:

bruderstein:

        it('calls click on a part of a component', () => {
            const renderer = TestUtils.createRenderer();
            renderer.render(<ClickableComponent />);

            expect(renderer, 'with event', 'click', 'on', <span className="item-click" />, 'to have rendered',
                <div>
                    <span className="item-click">Item clicked 1</span>
                </div>);
        });

papandreou: @bruderstein Whoa! How do you consume the optional 'on', element in with event? Looks like something that currently requires putting in <any*> and then fiddling around.
(looks like another use case for #225)

bruderstein Mar 27 23:31
@papandreou much easier - the with event does an expect.shift( { $$typeof: SYMBOL_FOR_PENDING_EVENT, renderer: subject }), then there's a on , that adds a target property to the subjects and shift's again. Then, there's a to have rendered , which actually calls the event, and then delegates to to have rendered .
Unexpected is just so awesome for this stuff.
#225 would be really useful here - obviously you can already use 'with event click', but with event click on doesn't seem to work....

[boneskull]

I'm interested in this. Right now, this works fine:

expect(() => foo.emit('bar'), 'to emit from', foo, 'bar');

I'm thinking this is actually more of a natural sentence, however:

expect(() => foo.emit('bar'), 'to emit', 'bar', 'from', foo);

Yet, this is more parameters than the first form, and is thusly clunky. So, be careful what you wish for, I guess. 😄

---

Going a bit further, for my particular use case, the following would work, since the event name is a string:

expect(() => foo.emit('bar'), 'to emit "bar" from', foo);

Would this be terrible to implement and/or support? Granted, it's a special case around handling of strings, which introduces some inconsistency into the API. However, coupled with Unexpected's support for custom types and/or JavaScript's type coercion (e.g. MyThing.prototype.toString() will be called), this could open up new ways to write elegant assertions.

If it's possible to implement the above in a plugin, please let me know? I may want to try it.

Apologies if this has been brought up before!

[bruderstein]

I think there's currently 2 ways to do your first suggestion:

expect(() => foo.emit('bar'), 'to emit', 'bar', 'from', foo);

... and no way yet to do the string-inlined version.

To implement the above, the two options are:

1. Have two assertions. The first one something like:

expect.addAssertion('<function> to emit <any> <assertion>', function (subject, value) {
  expect.shift({ emitter: subject, expectedEmitted: value, typeId: 'EMITTER_PARTIAL' });
});

And the second uses the new type

expect.addType({
   name: 'EmitterPartial',
   identify: function (value) {
        return value && typeof value === 'object' && value.typeId === 'EMITTER_PARTIAL';
  }
);

expect.addAssertion('<EmitterPartial> from <event-emitter>', function (emitterPartial, emitter) {
   // do your thing - the 3 parts are emitterPartial.emitter, emitterPartial.expectedEmitted, emitter
});

2. The second way is much more straightforward and utilises the fact that you always get all the arguments even if you don't "request" them.

expect.addAssertion('<function> to emit <any> <assertion?>', function (subject, value, maybeFrom, maybeEmitter) {
  if (maybeFrom !== 'from') {
     // fail with some error message - 
    expect.fail('You forgot the `from` after `to emit`');
  }
  // do your thing, parts are subject, value, maybeEmitter

});

I've used both of these - unexpected-react uses the first, and a private project uses the second. The second is definitely much easier, except if you are expecting a continuation of assertions (i.e. you have an assertion after 'to emit', 'foo', 'from', bar, 'and something satisfies', { id: 42 }, you need to use rest params or the equivalent arguments dance, not expect.shift.

I don't know how difficult the "in-string" version is to implement - this issue was raised as a placeholder for it.