search

unexpectedjs / should-be-unexpected

Should.js implemented with Unexpected.
3 stars 0 forks source link

should.ok() appears to be missing #5

Open alexjeffburke opened 8 years ago

alexjeffburke commented 8 years ago

Now what I hope is a genuine one - should supports a should.ok() which their docs says works like assert.ok().

gustavnikolaj commented 8 years ago

Sorry, but I haven't really ever used this for real - nor the original should...

How is this expected to work?

(true).should.ok();
(1).should.ok();
"foo".should.ok();

Sort of ĺike that?

gustavnikolaj commented 8 years ago

I copied in the test suite of should.js when I started, but I actually skipped the "assert" section: https://github.com/unexpectedjs/should-be-unexpected/commit/5081c51ad8eb9292f359151f43dddfcbb8f2a2f2

I don't know why I did that. I wasn't smart enough to document why...

I have the tests here: https://github.com/unexpectedjs/should-be-unexpected/blob/master/test/should.js-tests/ext/assert.test.js#L33-L48

So I know how to implement it. Maybe I just didn't expect people to actually use the assert interface with should.

This is turning out to be a real franken-library. Unexpected implementing Should implementing Assert. Assertception.

gustavnikolaj commented 8 years ago

I'll try to get a look at this asap so you can continue with your experiments :-)

alexjeffburke commented 8 years ago

It's as follows: should.ok(true); - yay should.ok(false); - boom

Interesting Q is probably if it should wrap 'to be true' or 'to be truthy', will try to test when back at comp a bit later.

And no rush :-)

alexjeffburke commented 8 years ago

Oh piddle, you're right - in should that ok() thing is literally a wrapper for assert.ok().

Frankenlibtastic!!

In all seriousness though, it's testing truthy values and I'd be tempted to emulate the monstrosity rather than also pull in assert by having it be expect(, 'to be truthy'). The only sticking point is the exception thrown in the negative case is a require('assert').AssertionError. How err compatible do you want to be? Because you could catch and throw one of them.. ducks while running for the hills

gustavnikolaj commented 8 years ago

For this and expect-the-unexpected, the guiding principle has been to emulate as close as possible, but opt out when we have a disagreement between unexpected and expect where the disagreement is a result of a conscious decision to do something better.

I haven't done any error mapping anywhere - I throw unexpected errors all over. I'd say that relying on the error type is an anti pattern; but then again - I haven't written a whole ton of tests using assert, so I don't really have the experience to tell if it's something you need to do.

So, I'd say go for implementing it - but go with unexpected errors unless you have a good reason why not to :-)

gustavnikolaj commented 8 years ago

If you take a look at #6 I added assert.ok and made those tests pass.

There's already a problem on the .equal though...

I'm not sure if we should actually just delegate to assert here...

gustavnikolaj commented 8 years ago

Actually, I think we shouldn't. The whole idea here is to display how much better an assertion library unexpected is, and lure people in little by little. Fixing the bad parts of asserts .toEqual etc is probably in line with the over all idea of the library.

alexjeffburke commented 8 years ago

So I think it's definitely better to run through unexpected assertions, but my first thought was nicely uniform output.

That being said I was pondering that earlier - aren't there other places that already deviate? I guess the answer to that might count as a precedent. Btw, does unexpected actually have any way to do loose equality?

If keeping the double equal behaviour, it'd be awesome to spell it out somewhere. If not, what about using expect.fail() to output a really useful message like "you're relying on loose quality" or something.