Pass rate is considering "pending" tests as failed checks
Closed this issue · 7 comments
In your entrypoint.sh you have:
test_failed_string="- test"
[...]
elif [[ $temp =~ ^$test_failed_string ]] ; then
FAILED=$((FAILED+1))
[...]
But this will report a false positive on this perfectly passing report.
*** Run Summary ***
res://tests/integration/test_piles.gd
- test_popup_discard_view
[Pending]: Drawing a card from the pile, picks it from the popup
res://tests/integration/test_scripting_engine_general.gd
- test_basics
[Pending]: Empty does not create a ScriptingEngine object
res://tests/integration/test_scripting_engine_nested.gd
- test_draw_more_cards_than_pile_max
[Pending]: Test good execution of nested tasks
[Pending]: Test failing cost in nested cost task aborts parent script
res://tests/integration/test_scripting_engine_tasks_execute_scripts.gd
- test_no_costs_are_paid_when_target_costs_cannot_be_paid
[Pending]: is_cost == true execute_tasks: No costs were paid because target card could not pay costs
[Pending]: is_cost == false execute_tasks: Cost were paid because target card could not pay costs
[Pending]: is_cost == true execute_tasks: costs were paid when subject == boardseek
The script needs to be able to distinguish reports of pending tests from actual faulted tests.
Personally, I think this extra check on your end is not necessary. Have you seen instances where you will catch a failure by grepping for those lines starting with - test?
If you want to keep this behaviour intact, another approach is to add another metric, like "max_failed_asserts" which will only take into account failing asserts, as reported by GUT. This way someone can specify , say 0 max_failed_asserts, but allow 90% general passrate.
Never used pending as an option.
I'll just switch to comparing passing asserts to failing asserts + script errors. I'll add a fake assert for each script failure as they remove assert visibility from the run summary.
Sounds good, I still think a companion or mutually-exclusive metric like "max failed asserts", where you simply provide an integer, would also make sense.
Oh actually I remember why I set it up like that. I can look for a failing assert after getting a - test line, and as long as it's there I'll add a failed test.
I'll also add in an assert check option so people can go by assert counts instead of tests that adds a dummy assert failure for each script error it sees.
Oh actually I remember why I set it up like that. I can look for a failing assert after getting a - test line, and as long as it's there I'll add a failed test.
Yes, indeed that is how it works, but unfortunately this is just a report per test,and will include the pending asserts under the same header ;)
I'm saying that I'll require at least one failed assert for each test marked up in the run summary as it's not making sure of that atm.
You sound like you'd rather check asserts though so you'll want to use assert-check: true, which I'll throw in.
I'm saying that I'll require at least one failed assert for each test marked up in the run summary as it's not making sure of that atm.
Ah I see. That might require some creative grepping :D