search

okpy / ok-client

A Python client for the OK autograding system
https://okpy.org/
Apache License 2.0
57 stars 42 forks source link

Fix: always increment total case count #466

Closed akshitdewan closed 2 years ago

akshitdewan commented 2 years ago

earlier, when we had an infinite loop, we would not increment the total case count. As a result, when a student timed out on the second test case, the parsons tool would receive {passed: 1, total: 1} and incorrectly display that all test cases passed.

pamelafox commented 2 years ago

Is it possible to write a test for this, or is this part of the client untested so far?

pamelafox commented 2 years ago

I found https://github.com/okpy/ok-client/blob/master/tests/sources/common/interpreter_test.py, can a test be added there? (This logic feels like the sort of thing that could easily get bungled in the future)

akshitdewan commented 2 years ago

I found https://github.com/okpy/ok-client/blob/master/tests/sources/common/interpreter_test.py, can a test be added there? (This logic feels like the sort of thing that could easily get bungled in the future)

agreed, added a few tests in pyconsole_test.py

pamelafox commented 2 years ago

(Should await Vanshaj's review too though)

akshitdewan commented 2 years ago

lgtm, though i also think that code choice that pamela pointed out is a bit strange. non-blocking suggestion is to let the newline stay but to change the case assertions to account for it (but honestly that's also confusing so i don't think there's a winning alternative here).

I prefer having the right number of cases_total, otherwise the reader has to do an off-by-one calculation. I added a comment explaining the weird [1:]. I don't have merge permissions—could you merge it if it looks good?