Skip to content

Add colors to summary #9875

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
merged 3 commits into from
May 12, 2022
Merged

Conversation

nicoddemus
Copy link
Member

@nicoddemus nicoddemus commented Apr 21, 2022

[0] Original output, for comparison:

image

[1] Coloring status words:

image

Note that here I also moved the XFAIL reason to the same line, similar to the summary we have for failures. I think this is more pleasing, and was implemented when we didn't have the failure summary on each line, and not updated as an oversight.

[2] Test names in bold:

image

Fix #9873


I personally like [2], but posting [1] in case people prefer it.


The fact that SKIPS are formatted differently and also folded together bothers me a bit, opened #9876 for discussion.

@nicoddemus nicoddemus force-pushed the summary-colors-9873 branch from 7efcfde to bcaff45 Compare April 21, 2022 11:42
@nicoddemus nicoddemus requested review from ssbarnea and removed request for ssbarnea April 21, 2022 12:39
@nicoddemus
Copy link
Member Author

cc @ssbarnea

@nicoddemus nicoddemus marked this pull request as ready for review April 21, 2022 12:40
@ssbarnea
Copy link
Member

@nicoddemus LOL! I did not expect anyone to react so quickly on my suggestion. I was waiting for some feedback, imagining that I will have to make it myself. Not a complaint.

I love it! And is it obvious from the photo how much easier is to read.

My vote is for first option, I personally dislike bold on terminal because it makes the shape of the letters harder to read, some terminals do not even support bold. Using default text color is good here.

If you use vscode, you might also be interested about microsoft/vscode#147769 -- i was bit engaged with microsoft on making the terminal links easier to click as sometimes it fails to open the files.

@ssbarnea ssbarnea added the type: enhancement new feature or API change, should be merged into features branch label Apr 21, 2022
@nicoddemus
Copy link
Member Author

nicoddemus commented Apr 21, 2022

LOL! I did not expect anyone to react so quickly on my suggestion. I was waiting for some feedback, imagining that I will have to make it myself. Not a complaint.

Hehehe got some time to work on pytest and seemed like a nice improvement to tackle in a couple of hours.

My vote is for first option, I personally dislike bold on terminal because it makes the shape of the letters harder to read

We can use another color for the test names too.

@bluetech
Copy link
Member

bluetech commented May 9, 2022

I think this is nice, 👍.

BTW, here is the output of a JavaScript test runner called jest:

image

jest1

Two relevant things:

  1. In some cases it uses green/yellow/red background instead of letter color.
  2. It hyperlinks the test names to the file using file:// URLs. This is a feature that a growing number of terminal emulators support. It adds the thin dashed underline.

@nicoddemus
Copy link
Member Author

In some cases it uses green/yellow/red background instead of letter color.

Yeah we can probably use that more. 👍

@nicoddemus nicoddemus force-pushed the summary-colors-9873 branch from 2d299c2 to 3e07127 Compare May 10, 2022 20:38
@nicoddemus
Copy link
Member Author

@bluetech how is your opinion on the use of bold for test names? I kind like it, but @ssbarnea said it makes the text harder to read in some terminal configurations.

@bluetech
Copy link
Member

I somewhat like the bold, and jest uses it too. I'll leave it to you to decide :)

@nicoddemus
Copy link
Member Author

Well since @ssbarnea didn't cast a hard no on it, being a personal preference, let's keep the bold then, because I also like it (and other runners also use bold as @bluetech said, so that's another argument in favor). 👍

@nicoddemus nicoddemus merged commit 69fb79e into pytest-dev:main May 12, 2022
@nicoddemus nicoddemus deleted the summary-colors-9873 branch May 12, 2022 12:55
@ssbarnea
Copy link
Member

Some things need to hit the market. If it proves less than ideal, we will see others complaining. Anyway thanks for the improvements!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

Short test summary info should use colors
3 participants