-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
#3829 -- Add the ability to show test progress as number of tests completed instead of a percent. #3880
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
#3829 -- Add the ability to show test progress as number of tests completed instead of a percent. #3880
Conversation
Thanks for tackling this @jeffreyrack! The counter text is going over the edge when the number of tests make the progress doesn't fit on a single line, at least for me on Windows:
How does it look on your system? Also with
I think there should be a space between them:
Finally, a little of bike shedding: I think we don't need the space between brackets and the numbers: |
About where to put the docs: we should put the new option in |
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.
Also take a look at _PROGRESS_LENGTH
and where it is used to fix the "past the edge" problem I mentioned. 👍
…st progress for count console output style.
src/_pytest/terminal.py
Outdated
@@ -254,7 +260,10 @@ def _determine_show_progress_info(self): | |||
# do not show progress if we are showing fixture setup/teardown | |||
if self.config.getoption("setupshow"): | |||
return False | |||
return self.config.getini("console_output_style") == "progress" | |||
return ( |
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.
I suggest to use simply:
return self.config.getini("console_output_style") in ("progress", "count")
src/_pytest/terminal.py
Outdated
if self.config.getini("console_output_style") == "count": | ||
num_tests = self._session.testscollected | ||
_PROGRESS_LENGTH = len( | ||
" [ {} / {} ]".format(str(num_tests), str(num_tests)) |
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.
What do you think about using " [{}/{}]"
here?
src/_pytest/terminal.py
Outdated
@@ -404,6 +413,14 @@ def pytest_runtest_logreport(self, report): | |||
self.currentfspath = -2 | |||
|
|||
def pytest_runtest_logfinish(self, nodeid): | |||
if self.config.getini("console_output_style") == "count": | |||
num_tests = self._session.testscollected | |||
_PROGRESS_LENGTH = len( |
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.
Now that this is no longer a constant, this should be lower case to conform to PEP-8: progress_length
src/_pytest/terminal.py
Outdated
counter_format = "{{:{}d}}".format(len(str(collected))) | ||
format_string = " [ {} / {{}} ]".format(counter_format) | ||
return format_string.format(len(progress), collected) | ||
return " [ {} / {} ]".format(collected, collected) |
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.
Ditto here for " [{}/{}]"
Codecov Report
@@ Coverage Diff @@
## features #3880 +/- ##
===========================================
Coverage ? 92.64%
===========================================
Files ? 51
Lines ? 9975
Branches ? 0
===========================================
Hits ? 9241
Misses ? 734
Partials ? 0
Continue to review full report at Codecov.
|
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.
Great work @jeffreyrack, thanks!
Implements #3829 by adding a config value that can be used to show the test progress as the number of completed tests instead of a percentage complete.
Wasn't sure where to add the documentation for this, but am willing to go and add it if somebody can suggest a good spot to mention the new config value in.