Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#32641 closed Cleanup/optimization (fixed)

Log the number of tests found in DiscoverRunner.build_suite()

Reported by: Chris Jerdonek Owned by: Girish Sontakke
Component: Testing framework Version: dev
Severity: Normal Keywords: DiscoverRunner, build_suite
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description (last modified by Chris Jerdonek)

Currently, when running tests with DiscoverRunner, the number of tests is displayed only at the very end of the test run.

However, knowing this number at the beginning of a test run could I think provide an increase in usability. For example, you'd be able to notice right away if a new test you're working on was or wasn't included as expected, based on whether the number is the same or different from the previous run. Similarly, the early feedback would be helpful as a sanity check if you're trying to reduce the number of tests using different command-line options, and you're not sure if your invocation is correct.

Thus, I'd like to suggest that DiscoverRunner display by default the number of tests discovered (and that match any filters) at the earliest point where this is known. This could be done inside DiscoverRunner.build_suite().

The code change could be as simple as:

diff --git a/django/test/runner.py b/django/test/runner.py
index f187107157..c975ed92be 100644
--- a/django/test/runner.py
+++ b/django/test/runner.py
@@ -652,7 +652,8 @@ class DiscoverRunner:
         # _FailedTest objects include things like test modules that couldn't be
         # found or that couldn't be loaded due to syntax errors.
         test_types = (unittest.loader._FailedTest, *self.reorder_by)
-        all_tests = reorder_tests(all_tests, test_types, self.reverse)
+        all_tests = list(reorder_tests(all_tests, test_types, self.reverse))
+        if self.verbosity >= 1:
+            print('Found %d tests' % len(all_tests))
         suite = self.test_suite(all_tests)
 
         if self.parallel > 1:

If there is any concern about adding an additional message, another possibility would be to display the current "Testing against Django installed in ..." message later in the test run, only after the number of tests is known, and include the test count in that message.

Change History (11)

comment:1 by Chris Jerdonek, 4 years ago

Description: modified (diff)

comment:2 by Mariusz Felisiak, 4 years ago

Easy pickings: set
Triage Stage: UnreviewedAccepted

An extra line in the output should be fine.

comment:3 by Girish Sontakke, 4 years ago

Owner: changed from nobody to Girish Sontakke
Status: newassigned

comment:4 by Girish Sontakke, 4 years ago

comment:5 by Mariusz Felisiak, 4 years ago

Has patch: set
Needs tests: set

comment:6 by Mariusz Felisiak, 4 years ago

Needs tests: unset
Triage Stage: AcceptedReady for checkin

comment:7 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

Resolution: fixed
Status: assignedclosed

In a815a6a3:

Fixed #32641 -- Made DiscoverRunner print the number of found tests.

Thanks Chris Jerdonek for reviews.

comment:8 by Chris Jerdonek, 4 years ago

I was trying this out, and it looks great. Thanks, Mariusz and Girish. The only thing is I noticed it can say:

Found 1 tests.

when there is just one test. So while not a big deal, it would be easy to add polish to that one special case. :)

Here is how that's handled in the similar message saying, "System check identified N issues...":
https://github.com/django/django/blob/e3e2276e6fe6fd77e4fbdeeb2a287288d31de3bb/django/core/management/base.py#L460-L463

Last edited 4 years ago by Chris Jerdonek (previous) (diff)

comment:9 by Mariusz Felisiak, 4 years ago

Here is how that's handled in the similar message saying, "System check identified N issues...":

I would use Found 1 test(s).

comment:10 by Chris Jerdonek, 4 years ago

I would use "Found 1 test(s)."

I made a PR for this: https://github.com/django/django/pull/14476

comment:11 by GitHub <noreply@…>, 4 years ago

In 1b4d1675:

Refs #32641 -- Made DiscoverRunner's "Found X tests" message work for finding one test.

This also removes passing level to log() as logging.INFO is the default.

Note: See TracTickets for help on using tickets.
Back to Top