Skip to content

Conversation

@efimov-mikhail
Copy link
Member

No description provided.

@bedevere-app bedevere-app bot added awaiting review tests Tests in the Lib/test dir labels Nov 9, 2025
@picnixz
Copy link
Member

picnixz commented Nov 9, 2025

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 rewrite test_decimal to remove duplicated code. For instance, all test methods do:

getcontext = self.decimal.getcontext
localcontext = 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...

@efimov-mikhail
Copy link
Member Author

efimov-mikhail commented Nov 9, 2025

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 split Lib/test/test_decimal.py and make package directory Lib/test/test_decimal, then we should run unittest.main in __main__.py. But there's used kinda hack, with setting global value of ARITH before unittest.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 commented Nov 9, 2025

then we should run unittest.main in __main__.py. But there's used kinda hack, with setting global value of ARITH

I think it's fine. We already do this for test_asyncio.

@efimov-mikhail
Copy link
Member Author

We already do this for test_asyncio.

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

@picnixz
Copy link
Member

picnixz commented Nov 9, 2025

Could you please guide me a little?

FTR, this was just for the __main__.py question, not about the CLI itself. In Lib/test/test_asyncio you can see that there is a __main__.py file that contains

from . import load_tests
import unittest

unittest.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).

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

Labels

awaiting review needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes skip issue skip news tests Tests in the Lib/test dir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants