-
-
Notifications
You must be signed in to change notification settings - Fork 30.7k
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
gh-84867: Do not load tests from TestCase
and FunctionTestCase
#100497
Conversation
@csabella since your are the one who reviewed the previous PR, maybe you will be interested in reviewing this one as well? I understand that two years have passed, but nevertheless :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but I have one question.
Do you have some concrete use case for _shouldBeLoaded()
? I do not like introducing yet one private method which can became de-facto a part of the interface.
I think that for now it would be better to inline _shouldBeLoaded()
and remove test test_loadTestsFromModule__TestCase_subclass_custom
. Later we can design more official way to declare TestCase
subclass a "base class" which should not be loaded.
Yes, I had some # Some typical django project's tests:
from django.test import (
LiveServerTestCase,
SimpleTestCase,
TestCase,
TransactionTestCase,
)
class MyTest(TestCase):
def test_sum(self):
self.assertEqual(1 + 1, 2) Now, let try to load these tests: In [1]: from server.apps.main import tests
In [2]: import unittest
In [3]: loader = unittest.TestLoader()
In [4]: loader.loadTestsFromModule(tests)
Out[5]: <unittest.suite.TestSuite tests=[<unittest.suite.TestSuite tests=[]>, <unittest.suite.TestSuite tests=[<server.apps.main.tests.MyTest testMethod=test_sum>]>, <unittest.suite.TestSuite tests=[]>, <unittest.suite.TestSuite tests=[]>, <unittest.suite.TestSuite tests=[]>]> I also know other projects that define But, I think it is fine to inline this for now, so we can later design a better API and refactor these changes to use it instead. |
Misc/NEWS.d/next/Library/2022-12-24-12-50-54.gh-issue-84867.OhaLbU.rst
Outdated
Show resolved
Hide resolved
Let keep this change more narrow. The idea about official method to exclude the class from automatic test loading already was discussed. Try to search in old issues. Although the issue where it was discussed may be closed. |
Co-authored-by: Serhiy Storchaka <[email protected]>
Thanks @sobolevn for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12. |
Thanks @sobolevn for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11. |
…ythonGH-100497) (cherry picked from commit 66d1d7e) Co-authored-by: Nikita Sobolev <[email protected]>
GH-109327 is a backport of this pull request to the 3.12 branch. |
GH-109328 is a backport of this pull request to the 3.11 branch. |
…ythonGH-100497) (cherry picked from commit 66d1d7e) Co-authored-by: Nikita Sobolev <[email protected]>
…GH-100497) (GH-109328) (cherry picked from commit 66d1d7e) Co-authored-by: Nikita Sobolev <[email protected]>
…GH-100497) (#109327) gh-84867: Do not load tests from TestCase and FunctionTestCase (GH-100497) (cherry picked from commit 66d1d7e) Co-authored-by: Nikita Sobolev <[email protected]>
There are several changes:
TestCase
andFunctionTestCase
when loading withloadTestsFromModule
. I've added two tests cases for this. Previously it was like this for importedTestCase
andFunctionTestCase
in a module:Now it is:
[]
loadTestsFromTestCase(unittest.FunctionTestCase)
caused a bug.FunctionTestCase
expects a function, but it was given a'runTest'
string bymap(testCaseClass, testCaseNames)
. And later it caused an exception. This is now also fixed, I've added a test case for it.Inspired by #20237