[WIP] nth-prime: Add test file generator. #411
Conversation
|
It is OK, because this exercise is not yet generated, and so does not need the BookKeeping information yet. Would you be up to writing the generator while you are here and interested? |
|
Sure 👍 |
Insti
changed the title
clettenberg
force-pushed
the
nth-prime-bookkeeping
branch
2 times, most recently
from
f99d227 to
29e7f58
Compare
clettenberg
force-pushed
the
nth-prime-bookkeeping
branch
2 times, most recently
from
21c627c to
2ebb7af
Compare
|
@kotp Just pushed up code for the generator. I also created a PR on x-common with the test case Let me know if I've missed something! |
|
|
||
| def test_bookkeeping | ||
| skip | ||
| assert_equal 2, BookKeeping::VERSION |
|
First look, this looks pretty good. Version number needs to be set back to 1. |
| 'test_%s' % description.downcase.gsub(/[ -]/, '_') | ||
| end | ||
|
|
||
| def do |
There was a problem hiding this comment.
I really don't like that there is a method called do,
I see it a lot, is there documentation that uses an example with do in it?
I'd prefer it be called actual to fit the assert_equal expected, actual pattern but am open to other suggestions for better names.
There was a problem hiding this comment.
This is true, it mirrors a keyword in Ruby, so is not a good feeling.
Let's avoid it if we can.
There was a problem hiding this comment.
There's no documentation on do. I had seen it in a few other files when I was working on this.
I'll change it to actual.
clettenberg
force-pushed
the
nth-prime-bookkeeping
branch
2 times, most recently
from
0887fe5 to
85d5686
Compare
|
I rolled back the version and updated it to reflect the changes to the x-common PR |
| def <%= test_case.name %><% if test_case.skipped? %> | ||
| skip<% end %> | ||
| assert_equal <%= test_case.expected %>, <%= test_case.actual %> | ||
| end |
There was a problem hiding this comment.
It would be good to remove the conditional from the template.
See how https://github.com/exercism/xruby/blob/master/lib/rna_transcription_cases.rb handles this.
|
Waiting on exercism/problem-specifications#332 which needs to be merged first. |
| @@ -22,13 +25,35 @@ def test_sixth_prime | |||
|
|
|||
| def test_big_prime | |||
| skip | |||
| assert_equal 104_743, Prime.nth(10_001) | |||
| assert_equal 104743, Prime.nth(10001) | |||
There was a problem hiding this comment.
If you wanted to be nice, you could make a helper method that formatted the numbers from the json into underscored versions in the test file.
(This is not at all a requirement, just an optional extra if you feel like it.)
clettenberg
force-pushed
the
nth-prime-bookkeeping
branch
from
85d5686 to
9b1b74b
Compare
|
This looks ready to merge to me. 👍 |
|
Still waiting on exercism/problem-specifications#332. Perhaps you could prod that along. |
|
The x-common PR has been merged. Is this PR in sync with the master branch of x-common and ready to be merged? |
clettenberg
force-pushed
the
nth-prime-bookkeeping
branch
from
9b1b74b to
cc9d7e8
Compare
|
@Cohen-Carlisle Sorry for the late reply. I think this PR is good to go with the recent update to the |
|
Thanks a lot @cacqw7 ❤️ |
A consonant+qu should be moved to the end of the word, but not a vowel+qu. I've added a test case for this, as I've seen multiple solutions where this rule wasn't followed exactly.
The Nth Prime problem was missing
BookKeepingmodule.