roman-numerals 1.0.0.2: add descriptions #535
Conversation
|
I'm not sure if there was a reasonable way to split the commit apart (whitespace change) to make ti easier. I generated these programmatically. require 'json'
json = JSON.parse(File.read(File.join(__dir__, 'canonical-data.json')))
json['cases'].each { |c|
puts " , Case { description = \"#{c['description']}\""
puts " , number = #{c['number']}"
puts " , expected = Just \"#{c['expected']}\""
puts " }"
}... has anyone been doing these for any of the other tests? |
No, but I envy your magic code snippet. 👍 |
| @@ -17,66 +17,85 @@ specs = describe "roman-numerals" $ | |||
|
|
|||
| test Case{..} = it explanation assertion | |||
| where | |||
| explanation = show number | |||
| explanation = show number ++ ": " ++ description | |||
There was a problem hiding this comment.
like "27: 20 is two X's"
I wanted to keep the actual number being tested because in some cases the number in the description is not the input. Thought it might have been confusing to have the test output "20 is two X's" when the input is 27
There was a problem hiding this comment.
Yeah! The 27's description is confusing...
I still prefer the assertEqual solution, instead of changing the test's description, because it is shown only when it fails.
Anyway, you solution definitely solves the problem here! 👍
| @@ -17,66 +17,85 @@ specs = describe "roman-numerals" $ | |||
|
|
|||
| test Case{..} = it explanation assertion | |||
| where | |||
| explanation = show number | |||
| explanation = show number ++ ": " ++ description | |||
There was a problem hiding this comment.
Yeah! The 27's description is confusing...
I still prefer the assertEqual solution, instead of changing the test's description, because it is shown only when it fails.
Anyway, you solution definitely solves the problem here! 👍
* bowling 1.0.0.2: drop unneeded rolls, edit a description * leap 1.0.0.2: explain criteria in descriptions, put in progress order * luhn 1.0.0.2: only test isValid, use (most) x-common cases * phone-number 1.2.0.3: enforce area/exchange not starting with 1 * roman-numerals 1.0.0.2: add descriptions
|
Thanks for the reviews! Octomerging soon, then seeing what to do about pov. |
|
38 minutes from declaration of "Octomerging soon" to actual merge for 5 PRs, so it is not sure whether it is a time savings (depends on how long each Travis build would have taken), but at least it saves me from having to rebase and click buttons all the time. I still maintain that it's a win. I don't expect to have to do that too often though. Only when there are a lot of simultaneous PRs in flight, and since the last one I'm dealing with now is POV, we will be OK. |
exercism/problem-specifications#707