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: adding a new scalene test for two smaller sides equal #1894

Merged
merged 8 commits into from
Aug 7, 2019
Merged

triangle: adding a new scalene test for two smaller sides equal #1894

merged 8 commits into from
Aug 7, 2019

Conversation

YuriyCherniy
Copy link
Contributor

Adding a new test for scalene(sides) function and renaming another test to match the new test. Current tests for scalene(sides) function can be hacked by this code:

def scalene(sides):
    sides.sort()
    if sum(sides[:2]) < sides[2] or sides[1] == sides[2]:
        return False
    return True

while the new test not.

@YuriyCherniy YuriyCherniy requested a review from a team as a code owner August 7, 2019 07:53
@YuriyCherniy YuriyCherniy changed the title Adding a new test and renaming another test. Triangle: adding a new test template and renaming another test. Aug 7, 2019
Copy link
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.

Hi! Thanks for the PR!

Most test cases in the Python track are derived from the problem-specifications repository. You've identified a gap in the tests, so normally I would recommend adding the test to the canonical data first before it is added here. However, since that repository is currently frozen, we can add this test as a track-specific test for now.

Please address the following, and we'll be good to go:

  • Revert name change on the existing test; the name comes from the canonical data, and we try to follow that when reasonable
  • Rename your new test to something descriptive of why that test is different than two_sides_are_equal. I've suggested a change below; if you have something better you don't need to use my suggestion exactly

I would also recommend creating an issue in the [problem specifications] repository so that when the freeze is over, this test can be added there.

exercises/triangle/triangle_test.py Outdated Show resolved Hide resolved
exercises/triangle/triangle_test.py Outdated Show resolved Hide resolved
YuriyCherniy and others added 3 commits August 7, 2019 20:14
add track-specific test

Co-Authored-By: Corey McCandless <cmccandless@users.noreply.github.com>
Co-Authored-By: Corey McCandless <cmccandless@users.noreply.github.com>
@cmccandless cmccandless changed the title Triangle: adding a new test template and renaming another test. Triangle: adding a new test Aug 7, 2019
@cmccandless cmccandless changed the title Triangle: adding a new test triangle: adding a new scalene test for two smaller sides equal Aug 7, 2019
@cmccandless cmccandless merged commit f811fc0 into exercism:master Aug 7, 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.

None yet

2 participants