Skip to content

Improve display of continuation lines with multiline errors#1807

Merged
nicoddemus merged 1 commit intopytest-dev:masterfrom
blueyed:improve-multiline-error-format
Aug 15, 2016
Merged

Improve display of continuation lines with multiline errors#1807
nicoddemus merged 1 commit intopytest-dev:masterfrom
blueyed:improve-multiline-error-format

Conversation

@blueyed
Copy link
Copy Markdown
Contributor

@blueyed blueyed commented Aug 11, 2016

Fixes #717.
Follow-up to #1762.

@coveralls
Copy link
Copy Markdown

coveralls commented Aug 11, 2016

Coverage Status

Coverage decreased (-0.0009%) to 92.324% when pulling bf1313a on blueyed:improve-multiline-error-format into 34925a3 on pytest-dev:master.

@nicoddemus
Copy link
Copy Markdown
Member

Hey @blueyed. thanks!

Looks good to me, perhaps though we should add a CHANGELOG entry for that as well.

Comment thread _pytest/python.py Outdated
prefix = ' '
tw.line(prefix + line.strip(), red=True)
if lines:
tw.line('E ' + lines[0].strip(), red=True)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just noticed that we have this in py._code.code.py:

class FormattedExcinfo(object):
    """ presenting information about failing Functions and Generators. """
    # for traceback entries
    flow_marker = ">"
    fail_marker = "E"

Could please change the code use those constants instead?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done. Not sure though if that is really good like that.
After all it should be some more generic method probably for this kind of messages?

@tomviner
Copy link
Copy Markdown
Contributor

LGTM 👍

@blueyed blueyed force-pushed the improve-multiline-error-format branch 2 times, most recently from 29ec412 to 998f9a5 Compare August 14, 2016 20:04
@blueyed blueyed force-pushed the improve-multiline-error-format branch from 998f9a5 to c163cc7 Compare August 14, 2016 20:34
@nicoddemus nicoddemus merged commit d58a8e3 into pytest-dev:master Aug 15, 2016
@nicoddemus
Copy link
Copy Markdown
Member

Thanks @blueyed and @tomviner!

@blueyed blueyed deleted the improve-multiline-error-format branch August 15, 2016 22:11
@blueyed
Copy link
Copy Markdown
Contributor Author

blueyed commented Aug 16, 2016

FWIW: #1762 should have been made against the features branch probably. There are huge conflicts now, since it was split in 8c49561.

@blueyed
Copy link
Copy Markdown
Contributor Author

blueyed commented Aug 16, 2016

@nicoddemus
In case you don't know about rerere already: https://git-scm.com/blog/2010/03/08/rerere.html (outdated, better check the man page probably).

@nicoddemus
Copy link
Copy Markdown
Member

Thanks for the heads up @blueyed!

@tomviner
Copy link
Copy Markdown
Contributor

@blueyed said:

FWIW: #1762 should have been made against the features branch probably.

That's probably by bad, I was advising new contributors at the Europython sprint 😬

Looking again at #717 (which #1762 attempts to fix) it's more of a "bring into line" (with other traceback output), than strictly a "bugfix". So I guess default to features branch for these kind of tickets?

@nicoddemus
Copy link
Copy Markdown
Member

@tomviner not sure, that's such a small change in output, I think it is OK for it to go to master. Others may have other opinions though. 😁

@The-Compiler
Copy link
Copy Markdown
Member

I'm still wondering what crazy output parsing people do (like for IDEs) though, and how much we break even with small output changes.

@tomviner
Copy link
Copy Markdown
Contributor

@The-Compiler I'm sure they do all sorts. But if we have faith in the longevity of pytest, perhaps being more consistent now will help future crazy output parsers ;-)

@The-Compiler
Copy link
Copy Markdown
Member

@tomviner Sure, but I think that's a good reason to have any changes to the output in features 😉

@tomviner
Copy link
Copy Markdown
Contributor

@The-Compiler see what you mean. So really, for patches that alter the pytest interface, only changes that were obviously broken before should be aimed at master.

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