busterjs / referee-combinators

Combinators goes assertion
0 stars 1 forks source link

Don't rely on AssertionError from failure #5

Open johlrogge opened 11 years ago

johlrogge commented 11 years ago

Both attr and structure rely on assertions throwing exceptions. This is of course not correct behavior since the exception-throwing part can be configured in referee. It will work for us for a while longer while we have focus elsewhere but we will have to fix this when we are production ready.

meisl commented 11 years ago

Yep, thx for making it an issue. I've also added a deferred test with a name stating this TODO to make really sure we won't forget ;)

I'd like to volunteer for this one, but of course only after the longer while

...and I'd suggest to change the title into "Don't rely on AssertionError from failure" or so.

johlrogge commented 11 years ago

Feel free! See my comment in #6 for some input

meisl commented 11 years ago

Ok, started working on applying my approach from ramp-resources tests, as said in #10. And, as promised, trying to stay open for plugging in / replacing some parts by @johlrogge's 'internal' / 'raw' assertion results.

While making it a bit more concrete I came to think, fortunately, that actually there shouldn't be too much overlap. My "trick" from ramp-resources - basically: intercepting fail - is only a rather small and basic thing and layers around it are needed to make it reasonably usable. So that means it will be this (and not much more, if anything at all) that will be replaced by (or flanked by, as it may turn out) 'raw' assertion results.

In the following I'll be using the terms/definitions from #11, they'll appear in _bold italic_ - even though it looks pretty ugly...:( I'll adjust my description to whatever we decide to change wrt there (within a reasonable time frame).

So here's the agenda:

meisl commented 11 years ago

Re environment (last bullet): what's described in the README, is that working for you? That's what I'm doing right now.