Opened 4 years ago
Closed 4 years ago
#32540 closed Cleanup/optimization (fixed)
Only do "top level" detection in DiscoverRunner.build_suite() when needed
Reported by: | Chris Jerdonek | Owned by: | Chris Jerdonek |
---|---|---|---|
Component: | Testing framework | Version: | dev |
Severity: | Normal | Keywords: | DiscoverRunner, build_suite, discovery |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
Currently, DiscoverRunner.build_suite()
is a bit confusing to read because it does "top level" detection for test discovery even when discovery won't be taking place.
My suggestion is to move the top-level detection logic (and large code comment) into a find_top_level(top_level)
function that is placed immediately before where is_discoverable()
is defined. Then, only call find_top_level()
when needed, namely right before where self.test_loader.discover() is called.
Similarly, the kwargs = discover_kwargs.copy()
line can also go into that if block right before that line, because that's the only code path where kwargs
is actually used. Without this change, it's hard for one to notice that the kwargs
are in fact only used for that one line.
Together, this will cut down on the size of build_suite()
quite a bit and make it easier to understand.
Change History (7)
comment:1 by , 4 years ago
Description: | modified (diff) |
---|
comment:2 by , 4 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:3 by , 4 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:4 by , 4 years ago
Has patch: | set |
---|
comment:5 by , 4 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
PR: https://github.com/django/django/pull/14120