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

[WIP] Avoid truncation when longer output, ref. #6267 #6462

Conversation

erheron
Copy link
Contributor

@erheron erheron commented Jan 15, 2020

Fixes #6267
Surely, because it's user-facing change, documentation/tests would need to change too, but now it's rather a proposal to mentioned issue.

@erheron erheron changed the title Avoid truncation when longer output 6267 [WIP] Avoid truncation when longer output, ref. #6267 Jan 18, 2020
@blueyed
Copy link
Contributor

blueyed commented Feb 12, 2020

As for a test I've noticed a change with https://github.com/pytest-dev/pytest/pull/6702/files#diff-14ad89cac128f1dd3aaa927c656035b3L992-R1025 in that regard.
(I was a bit surprised for the explanation to also count towards the lines, but it makes a bit of sense - but is likely more due to there being no concept of "header notes" and "diff output/details")
(I've not checked #6267 yet)

@blueyed blueyed added topic: rewrite related to the assertion rewrite mechanism type: enhancement new feature or API change, should be merged into features branch labels Feb 12, 2020
@bluetech

This comment has been minimized.

Copy link
Contributor

@blueyed blueyed left a comment

Choose a reason for hiding this comment

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

Thanks for addressing this. I've read #6267, and agree that it can make it more verbose etc.
Will comment there.

src/_pytest/assertion/truncate.py Outdated Show resolved Hide resolved
else:
msg += " ({} lines hidden".format(truncated_line_count)
if last_truncated:
msg += ",1 truncated)"
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like a missing space (after comma)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the space was missing. I've commited 2 changes, haven't squashed yet

@bluetech bluetech changed the base branch from features to master February 16, 2020 15:17
@bluetech
Copy link
Member

We've recently stopped using the features branch

(I've changed the base on my own now).

@nicoddemus
Copy link
Member

Hi @erheron,

First of all we would like to thank you for your time and effort on working on this, the pytest team deeply appreciates it.

We noticed it has been awhile since you have updated this PR, however. pytest is a high activity project, with many issues/PRs being opened daily, so it is hard for us maintainers to track which PRs are ready for merging, for review, or need more attention.

So for those reasons we think it is best to close the PR for now, but with the only intention to cleanup our queue, it is by no means a rejection of your changes. We still encourage you to re-open this PR (it is just a click of a button away) when you are ready to get back to it.

Again we appreciate your time for working on this, and hope you might get back to this at a later time!

Cheers!

@nicoddemus nicoddemus closed this Jun 9, 2020
@erheron
Copy link
Contributor Author

erheron commented Jun 9, 2020

Hi @nicoddemus. While working on this issue I got a feeling, that this is kind of minor change, and I'm not sure I want to re-open this. However, if someone else decides to work on this issue - please, feel free to use my code, if it would help You in any way!

@nicoddemus
Copy link
Member

Sure @erheron! No worries, thanks a lot anyway, we appreciate it. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: rewrite related to the assertion rewrite mechanism type: enhancement new feature or API change, should be merged into features branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Avoid truncation when it makes the output longer
4 participants