tournament: Start with simpler tests / remove invald input test. #773
Conversation
|
Resolves: #720 |
| @@ -60,8 +60,3 @@ Devastating Donkeys;Courageous Californians;draw | |||
| ``` | |||
|
|
|||
| Means that the Devastating Donkeys and Courageous Californians tied. | |||
|
|
|||
| Your program should only process input lines that follow this format. | |||
There was a problem hiding this comment.
This PR comment is inspired by the fact that one of the questions asked in exercism/rust#122 is "should I expect invalid lines?" despite the fact that invalid lines were in fact included in the test suite at the time (so the answer to the question was specified in the test suite)
The alternative to removing completely is a sentence saying "You need only process input lines that follow this format, as invalid lines will not be tested."
I very weakly support the addition of such a sentence, because:
One might consider this a useful addition if students will ask this question and it is useful to get an answer without looking at the test suite. Note that it is easier to figure out "there is an invalid input" (you only need to look for any one test case that has it) than "there are no invalid inputs" (you need to look through all test cases and confirm that everything is valid).
Wouldn't it be the case that forcing the student to look at every test case to answer that question is going against the one-test-at-a-time metholodogy? Even if we pretended this is a scenario where I am writing the tests from scratch (instead of having them be provided but not run, as various tracks do), that I would want to know whether I would have to deal with invalid tests ahead of time?
Side notes:
- I support removing "program" because Is "write a program" accurate? #321.
- No opinion expressed on the one-test-at-a-time approach.
There was a problem hiding this comment.
Thanks @petertseng , good points.
"should I expect invalid lines?"
Isn't "invalid input will not be tested" up to the tracks to decide, since they are free to add or remove tests.
Wouldn't it be the case that forcing the student to look at every test case to answer that question is going against the one-test-at-a-time metholodogy?
Why would they be writing code to handle it without a failing test first? Once they get to the end they might notice that they have not had to handle invalid input.
(I'm not against adding something about test input validity to the description.)
There was a problem hiding this comment.
Wouldn't it be the case that forcing the student to look at every test case to answer that question is going against the one-test-at-a-time metholodogy?
Why would they be writing code to handle it without a failing test first?
I don't think someone doing test-first will write that code without a test. But I don't think this affects whether I would want this question answered (whether I want the sentence to appear in the README) before seeing/writing a test.
If I pretend that I'm writing tests from scratch, I imagine I am aware of some set of requirements before having written any test and they inform the tests that I will write. I would look to the README for those requirements.
The general question would be stated as "What is the role of an exercise's README? To what extent need it describe requirements and non-requirements?" I would say that exercism/go#189 affects how I think about this question (steers me to ask more of a README).
Maybe nobody will worry about this. After all, the proposed sentence states a non-requirement. The consequence of omission is that maybe people code too defensively. That's less severe than the consequence of omitting a requirement.
It also does not really affect me since I generally do look at all the tests beforehand.
"should I expect invalid lines?"
Isn't "invalid input will not be tested" up to the tracks to decide, since they are free to add or remove tests.
Oh! So benefit of leaving the sentence off is to give tracks the option. Okay.
I hope we don't run into a situation where all tracks that don't take the option have to each add a HINTS.md saying "we won't test invalid inputs" if we leave the option open and students express a preference for answering the question upfront in the README. Of course, we don't need to think about that until it becomes a problem.
As I hope is indicated by my prior approval, it does not matter to me whether the proposed sentence is added or not, but I'm happy to continue discussing if it inspires anyone else to have a strong opinion.
There was a problem hiding this comment.
My preference is to not add anything to the readme about what is tested.
I'd like to try the new readme that doesn't mention test input validity at all for a while and If it proves to be an issue in the future, someone can submit a PR to add the relevant language.
There was a problem hiding this comment.
I'm with @Insti: omitting that line about validity seems to make the most sense to me.
|
(https://github.com/petertseng/x-common/blob/verify/exercises/tournament/verify.rb says the new cases are good) |
| @@ -1,6 +1,6 @@ | |||
| { | |||
| "exercise": "tournament", | |||
| "version": "1.1.0", | |||
| "version": "1.3.0", | |||
There was a problem hiding this comment.
I might have missed it, but shouldn't this be 1.2.0?
There was a problem hiding this comment.
Removing the invalid input requirement was '1.2.0' adding the new initial tests made it '1.3.0', since '1.2.0' was not a separate PR and has never been released I can see an argument that we should stay on '1.2.0'.
I have a slight preference for '1.3.0' since it requires no work to change.
But if you think it should be '1.2.0' I'll change it.
There was a problem hiding this comment.
Ah I see. I don't really have a problem with using 1.3.0, I just didn't understand. It's fine to leave it as is
| @@ -60,8 +60,3 @@ Devastating Donkeys;Courageous Californians;draw | |||
| ``` | |||
|
|
|||
| Means that the Devastating Donkeys and Courageous Californians tied. | |||
|
|
|||
| Your program should only process input lines that follow this format. | |||
There was a problem hiding this comment.
I'm with @Insti: omitting that line about validity seems to make the most sense to me.
|
Merged. Thanks for the reviews. 👍 |
The previous version of these tests "threw you in at the deep end." and expected you to have a fully formed solution immediately.
This PR adds a series of smaller, simpler tests that can be incrementally passed while building up to a full solution.
Included: Removal of test for malformed input data.
This is not a problem about string parsing, so it is OK to expect that all test input will be valid.