Tests output panel misleads people to think that their solution passes all tests, but fails

[hobovsky]

Describe the bug

It's somewhat common, especially among newbies, that users interpret all-green output panel as a sign that their invalid solution passes all tests, but for some reason (not clear to them) gets rejected by the system.

It generally happens when all assertions in the test suite are successful, but it either crashes or is aborted by the runner. Such situation results in no distinctive, red messages, and users are not sufficiently hinted that tests fail because of problems with the submitted solution, and not for example the runner or system.

To Reproduce

There are several scenarios to cause similar behavior, some are more obvious that the others. I will post a few I remember, but I believe actual occurrences can differ by language, testing framework, and actual cause.

Tests aborted on timeout:

This kumite: https://www.codewars.com/kumite/5f2bececae0b73001a1b152a
This discourse: https://www.codewars.com/kata/5544c7a5cb454edb3c000047/discuss#5f2cee3f41132d000fc862b1 and many similar

I know there's a big message at the bottom hinting users what happened, but still amount of raised issues implies they don;t really get it. Needs more red.

Tests crash with stack overflow in Scala:

This kumite: https://www.codewars.com/kumite/5f2d79c341132d0023c94da6
This discourse: https://www.codewars.com/kata/5ce399e0047a45001c853c2b/discuss#5f2934af17642000193c1599

Notice how majority of posts contains phrase "my solution passes all tests, but fails", or similar.

I can think of more examples if you think it's necessary, but the problem generally applies to majority of cases when test suite crashes or exits forcibly.

Expected Behavior

If test suite is aborted due to invalid solution, I (and probably users) would expect it to be presented in a manner similar to failed assertions: with distinctive, red, big, difficult to miss message.

[kazk]

Thanks @hobovsky.

For the timeout, I agree that it should be more clear. Maybe a more clear message like "Execution timed out. Failed to pass all the tests." at the top.

For Scala crashing, it looks like a bug in the test output. CodeRunner is returning the information about the crashed test case, but the output is not showing it.

The test output UI is mostly unchanged from the old Code Runner days and it has some issues.

[hobovsky]

Do you need us to collect more cases? Do you need to collect every scenario in every language to have them fixed separately, or can all of them be corrected with the same fix?

[kazk]

I'm not sure what the cause is yet, but if the same thing (not showing the failure even the data is there) is happening with other languages, then it's possible that a same fix will fix them all.

So yes, I think it'll be helpful to have more cases.

[hobovsky]

How can I get a timeout but pass all the tests at the same time? I mean... Never happened something like this with me before.

https://www.codewars.com/kata/55c04b4cc56a697bb0000048/discuss#5f383c4253f391002e84eeaa

[kazk]

Changed how timed out is displayed. Failed: 0 was misleading, so I changed to ?. If there are n failures, it's displayed as n+. Added red Timed Out and fixed the green next to "Test Results".

Fixed crashed test not displayed:

[FArekkusu]

@kazk did you change the way it blocks without assertions are handled? This kata had a test structure similar to the following snippet:

describe("Random tests", () => {
  it("Tests", () => {
    let tests = [...];
    for (let t of tests)
      it(`testing for ${t.input}`, () => {
        assert.equal(solution(t.input), t.expected);
      })
  })
})

and it was uncompletable due to the outer it block having no assertions directly inside it.

The same thing happens in Python:

import codewars_test as test

@test.describe("Tests")
def _():
    @test.it("good") # this passes
    def _():
        test.assert_equals(1, 1)
    
    @test.it("empty") # this fails
    def _():
        pass

and Ruby 2.5 (but not Ruby 3.0):

describe "Tests" do
  it "good" do # this passes
    Test.assert_equals(f(1), 1)
  end
  
  it "empty" do # this fails
  end
end

It seems test failure is also triggered by the absence of PASSED messages, not only by presence of FAILED's. Did this change happen some time ago and nobody noticed/spoke about it, or is this a side effect of this update? Judging by the fact the issues started appearing today, it is indeed the result of this update. As I expected, many old katas/translation are broken now...

[kazk]

Ugh, I need to revert that then. Empty test cases were always treated as failed (p: false), but the UI wasn't showing it.

#69 (comment)