#32540: Only do "top level" detection in DiscoverRunner.build_suite() when
needed
-------------------------------------+-------------------------------------
Reporter: Chris Jerdonek | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Testing framework | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage:
DiscoverRunner,build_suite,discovery| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Description changed by Chris Jerdonek:
Old description:
> Currently, `DiscoverRunner.build_suite()` is a bit confusing to read
> because it does
> [https://github.com/django/django/blob/551b0f94bf62c81d9ff9d5f7e261bcd2a594a4d1/django/test/runner.py#L577-L599
> "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
> [https://github.com/django/django/blob/551b0f94bf62c81d9ff9d5f7e261bcd2a594a4d1/django/test/runner.py#L603
> 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.
New description:
Currently, `DiscoverRunner.build_suite()` is a bit confusing to read
because it does
[https://github.com/django/django/blob/551b0f94bf62c81d9ff9d5f7e261bcd2a594a4d1/django/test/runner.py#L576-L599
"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
[https://github.com/django/django/blob/551b0f94bf62c81d9ff9d5f7e261bcd2a594a4d1/django/test/runner.py#L603
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.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/32540#comment:1>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/067.a02c3beb59bbd9e8148268765142cb6d%40djangoproject.com.
