Control traceback length via assignment config #536
Conversation
|
Note to self, should change to allow for any type of error to be propagated and not just assertion errors to account for situations where the failure message is not helpful to understand the actual reason the test fails, e.g. an undefined variable name:
|
There was a problem hiding this comment.
I think some thought should go in to how this gets specified; specifically, the granularity of this configuration. Is it something that gets applied in the assignment config and applies to every test in the assignment, or can it be specified on a question-by-question or test case-by-test case basis? If the latter, then this is something that gets specified in the question or test case config, which changes how you access this information when generating each test case.
Regardless, generally the way to pass configurations to the TestFiles is to make it part of a key in the OK test format or a keyword argument in otter.test_files.test_case (for exception-based tests). Take a look at OKTestFile.from_spec to see how the OK format is parsed. You would then pass the value from the Assignment instance into one of these in this function (or, more likely, in one of the functions/templates it uses).
| @@ -16,6 +16,13 @@ class Assignment(fica.Config, Loggable): | |||
| Configurations for the assignment. | |||
| """ | |||
|
|
|||
|
|
|||
| traceback_length: Optional[str] = fica.Key( | |||
There was a problem hiding this comment.
nit: please add new configurations at the bottom of the Keys (i.e. after runs_on)
| from ..assign.assignment import Assignment | ||
| if Assignment().traceback_length == 'full': | ||
| return False, runresults.getvalue() | ||
| elif Assignment().traceback_length == 'assertion_msg': | ||
| err_msg = runresults.getvalue() | ||
| if 'AssertionError: ' in err_msg: | ||
| return False, err_msg[err_msg.index('AssertionError: '):] | ||
| else: | ||
| return False, '' | ||
| elif Assignment().traceback_length == 'none': | ||
| return False, '' |
There was a problem hiding this comment.
Calling Assignment() just creates an Assignment instance with the default values, so traceback_length will always be the default.
My vote would be to make it an assignment wide option first since I imagine that the most common use case would be to change how all tests are displayed, but keep uniformity between tests withing the same assignment. It would always be possible to add an option later to control this on a test by test level if there is interest in that. And thanks for the pointers of how to pass the options around, I will have a look! |
|
One distinction I could see as useful would be to show the full traceback or at least the line that makes the assertion on gradescope if |
SGTM. Also, for adding this to the |
joelostblom
force-pushed
the
truncate-traceback
branch
from
bd16488
to
b258869
Compare
Resolves #419
@chrispyles This sits on top of #533, so the diff looks bigger than what it is. I can't quite figure out how to pass options from from the assignment config to the doctest runner. What I have currently only works for the default value, but it does not change when including the key in the first cell of the course notebook so I would appreciate any pointers you might have on how to achieve this. I'm also curious to hear your thoughts in general on the approach I have taken here.
These are the results of the three different options:
assertion_msgThis is helpful because the assertion message can change dynamically (such as inpd.assert_frame_equal) and is therefore more informative than a static manually written message:noneThis is the least verbose output that gives away the least possible cuesfullThis is the current default and in my opinion the hardest to read for student and it gives away the specific test that is being used: