Skip to content

Add test for saddle points in row#1427

Merged
cmccandless merged 3 commits intoexercism:masterfrom
DocFogel:Add_test_for_saddle_points_in_row
Jul 27, 2018
Merged

Add test for saddle points in row#1427
cmccandless merged 3 commits intoexercism:masterfrom
DocFogel:Add_test_for_saddle_points_in_row

Conversation

@DocFogel
Copy link
Copy Markdown
Contributor

There is a test for multiple saddle points in a column, but there is no test for multiple points in a row.

These changes rename the existing test to indicate it tests for saddle points in a column and adds a test for multiple points in a row.

DocFogel added 2 commits July 22, 2018 20:40
There is a test for multiple saddle points in a column, but no test for
multiple points in a row.
Test for multiple saddle points in the same column renamed similar to
that for multiple points in a row
@cmccandless
Copy link
Copy Markdown
Contributor

Hi, thanks for the PR!

The way we have things structured here, any test suite containing the following comment are merely implementations of the global canonical data. We usually try to avoid adding tests that are not found there. However, I think this is a good test idea, so I would suggest creating a PR over at https://github.com/exercism/problem-specifications to add this case there. Once that PR is merged, this one may be updated to match the new canonical data version and merged here.

# Tests adapted from `problem-specifications//canonical-data.json` @ v1.1.0

@cmccandless
Copy link
Copy Markdown
Contributor

Awaiting merge of exercism/problem-specifications#1272.

Copy link
Copy Markdown
Contributor

@cmccandless cmccandless left a comment

Choose a reason for hiding this comment

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

Changes look good. Please update the referenced canonical data version and this PR will be ready to merge

* update the version of referenced canonical-data.json
* make test-data of new test same as that in 
   referenced canonical-data.json
@DocFogel
Copy link
Copy Markdown
Contributor Author

Reference updated. I also harmonized the test data in the new test to perfectly match that of the canonical data specification.

@cmccandless cmccandless merged commit a5f9705 into exercism:master Jul 27, 2018
@cmccandless
Copy link
Copy Markdown
Contributor

Merged; thanks for working on this!

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