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
Display message from reprcrash in short test summary #5013
Merged
Merged
Changes from 3 commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
3d0ecd0
Display message from reprcrash in short test summary
blueyed 37ecca3
factor out _get_line_with_reprcrash_message
blueyed 1597044
change separator to hyphen
blueyed f599172
test with 😄 in message
blueyed df377b5
use wcwidth
blueyed 2ebb69b
py2 fixes
blueyed 2b1ae8a
__tracebackhide__ for check
blueyed 14d3d91
Remove partial unicode characters from summary messages in Python 2
nicoddemus df1d110
Merge remote-tracking branch 'origin/features' into short-summary-mes…
blueyed c3178a1
move test
blueyed 0e8a8f9
Add encoding header to test_terminal.py
nicoddemus 32a5e80
Add encoding: header and fix rep mock in test_line_with_reprcrash on …
nicoddemus c04767f
Use msg.rstrip() as suggested in review
nicoddemus f339147
Add CHANGELOG entry about depending on wcwidth
nicoddemus File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Messages from crash reports are displayed within test summaries now, truncated to the terminal width. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh just realized this: is
msg
unicode-safe in Python 2? because we will be joining astr/bytes
with a potential unicode message, so there's a good chance this might blow upThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would appreciate a failing example.. do you mean we should prefix
sep
etc also withu
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm actually no, this is safe: if
msg
is unicode, Python 2 will try to elevatesep
to unicode using ascii (which is fine), not the other way around. Sorry for the brain fart. 👍There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you want to play it safe and ensure we don't regress though, you can change the message to use a smiling face emoji instead (😄), but is up to you. 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added.. but it shows that we're not handling terminal cells there really - i.e. it would wrap due to this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh. Hmm not even sure what the solution would be, TBH.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added a commit using wcwidth.. not sure if it is worth the extra dependency, but it worked well.. ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think adding
wcwidth
is fine: small, license is OK, seems to be maintained