Remove all util functions from test files #1824
Comments
|
Apart from util functions, declarative macros should not be used either for the same reason. |
|
On second thoughts, macros might be fine. If we're going to get the source code of the tests by AST, macros should already be resolved by that point. And using tera templates for exercises which don't come from problem-specifications might be inconvenient. Macros can still be a good option in that case. |
|
Might also be a good idea to document this in |
|
For reference, here is an earlier discussion about using table-driven tests for this track. (its conclusion: table-driven bad, macros good) |
This is incorrect, AST parsing with |
[no important files changed] part of #1824
[no important files changed] part of #1824
[no important files changed] part of #1824 This is simply a manual expansion of the macros. There is no canonical data from problem specifications, so the tests won't change often and the maintenance burden of the duplication should be acceptable.
[no important files changed] part of #1824
[no important files changed] part of #1824
[no important files changed] part of #1824
[no important files changed] part of #1824 The small util function for constructing a `DateTime` value was left as is because it's only noise and not necessary to understand what's going on in the test case.
[no important files changed] part of #1824
[no important files changed] part of #1824
[no important files changed] part of #1824
[no important files changed] part of #1824
part of #1824 This is by no means perfect, but I attempted to make the test functions a little more concise and readable. This is desirable when the test code is shown to users online, while it wasn't a requirement when these tests were generated by macros.
part of #1824 This is another case where I think it makes sense to actually keep the util function around. It produces a good error message and in combination with the body of the test function, users should have enough information.
[no important files changed] part of #1824
[no important files changed] part of #1824 This is by no means perfect, but I attempted to make the test functions a little more concise and readable. This is desirable when the test code is shown to users online, while it wasn't a requirement when these tests were generated by macros.
[no important files changed] part of #1824 This is another case where I think it makes sense to actually keep the util function around. It produces a good error message and in combination with the body of the test function, users should have enough information. As the comment in the diff already states, the removed tests are redundant because the type system prevents such errors anyway.
[no important files changed] part of #1824
|
There are still a few of these around. Not every piece of testing code has been moved into a test function body. But I think for the remaining occurrences, it makes sense and the test functions are still understandable on their own. |
Some test files include some common code in the form of a funtion. Individual tests then call that function. This is a good instinct to do in normal code: reduce duplication (DRY). But our test runner should at some point (hopefully soon) gain the ability to extract the code of a test to show it to students when a test fails. In that case, it is very bad to call a separate function in the test body. Every test function should contain all of the logic required to understand what it's doing.
Here's an example of such a util function.
Since we're now using a test generator anyway, we don't need to worry about having to keep all this copied logic in sync manually.
Make sure to update the documentation where these util funtions are encouraged.
The text was updated successfully, but these errors were encountered: