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

Anagram: relax test requirement for ordered elements #1828

Merged
merged 4 commits into from
Jun 19, 2019
Merged

Anagram: relax test requirement for ordered elements #1828

merged 4 commits into from
Jun 19, 2019

Conversation

roadfoodr
Copy link
Contributor

@roadfoodr roadfoodr commented Jun 19, 2019

See issue #1826.

  • Tested against reference solution and own solution.
  • Tested full track-level test suite.

Note: this implementation uses assertCountEqual(), first available in Python 3.2.

"Update tests to v1.4.1" references: exercism/problem-specifications#1534

@roadfoodr roadfoodr requested a review from a team as a code owner June 19, 2019 04:52
@yawpitch
Copy link
Contributor

yawpitch commented Jun 19, 2019

Thanks for working towards improving the tests, I agree there’s no reason to worry about the sorted order of the results, however we can’t drop Python 2 comparability completely just yet.

Could you please insert this at the top of the file, after the imports:

# Python 2/3 compatibility
if not hasattr(unittest.TestCase, 'assertCountEqual'):
    unittest.TestCase.assertCountEqual = unittest.TestCase.assertItemsEqual

We’ve used it in other exercises to the same end.

@roadfoodr
Copy link
Contributor Author

Done, thanks. (I was wondering about compatibility since README.md describes the use of assertIs(), also new in Python 3.)

@yawpitch
Copy link
Contributor

yawpitch commented Jun 19, 2019

The TestCase.assertIs method was added to 2.7 some time ago, so no compatibility issue there.

@cmccandless I’d say this one is ready to (squash and) merge.

@cmccandless cmccandless merged commit 0ea1041 into exercism:master Jun 19, 2019
@cmccandless
Copy link
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.

3 participants