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

Triangle: Update tests to match canonical-data.json. #437

Merged
merged 1 commit into from
Sep 27, 2016
Merged

Triangle: Update tests to match canonical-data.json. #437

merged 1 commit into from
Sep 27, 2016

Conversation

durrellchamorro
Copy link
Contributor

As discussed at exercism/problem-specifications#311 and exercism/problem-specifications#375 here are my proposed updates to the tests which reflect the canonical-data.json.

@kotp
Copy link
Member

kotp commented Sep 18, 2016

@durrellchamorro are you interested in doing the generator for the test so that it can be done once, and regenerated if that json file changes?

Copy link
Member

@kotp kotp left a comment

Choose a reason for hiding this comment

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

Quick review, this looks good. Would be even better if it were generated, to keep in line with that json file.

@durrellchamorro
Copy link
Contributor Author

I just looked at generator.rb and some of those cases.rb files. It's really neat how you generate the tests this way. I didn't realize that's how it is done and I'm not exactly sure how it all works, so I'll pass on writing the generator for this test. If you don't want to merge this PR, I understand.

@kotp
Copy link
Member

kotp commented Sep 20, 2016

It is definitely not that I don't want to merge this in, I just know that it will come back because it is not using that data directly, so this will (eventually) need to be revisited. With that said, the work is done in the form already, and so... it should come in.

👀 @exercism/ruby

@kotp
Copy link
Member

kotp commented Sep 26, 2016

If by no one objects, I will consider no objections to merging.

@Insti Insti changed the title Update tests to match canonical-data.json. Triangle: Update tests to match canonical-data.json. Sep 27, 2016
@Insti
Copy link
Contributor

Insti commented Sep 27, 2016

Correct manually created tests are better than incorrect tests. Merge 👍

@kotp kotp merged commit 7d89207 into exercism:master Sep 27, 2016
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.

3 participants