Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

poker: create test case generator #686

Merged
merged 1 commit into from
Jun 3, 2017

Conversation

leenipper
Copy link
Contributor

Generate validTestCases array from the canonical-data.json.
Continue to use the invalidTestCases array in the test program,
since the .json had no invalid cases.

Update example solution to pass the test case with
an Ace .. 5 straight being lower value than a 2 .. 6 straight.

Update test program to use the generated validTestCases array.

Increment testVersion and targetTestVersion in
example solution and test program.

For #605.

Generate validTestCases array from the canonical-data.json.
Continue to use the invalidTestCases array in the test program,
since the .json had no invalid cases.

Update example solution to pass the test case with
an Ace .. 5 straight being lower value than a 2 .. 6 straight.

Update test program to use the generated validTestCases array.

Increment testVersion and targetTestVersion in
example solution and test program.
Copy link
Member

@petertseng petertseng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of the changes, if:

Update example solution to pass the test case with
an Ace .. 5 straight being lower value than a 2 .. 6 straight

It would be possible to put that in a separate commit if desired, if it comes before the new tests are added. The new tests depend on the new behaviour, but not the other way around. It does also make sense to put the change in the same commit with where the new code gets used though. Either choice is OK to me (let me know if anyone else has a preference for one or the other).

So it is good to merge as is.

@leenipper
Copy link
Contributor Author

in a separate commit if desired

Since the generator picked up the new test cases, it seemed to be reasonable to put the change for example in the same commit. I agree that it could go separately for clarity. I had put changes due to new test cases in same commit previously for adding a generator - see #664, so following that same line here.

I think I will let it stay for this one, and in the future use a separate commit for example update due only to test case change.

@leenipper leenipper merged commit 1cb1606 into exercism:master Jun 3, 2017
@leenipper leenipper deleted the poker-add-generator branch June 3, 2017 04:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants