forth: Rewrite tests to use hspec with fail-fast. #388
Conversation
| runTexts ["1 2 3 4 5"] `shouldBe` Right "1 2 3 4 5" | ||
|
|
||
| it "non-word characters are separators" $ | ||
| runTexts ["1\NUL2\SOH3\n4\r5 6\t7"] `shouldBe` Right "1 2 3 4 5 6 7" |
There was a problem hiding this comment.
unrelated to this PR, but now I would like to figure out the origin of this test...
There was a problem hiding this comment.
found it exercism/exercism#1188
| , testCase "numbers just get pushed onto the stack" $ | ||
| Right "1 2 3 4 5" @=? runTexts ["1 2 3 4 5"] | ||
| , testCase "non-word characters are separators" $ | ||
| -- Note the Ogham Space Mark ( ), this is a spacing character. |
There was a problem hiding this comment.
If there is someone who does not read the README but only look at the tests, the lack of this comment might make things confusing. Do you think this is something to worry about? (Are there people who don't read the README?)
There was a problem hiding this comment.
I think that, ideally, people should get all the information needed to solve an exercise in the following order:
- Running the tests - As a general rule this should be enough to know what went wrong.
- Reading the hints - This could help when people get stuck.
- Inspecting the test suite - Forcing people to read code should be a last resort solution, but I understand that some things are too complex to explain in English.
If there is someone who does not read the README but only look at the tests, the lack of this comment might make things confusing.
It could happen but, if HINTS.md is the right place to have the hints, we should enforce that there are no hints in the test suites or in the stub solutions.
IMHO, the only comments that should be in the test suites are the ones related to the test's implementation, to assist us in maintaining the tests suites.
An alternative would be to improve the tests and descriptions, if you think they are not good enough.
But this is just my opinion. If you think it is important to keep the comment there, I can update the PR immediately.
There was a problem hiding this comment.
I think that, ideally, people should get all the information needed to solve an exercise in the following order:
This seems to make sense; I think it represents how we would interact with a library we are considering to use - we either run the tests or read its documentation (some people switch the order of these two don't they) and only as the resort will they read the code.
When I am solving an exercise I tend to look quite often at the tests (so I know what's coming up next and I know what's expected of me), but I can't speak for everyone.
I think my main question is - now that this line appears in the README, when someone looks at that, what do you think their reaction will be? Will it be "I'll immediately search for all instances of this character to see what this is all about", or will it be "Why are you telling me this? I will forget about it". Well, it's not my place to predict how people will act.
An alternative would be to improve the tests and descriptions, if you think they are not good enough.
I'm not sure if "non-word characters are separators, including the Ogham Space Mark ( )" would be too much detail...
I question the case because visually the Ogham space mark is very easy to mistake for a minus sign, so someone looking at this might think this is five minus six, rather than five space six.
Perhaps I will take solace in the fact that most people probably don't fail this test because isSpace will do the right thing.
There was a problem hiding this comment.
think my main question is - now that this line appears in the README, when someone looks at that, what do you think their reaction will be? Will it be "I'll immediately search for all instances of this character to see what this is all about", or will it be "Why are you telling me this? I will forget about it". Well, it's not my place to predict how people will act.
Maybe it doesn't belong in README...
After some thought, I think that we should simply remove the comment. It is too specific!
If it is part of testing non-word character, it doesn't deserve a comment more than NULL, SOH or tabs. If it is really a special case, it should have its own test.
What do you think?
There was a problem hiding this comment.
I imagine the only reason it was there in the first place is so that people don't confuse it with the minus sign.
If I ever write the canonical data for this problem, I plan not to include the Ogham space. But that is a question for another day.
As far as I can tell, it is not a special case at all. Just another whitespace character. It's not really adding anything to the test.
So what does this mean! I noticed recently that I often imagine problems but don't have evidence that others will experience the same problem (in this case, the problem is "people might mistake the Ogham space for a minus sign"). So if you think the comment should simply be removed, let's do it. Otherwise we won't know if others have the same problem.
There was a problem hiding this comment.
If I ever write the canonical data for this problem, I plan not to include the Ogham space. But that is a question for another day.
I agree! It adds nothing!
So if you think the comment should simply be removed, let's do it. Otherwise we won't know if others have the same problem.
Done! 😄
|
Suggest that "- Move hint from test suite to |
|
Sorry for that, I completely forgot to review the commit message. Fixed! 😬 |
|
It may be time for me to write that canonical-data.json soon if I want to get rid of that Ogham space mark... |
hspec.Related to #211.