Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

gh-119189: Add more tests for mixed Fraction arithmetic#119236

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation

serhiy-storchaka
Copy link
Member

@serhiy-storchakaserhiy-storchaka commentedMay 20, 2024
edited by bedevere-appbot
Loading

Copy link
Member

@mdickinsonmdickinson left a comment

Choose a reason for hiding this comment

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

LGTM; two nitpick-level comments.

@serhiy-storchakaserhiy-storchakaforce-pushed thetests-fractions-mixed-arithmetic branch fromdc34aa1 toc1f6081CompareMay 20, 2024 18:37
@serhiy-storchakaserhiy-storchaka merged commitfe67af1 intopython:mainMay 20, 2024
@miss-islington-app
Copy link

Thanks@serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13.
🐍🍒⛏🤖

@serhiy-storchakaserhiy-storchaka deleted the tests-fractions-mixed-arithmetic branchMay 20, 2024 19:34
miss-islington pushed a commit to miss-islington/cpython that referenced this pull requestMay 20, 2024
…GH-119236)(cherry picked from commitfe67af1)Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull requestMay 20, 2024
…GH-119236)(cherry picked from commitfe67af1)Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
@bedevere-app
Copy link

GH-119255 is a backport of this pull request to the3.13 branch.

@bedevere-appbedevere-appbot removed the needs backport to 3.13bugs and security fixes labelMay 20, 2024
@bedevere-app
Copy link

GH-119256 is a backport of this pull request to the3.12 branch.

@bedevere-appbedevere-appbot removed the needs backport to 3.12only security fixes labelMay 20, 2024
serhiy-storchaka added a commit that referenced this pull requestMay 20, 2024
…9236) (GH-119256)(cherry picked from commitfe67af1)Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
serhiy-storchaka added a commit that referenced this pull requestMay 20, 2024
…9236) (GH-119255)(cherry picked from commitfe67af1)Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
@scoder
Copy link
Contributor

I noticed that the newly added tests are written the wrong way around. When a test fails, it says something like

Traceback (most recent call last):  File "test_fractions.py", line …, in testMixedDivision    self.assertTypedEquals(F(3, 2) / Rat(3, 5), Rat(15, 6))  File "test_fractions.py", line …, in assertTypedEquals    self.assertEqual(type(expected), type(actual))AssertionError: <class 'Fraction'> != <class '__main__.Rat'>

Note howexpected andactual are reversed here. This is misleading and it took me a couple of looks to understand that I was simply searching for the wrong kind of bug. The previously existing tests were apparently written the other way round:

        self.assertTypedEquals(F(1, 10), F(1, 10) / 1)  # existing

versus

        self.assertTypedEquals(F(3, 2) / DummyFraction(3, 5), F(5, 2))  # new

scoder added a commit to scoder/quicktions that referenced this pull requestNov 28, 2024
@serhiy-storchaka
Copy link
MemberAuthor

The normal order for assertEqual() is actual, expected. But some old tests are written with other order. We do not touch them to avoid code churn. It seems that old tests here were written in such order, but assertTy po edEquals then reverted the order .

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@mdickinsonmdickinsonmdickinson approved these changes

Assignees
No one assigned
Labels
skip newstestsTests in the Lib/test dir
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

3 participants
@serhiy-storchaka@scoder@mdickinson

[8]ページ先頭

©2009-2025 Movatter.jp