Skip to content
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

RFC Multiline logging #4485

Closed
wants to merge 1 commit into from
Closed

Conversation

twmr
Copy link
Contributor

@twmr twmr commented Nov 30, 2018

This adds support for aligned multi-line log messages:

image

What this change is doing is inserting white-space to all lines except the first of the "message" part of the log msg.

@codecov
Copy link

codecov bot commented Nov 30, 2018

Codecov Report

Merging #4485 into features will decrease coverage by 0.03%.
The diff coverage is 56.25%.

Impacted file tree graph

@@             Coverage Diff              @@
##           features    #4485      +/-   ##
============================================
- Coverage     95.81%   95.77%   -0.04%     
============================================
  Files           111      111              
  Lines         25038    25063      +25     
  Branches       2463     2468       +5     
============================================
+ Hits          23989    24003      +14     
- Misses          738      746       +8     
- Partials        311      314       +3
Flag Coverage Δ
#docs 29.3% <12.5%> (+0.06%) ⬆️
#doctesting 29.3% <12.5%> (+0.06%) ⬆️
#linting 29.3% <12.5%> (+0.06%) ⬆️
#linux 95.6% <56.25%> (-0.04%) ⬇️
#nobyte 92.51% <53.12%> (-0.05%) ⬇️
#numpy 93.23% <53.12%> (-0.05%) ⬇️
#pexpect 41.6% <12.5%> (-0.04%) ⬇️
#py27 93.84% <53.12%> (-0.03%) ⬇️
#py34 92.01% <43.75%> (+0.01%) ⬆️
#py35 92.03% <43.75%> (+0.01%) ⬆️
#py36 92.05% <43.75%> (+0.01%) ⬆️
#py37 93.96% <43.75%> (-0.04%) ⬇️
#trial 93.23% <53.12%> (-0.05%) ⬇️
#windows 94.02% <56.25%> (-0.07%) ⬇️
#xdist 93.88% <56.25%> (-0.06%) ⬇️
Impacted Files Coverage Δ
src/_pytest/logging.py 91.77% <56.25%> (-4.29%) ⬇️
src/_pytest/cacheprovider.py 96.15% <0%> (-0.97%) ⬇️
src/_pytest/terminal.py 92.95% <0%> (+0.83%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e0ba1cb...d982dda. Read the comment docs.

@codecov
Copy link

codecov bot commented Nov 30, 2018

Codecov Report

Merging #4485 into features will increase coverage by 0.02%.
The diff coverage is 56.25%.

Impacted file tree graph

@@             Coverage Diff              @@
##           features    #4485      +/-   ##
============================================
+ Coverage     95.74%   95.77%   +0.02%     
============================================
  Files           111      111              
  Lines         24951    25063     +112     
  Branches       2459     2468       +9     
============================================
+ Hits          23889    24003     +114     
+ Misses          748      746       -2     
  Partials        314      314
Flag Coverage Δ
#docs 29.3% <12.5%> (+0.03%) ⬆️
#doctesting 29.3% <12.5%> (+0.03%) ⬆️
#linting 29.3% <12.5%> (+0.03%) ⬆️
#linux 95.6% <56.25%> (+0.02%) ⬆️
#nobyte 92.51% <53.12%> (+0.08%) ⬆️
#numpy 93.23% <53.12%> (+0.01%) ⬆️
#pexpect 41.6% <12.5%> (-0.16%) ⬇️
#py27 93.84% <53.12%> (+0.04%) ⬆️
#py34 92.01% <43.75%> (+0.16%) ⬆️
#py35 92.03% <43.75%> (+0.16%) ⬆️
#py36 92.05% <43.75%> (+0.15%) ⬆️
#py37 93.96% <43.75%> (+0.03%) ⬆️
#trial 93.23% <53.12%> (+0.01%) ⬆️
#windows 94.02% <56.25%> (+0.05%) ⬆️
#xdist 93.88% <56.25%> (+0.05%) ⬆️
Impacted Files Coverage Δ
src/_pytest/logging.py 91.77% <56.25%> (-4.29%) ⬇️
src/_pytest/junitxml.py 93.37% <0%> (-1.03%) ⬇️
src/_pytest/debugging.py 81.25% <0%> (-0.51%) ⬇️
testing/python/metafunc.py 95.35% <0%> (-0.42%) ⬇️
src/_pytest/capture.py 93.42% <0%> (-0.25%) ⬇️
testing/python/raises.py 94.07% <0%> (-0.22%) ⬇️
testing/python/collect.py 99.15% <0%> (-0.22%) ⬇️
testing/test_warnings.py 98.64% <0%> (-0.17%) ⬇️
testing/test_parseopt.py 94.71% <0%> (-0.09%) ⬇️
src/_pytest/config/argparsing.py 88.15% <0%> (-0.06%) ⬇️
... and 23 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e63c7a1...4c846c8. Read the comment docs.

@blueyed
Copy link
Contributor

blueyed commented Dec 1, 2018

Interesting.
I wonder though why not filename:lineno is used (to make it more condense), but that appears to not have changed here, or has it?

@blueyed
Copy link
Contributor

blueyed commented Dec 1, 2018

Also just having "E", "W", "I" etc might be worthwile - think about smaller screen, e.g. 80 chars.

@twmr
Copy link
Contributor Author

twmr commented Dec 1, 2018

I like the idea about the short levelnames. However, colored short levelnames are currently not support by pytest, because the regex in the logging formatter does not support the levelname fmt '%(levelname).1s'.

At work I use a custom fmt string because I don't like the default one.

--log-cli-format='%(asctime)s.%(msecs)03d %(levelname).1s %(message)s'

Note the above one only looks good with a patched (support for single letter levelnames) pytest.

Copy link
Contributor

@s0undt3ch s0undt3ch left a comment

Choose a reason for hiding this comment

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

If this lands, can it be optional?

@twmr
Copy link
Contributor Author

twmr commented Jan 12, 2019

I think we should rather make the current integration of the log formatter a bit more extensible. What I have in mind is that the default formatter should be easily replaceable by a custom log formatter. Ideally we could create a collection of predefined log formatters in a new pytest package that can then be selected by name.

@s0undt3ch
Copy link
Contributor

Even better then :)

@nicoddemus
Copy link
Member

Hi @Thisch,

It has been a long time since it has last seen activity, plus we have made sweeping changes on master to drop Python 2.7 and 3.4 support, so this PR has some conflicts which require attention.

In order to clear up our queue and let us focus on the active PRs, I'm closing this PR for now.

Please don't consider this a rejection of your PR, we just want to get this out of sight until you have the time to tackle this again. If you get around to work on this in the future, please don't hesitate in re-opening this!

Thanks for your work, the team definitely appreciates it!

@nicoddemus nicoddemus closed this Jun 6, 2019
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.

4 participants