Add todo reporting#67
Conversation
|
Hello! Are you still working on this? |
|
Sorry, I thought I had marked it ready for review already. |
lpil
left a comment
lpil
left a comment
There was a problem hiding this comment.
Thank you!! I've left a comment inline.
|
|
||
| handle_cancel(_test_or_group, Data, State) -> | ||
| ?reporting:test_failed(State, <<"gleeunit">>, <<"main">>, Data). | ||
| handle_cancel(test, Data, State) -> |
There was a problem hiding this comment.
What's this new code for? It seems unrelated to todo?
I think it might be code for another feature that got included by mistake?
There was a problem hiding this comment.
I added it to support a case when the todo was hit inside a spawned process ( i.e. OTP ), it wasn't reporting it correctly as a todo. If you prefer to leave that case unsupported this can be removed, and they'll continue to fail via error as current behaviour.
There was a problem hiding this comment.
I couldn't really add a good test case to demonstrate / cover this within the tests without bringing in dependencies that wouldn't be wanted in this repo.
3 participants
Satisfies #61
This adds todo reporting as a separate category in the final summary.
When there are only failing todos and no failures from asserts or panics, then this will show yellow, but still exit code 1, so will still fail in CI environments, to maintain compatibility with current behaviour.
There are a number of accomodations that had to be made to get this tested within this repository. I had to suppress the raw test output, so that it is not confused with the tests being asserted in the test module. Then in order to that in a compatible way, I had to special case Node and Deno handling.