Skip to content

Color percentage indicator #6107

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

MarcoGorelli
Copy link
Contributor

@MarcoGorelli MarcoGorelli commented Oct 30, 2019

Fixes #6097.

@MarcoGorelli
Copy link
Contributor Author

I'm aware I'm supposed to write tests before writing code, I just haven't (yet) figured out how to test this, sorry

@blueyed
Copy link
Contributor

blueyed commented Oct 30, 2019

No worries, if you're lucky you can adjust an existing one.
(I've edited the original comment, to add a reference, and remove the boilerplate)
Thanks for working on this! :)

@blueyed
Copy link
Contributor

blueyed commented Oct 31, 2019

While working on this we should keep in mind to have it usable also for the "SKIPPED", "FAILED", "XFAIL", etc prefixes in the short test summary later.

@blueyed
Copy link
Contributor

blueyed commented Nov 1, 2019

CI passes (except for the flaky hypothesis issue; restarted to get coverage at least maybe), so you need a new test.
I suggest using one based on monkeypatch.setenv("PY_COLORS", "1"), so that color output is forced.

@MarcoGorelli
Copy link
Contributor Author

Thanks, I was wondering about the hypothesis issue.

OK, I'll give that a go and will let you know

@MarcoGorelli MarcoGorelli force-pushed the color-percentage-indicator branch from 464eef9 to 009c0b6 Compare November 3, 2019 16:18
blueyed pushed a commit that referenced this pull request Nov 3, 2019
"*= ERRORS =*",
"*_ ERROR collecting test_collecterror.py _*",
"E SyntaxError: *",
"*= short test summary info =*",
"ERROR test_collecterror.py",
"*! Interrupted: 1 errors during collection !*",
"*! Interrupted: 1 error during collection !*",
"*= 1 error in *",
]
)

This comment was marked as resolved.

@MarcoGorelli MarcoGorelli changed the title [WIP] Color percentage indicator Color percentage indicator Nov 3, 2019
@MarcoGorelli
Copy link
Contributor Author

I've added tests with monkeypatch.setenv("PY_COLORS", "1"), is this what you had in mind?

@blueyed
Copy link
Contributor

blueyed commented Nov 3, 2019

I've added tests with monkeypatch.setenv("PY_COLORS", "1"), is this what you had in mind?

Yes. Although I think we should them more readable (via format strings for color escapes), so that it would use "{red}" there then.
Also I am not sure if the base/default test is really needed, and if it would be faster to have them all in a single run (have not looked at what the "many files" fixture really does).
Looking forward to testing this out later..!

@MarcoGorelli
Copy link
Contributor Author

Ok, I've removed two of the tests and have kept the one that has all three colors in it (green, red, yellow). I've also added constants to make the tests more readable.

Just realised that pytest-CI checks for Python 3.5 to work too, so no f-strings....I'll change this later today

@blueyed
Copy link
Contributor

blueyed commented Nov 4, 2019

Nice! You also need a changelog then (see changelog/README.rst).

@MarcoGorelli
Copy link
Contributor Author

Thanks! Changelog added

@blueyed
Copy link
Contributor

blueyed commented Nov 4, 2019

Thanks, will review it properly later - you could however squash and force-push it already (sorry, I'd like we could just do it here (#4361)).

indicate current outcome/status with color of percentage indicator

Fix type annotation, refactor _write_progress_information_filling_space

Keep code in _get_main_color as similar as possible to how it was before

Write test

Make black-compliant

Fix error in newly introduced test_collecterror

Make tests more readable by using constants and f-strings

Remove accidentally added monkeypatch

Make Python 3.5-compatible, add changelog entry

Add newline at the end of changelog file
@MarcoGorelli MarcoGorelli force-pushed the color-percentage-indicator branch from e8b7a11 to 0d79061 Compare November 4, 2019 19:59
@blueyed
Copy link
Contributor

blueyed commented Nov 5, 2019

Awesome!

@blueyed blueyed merged commit 00f6749 into pytest-dev:features Nov 5, 2019
@MarcoGorelli MarcoGorelli deleted the color-percentage-indicator branch November 6, 2019 09:42
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.

2 participants