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

Remove redundant line in test_decimal#141281

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

Open
efimov-mikhail wants to merge1 commit intopython:main
base:main
Choose a base branch
Loading
fromefimov-mikhail:test_decimal_redundant_line

Conversation

@efimov-mikhail
Copy link
Member

No description provided.

@bedevere-appbedevere-appbot added awaiting review testsTests in the Lib/test dir labelsNov 9, 2025
@picnixz
Copy link
Member

Usually, we don't accept PRs that are only cosmetic, especially in tests. So I won't approve this one, sorry. We can clean it up if we were to rewritetest_decimal to remove duplicated code. For instance, all test methods do:

getcontext = self.decimal.getcontextlocalcontext = self.decimal.localcontext

To save a few keystrokes. In this case, why not having a method for that on the test class itself.

Now, I would be more interested in reorganizing this test file because it's 6 THOUSANDS lines...

skirpichev reacted with thumbs up emoji

@efimov-mikhail
Copy link
MemberAuthor

efimov-mikhail commentedNov 9, 2025
edited
Loading

Usually, we don't accept PRs that are only cosmetic, especially in tests. So I won't approve this one, sorry.

Ok, no problems.

Now, I would be more interested in reorganizing this test file because it's 6 THOUSANDS lines...

I agree, that's a lot of code.

I've tried to make this reorganization.
But there's a problem with command line arguments of this test.
If we splitLib/test/test_decimal.py and make package directoryLib/test/test_decimal, then we should rununittest.main in__main__.py. But there's used kinda hack, with setting global value ofARITH beforeunittest.main. And this doesn't work in case of different files.

Maybe you have some idea on that topic?
Do we really want to keep--debug and--skip flags here or we can remove them?

@picnixz
Copy link
Member

picnixz commentedNov 9, 2025
edited by efimov-mikhail
Loading

then we should rununittest.main in__main__.py. But there's used kinda hack, with setting global value ofARITH

Ithink it's fine. We already do this fortest_asyncio.

@efimov-mikhail
Copy link
MemberAuthor

We already do this fortest_asyncio.

Could you please guide me a little?
Link to some PR or commit would be great.

@picnixz
Copy link
Member

picnixz commentedNov 9, 2025
edited by efimov-mikhail
Loading

Could you please guide me a little?

FTR, this was just for the__main__.py question, not about the CLI itself. InLib/test/test_asyncio you can see that there is a__main__.py file that contains

from .importload_testsimportunittestunittest.main()

and in__init__.py there is a function that configure the tests. Instead of global variables, we can have a base class with class variables that would then be configured to be skipped or not depending on the CLI args.

I don't really know whether it's possible to do it though. Maybe it's not! However, I think we should discuss this either on Discord or create an issue for that (at least this PR shouldn't be used for discussion).

efimov-mikhail reacted with thumbs up emoji

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

Reviewers

@picnixzpicnixzAwaiting requested review from picnixz

Assignees

No one assigned

Labels

awaiting reviewneeds backport to 3.13bugs and security fixesneeds backport to 3.14bugs and security fixesskip issueskip newstestsTests in the Lib/test dir

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@efimov-mikhail@picnixz

[8]ページ先頭

©2009-2025 Movatter.jp