Bob: Add test cases #1753
Bob: Add test cases #1753
Conversation
In a language tandem situation it's not uncommon to speak different languages in the same conversation, so the context still makes sense.
These don't cover new cases, they're merely fun and make the story more interesting, as well as adding non-US-American cultural references for a change Context: https://youtu.be/iuXR53ex4iI
| "scenarios": ["unicode"], | ||
| "property": "response", | ||
| "input": { | ||
| "heyBob": "FCÄEÜCÖDFCẞAB" |
There was a problem hiding this comment.
It's not clear in GitHub's fonts, but this is an uppercase ẞ, not a lowercase ß.
| "description": "Bob in Vienna", | ||
| "property": "response", | ||
| "input": { | ||
| "heyBob": "OIDA!" |
There was a problem hiding this comment.
Let's stick to English for non-unicode examples.
There was a problem hiding this comment.
Why? Can you tell me an equivalent phrase/word in English then?
There was a problem hiding this comment.
Why?
Because Exercism's exercises are written in English. Once we cross the mutlilingual bridge, we might consider how this changes, but for now, they're in English.
Can you tell me an equivalent phrase/word in English then?
No, because I don't know what the word means, but I can refer you to the first google result:
| @@ -127,6 +176,15 @@ | |||
| }, | |||
| "expected": "Whoa, chill out!" | |||
| }, | |||
| { | |||
| "uuid": "2a6431bd-43e3-495d-9fff-8834a387329a", | |||
| "description": "Bob in Vienna", | |||
There was a problem hiding this comment.
This doesn't tell me what is actually being tested in this test.
|
Presuming the purpose of these tests is to check for unicode, I don't think we need to be as exhaustive. I think one or two would surfice. For the others, the purpose of them isn't clear to me. Could you explain? |
There's one case per response. Many solutions define one method per response type, so it makes sense to check every response.
More variety, and they show off very concisely what the rules are since only case and punctuation change. There are no existing cases that manage to explain the rules of bob this concisely. |
I think my concern here is that these tests aren't adding any new test-behaviour, at least that I can see. They seem to duplicate the existing tests. Is there something that would pass the existing tests, but fail in these extra ones? |
I was under the impression that #1674 meant we could use this repo as a place to share test cases used by the tracks again without having to justify each individual addition. Apparently I was wrong. I don't feel like spending my time on discussions like this. |
|
I think if you're basically forking an exercise in a different direction, then it's best to fork it locally. If we end up with lots of variations of the same exercise in one repo, it's going to make it very hard for the tracks to know what set of tests sit together, and very hard for tracks to know whether they should be implementing the new tests etc without reading the whole body of PRs. I think if you're basically wanting to rewrite the tests, there's a fair argument for making a whole new exercise and putting it in problem specs, or just changing a language's specific one to do that. We really want exercises here to be the shared set of exercises that tracks can pick up and quickly implement. For the unicode tests in this PR though, I'm a 👍 on adding at least a couple of those, so if you want to reopen this for those, that would be welcomed as it's a simple toggle on/off for other tracks (supporting the unicode scenario).
The definition of the purpose of this repo in that issue is: "Problem Specifications exists to cover the "happy path". That means it should work for most tracks most of the time. In situations that aren't in the "happy path", it is down to individual tracks to solve those issues locally.". For an exercise like Bob, which is very old in Exercism-terms, there's a clear happy path defined. Adding the unicode tests adds extra value to the exercise, so would be welcome. But having alternative sets of tests proposed by each maintainer will lead to everything being very messy/complex, for no real value (as tracks can specify those alternative/additional tests using the mechanism we created for that purpose).
Entirely reasonable 🙂 |
|
Having spoken to @SaschaMann more about this, I can see the argument for the "vienna" tests if we could put them into more of some sort of "Bonus" tests (as per #1749). I also am (and was) happy for the unicode tests to be added, although I think we probably don't need all of them (specifically the gibberish one breaks the "someone speaking" metaphor a bit for me - but I don't have a hugely strong opinion either way. Sascha isn't interested in discussing this anymore, but is happy for us to reopen and merge if we want to. |
should be rebased, not squashed