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

Add confval console_output_style option times #13126

Open
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

afcmrp
Copy link

@afcmrp afcmrp commented Jan 11, 2025

Adds an additional confval times to console_output_style.

Design decisions

Duration format

The duration format should be as short, detailed and human readable as possible. It also needs to predictable in terms of character length to know how much space to allocate to the right in the console output.

I landed on this:

  • 00:30:59.1236 -> 30m 59s
  • 00:00:10.1236 -> 10.124s
  • 00:00:09.1236 -> 9.124s
  • 00:00:00.1236 -> 123.6ms
  • 00:00:00.01236 -> 12.36ms

This maintains relatively high resolution for all duration lengths, keeps the string maximum 7 characters long (for durations below 100 hours) and is pleasant to look at.

Verbose vs non-verbose

For the non-verbose case I decided to sum up the durations for all tests per file to keep things as consistent with the other console_output_style's as possible. Additionally, teardown is currently only counted for the non-verbose case.

Example output

Non-verbose

test_bar.py ..........                                                  4.094ms
test_foo.py .....                                                       2.013ms
test_foobar.py .....                                                    1.837ms

Verbose

test_bar.py::test_bar[0] PASSED                                         320.8us
test_bar.py::test_bar[1] PASSED                                         260.9us
test_bar.py::test_bar[2] PASSED                                         236.9us
test_bar.py::test_bar[3] PASSED                                         234.2us
test_bar.py::test_bar[4] PASSED                                         241.9us
test_bar.py::test_bar[5] PASSED                                         230.3us
test_bar.py::test_bar[6] PASSED                                         230.7us
test_bar.py::test_bar[7] PASSED                                         248.6us
test_bar.py::test_bar[8] PASSED                                         245.6us
test_bar.py::test_bar[9] PASSED                                         231.0us
test_foo.py::test_foo[0] PASSED                                         269.7us
test_foo.py::test_foo[1] PASSED                                         250.6us
test_foo.py::test_foo[2] PASSED                                         241.2us
test_foo.py::test_foo[3] PASSED                                         243.0us
test_foo.py::test_foo[4] PASSED                                         242.3us
test_foobar.py::test_foobar[0] PASSED                                   236.1us
test_foobar.py::test_foobar[1] PASSED                                   253.7us
test_foobar.py::test_foobar[2] PASSED                                   264.8us
test_foobar.py::test_foobar[3] PASSED                                   246.1us
test_foobar.py::test_foobar[4] PASSED                                   243.9us

Closes #13125

@psf-chronographer psf-chronographer bot added the bot:chronographer:provided (automation) changelog entry is part of PR label Jan 11, 2025
@nicoddemus
Copy link
Member

@afcmrp can you add a sample output using the new option to the PR description?

@afcmrp
Copy link
Author

afcmrp commented Jan 11, 2025

@afcmrp can you add a sample output using the new option to the PR description?

@nicoddemus Yes of course.

Non-verbose:

test_bar.py ..........                                                  4.094ms
test_foo.py .....                                                       2.013ms
test_foobar.py .....                                                    1.837ms

Verbose:

test_bar.py::test_bar[0] PASSED                                         320.8us
test_bar.py::test_bar[1] PASSED                                         260.9us
test_bar.py::test_bar[2] PASSED                                         236.9us
test_bar.py::test_bar[3] PASSED                                         234.2us
test_bar.py::test_bar[4] PASSED                                         241.9us
test_bar.py::test_bar[5] PASSED                                         230.3us
test_bar.py::test_bar[6] PASSED                                         230.7us
test_bar.py::test_bar[7] PASSED                                         248.6us
test_bar.py::test_bar[8] PASSED                                         245.6us
test_bar.py::test_bar[9] PASSED                                         231.0us
test_foo.py::test_foo[0] PASSED                                         269.7us
test_foo.py::test_foo[1] PASSED                                         250.6us
test_foo.py::test_foo[2] PASSED                                         241.2us
test_foo.py::test_foo[3] PASSED                                         243.0us
test_foo.py::test_foo[4] PASSED                                         242.3us
test_foobar.py::test_foobar[0] PASSED                                   236.1us
test_foobar.py::test_foobar[1] PASSED                                   253.7us
test_foobar.py::test_foobar[2] PASSED                                   264.8us
test_foobar.py::test_foobar[3] PASSED                                   246.1us
test_foobar.py::test_foobar[4] PASSED                                   243.9us

@nicoddemus
Copy link
Member

I like the output, thanks!

I will review the code ASAP.

Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

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

Overall this all looks great!

Playing around with it with xdist:

[gw0]101.1ms PASSED .tmp/test_bar.py::test_bar[0]
.tmp/test_bar.py::test_bar[1]
[gw0]541.1ms PASSED .tmp/test_bar.py::test_bar[1]
.tmp/test_bar.py::test_bar[6]
[gw2]179.7us PASSED .tmp/test_bar.py::test_bar[4]
.tmp/test_bar.py::test_bar[5]
[gw1]188.4us PASSED .tmp/test_bar.py::test_bar[2]

And without the option:

[gw2] [  6%] PASSED .tmp/test_bar.py::test_bar[4]
.tmp/test_bar.py::test_bar[5]
[gw0] [ 12%] PASSED .tmp/test_bar.py::test_bar[0]
.tmp/test_bar.py::test_bar[1]
[gw2] [ 18%] PASSED .tmp/test_bar.py::test_bar[5]
.tmp/test_bar.py::test_bar[6]
[gw1] [ 25%] PASSED .tmp/test_bar.py::test_bar[2]

The timing is "glued" to the gateway name ([gw0]101.1ms PASSED), I think it would look better if it was separated by a space ([gw0] 101.1ms PASSED).

Can you fix that and add a test for it?

testing/test_terminal.py Outdated Show resolved Hide resolved
@nicoddemus
Copy link
Member

Some final bikeshedding: perhaps the option should be timings or times, meaning what the user sees rather than the mechanism used to compute the times? But this is really minor.

@afcmrp
Copy link
Author

afcmrp commented Jan 17, 2025

Some final bikeshedding: perhaps the option should be timings or times, meaning what the user sees rather than the mechanism used to compute the times? But this is really minor.

Yes, agreed. Of those two I prefer times, shorter and closer to "base" form of the word. Do you think anyone else have a good idea for what to name it?

@nicoddemus
Copy link
Member

Of those two I prefer times, shorter and closer to "base" form of the word. Do you think anyone else have a good idea for what to name it?

To me times sounds good. 👍

@afcmrp afcmrp changed the title Add confval console_output_style option timer Add confval console_output_style option times Jan 17, 2025
Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

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

Awesome, thanks, excellent contribution!

I will leave this open for a few more days to give others a chance to chime in.

changelog/13125.feature.rst Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot:chronographer:provided (automation) changelog entry is part of PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Console output for execution time of tests
2 participants