Skip to content

Console progress output #2858

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

Merged

Conversation

nicoddemus
Copy link
Member

@nicoddemus nicoddemus commented Oct 21, 2017

Still needs tests, docs and some cleanup, I'm opening this to gather some feedback.

This works in "normal" mode (including xdist), still needs work with -v.

Please see comments below.

@@ -155,13 +160,37 @@ def _tw(self):
stacklevel=2)
return self.writer

@staticmethod
def _new_tw_write(msg, **kw):
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course this is hackish. 😉

I believe the correct thing would be to add this functionality to py.io.TerminalWriter instead, what do you guys think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

before i tried to kill TerminalWriter altogether - it deserves a general removal as its quite bad as an api and people regularly try to obtain it in a hackish was inside of pytest

@@ -320,7 +375,7 @@ def report_collect(self, final=False):
if skipped:
line += " / %d skipped" % skipped
if self.isatty:
self.rewrite(line, bold=True, erase=True)
self.writer.reline(line, bold=True)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This avoids the flickering while collecting, not really related.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) to 92.698% when pulling 1e95642 on nicoddemus:console-progress-2657 into 083084f on pytest-dev:features.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) to 92.698% when pulling 1e95642 on nicoddemus:console-progress-2657 into 083084f on pytest-dev:features.


self.stats = {}
self.startdir = py.path.local()
if file is None:
file = sys.stdout
self._writer = _pytest.config.create_terminal_writer(config, file)
self._screen_width = self.writer.fullwidth
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to cache this otherwise I noticed a good 20% performance penalty. We probably need to fix this in py somehow, which we have tried to do in the past.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 92.73% when pulling d01e871 on nicoddemus:console-progress-2657 into 685387a on pytest-dev:features.

setup.py Outdated
@@ -45,7 +45,7 @@ def has_environment_marker_support():
def main():
extras_require = {}
install_requires = [
'py>=1.4.33',
'py>=1.5.0',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please rebase to avoid this bit creating issues

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely, I'm still working on this.

@nicoddemus nicoddemus force-pushed the console-progress-2657 branch from 4658781 to 456871c Compare November 18, 2017 13:26
@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 92.739% when pulling 456871c on nicoddemus:console-progress-2657 into ca1f4bc on pytest-dev:features.

@nicoddemus
Copy link
Member Author

Also changed the color to CYAN. Some screenshots:
console-progress

console-progress-2

@nicoddemus
Copy link
Member Author

I will also add an ini option (console_output_style=classic/progress) so this can be disabled if the user wants to. I want to add this option in case this somehow causes problems in some edge cases.

@nicoddemus
Copy link
Member Author

Still needs to update the docs and CHANGELOG. I will get to it on Monday.

@nicoddemus nicoddemus force-pushed the console-progress-2657 branch from 5a2d55e to 2455f86 Compare November 21, 2017 23:06
@coveralls
Copy link

Coverage Status

Coverage increased (+0.003%) to 92.709% when pulling 2455f86 on nicoddemus:console-progress-2657 into ca1f4bc on pytest-dev:features.

@nicoddemus nicoddemus changed the title [WIP] Console progress output Console progress output Nov 21, 2017
@nicoddemus
Copy link
Member Author

Cleaned up commits and added what was missing, so this is now ready for review as far as I'm concerned. 😁

@nicoddemus nicoddemus force-pushed the console-progress-2657 branch from 01391b9 to f05333a Compare November 22, 2017 02:00
@coveralls
Copy link

Coverage Status

Coverage increased (+0.003%) to 92.709% when pulling f05333a on nicoddemus:console-progress-2657 into ca1f4bc on pytest-dev:features.

Copy link
Member

@The-Compiler The-Compiler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't really reviewed the logic clearly - I commented on what caught my eye at 7am 😉

"""Check lines exist in the output.

The argument is a list of lines which have to occur in the
output, in any order. Each line can contain glob whildcards.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whildcards -> wildcards - but it looks like this actually takes a regex pattern, and not globs?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done, thanks

self.currentfspath = None
self.reportchars = getreportopt(config)
self.hasmarkup = self.writer.hasmarkup
self.isatty = file.isatty()
self._progress_items_reported = 0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really have no way already to get how many tests have run so far?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not that I know of; usually this is not really tracked by pytest explicitly, each runtestprotocol just triggers the appropriate hooks and each plugin does with that information what it needs.

def test_normal(self, many_tests_file, testdir):
output = testdir.runpytest()
output.stdout.re_match_lines([
r'test_bar.py \.\.\.\.\.\.\.\.\.\. \s+ \[ 50%\]',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might want to do \.{10} here and in other tests.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely 👍

@nicoddemus
Copy link
Member Author

@The-Compiler thanks for the review, I believe I have addressed all your comments so far. 👍

@coveralls
Copy link

Coverage Status

Coverage increased (+0.003%) to 92.709% when pulling dc574c6 on nicoddemus:console-progress-2657 into ca1f4bc on pytest-dev:features.

@RonnyPfannschmidt RonnyPfannschmidt merged commit b533c26 into pytest-dev:features Nov 23, 2017
@wimglenn
Copy link
Member

wimglenn commented Aug 25, 2018

Note: this progress output inspired pytest-dev/py#196
If we can get that reliable and performant enough, I would like to change self.writer.chars_on_current_line into self.writer.width_of_current_line. Or maybe width issues will go away, if we come across some better solution when purging py code from pytest.

@nicoddemus nicoddemus deleted the console-progress-2657 branch August 26, 2018 01:21
@nicoddemus
Copy link
Member Author

@wimglenn definitely! Thanks a lot for that implementation no py! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants