Skip to content

Conversation

@HolyMagician03-UMich
Copy link
Contributor

@HolyMagician03-UMich HolyMagician03-UMich commented Apr 6, 2024

Increases width for short test summary info to reflect verbosity settings. Temporarily resolves #11777

@HolyMagician03-UMich
Copy link
Contributor Author

Hi! Can I get some opinion on what the available_width should be set to? Right now it's hard-coded to 500, so I imagine that if the list gets longer, the same issue will occur again.

Also, this is the first time I'm contributing to an open-source project, so please let me know what I'm missing in this commit. Thanks!

@nicoddemus
Copy link
Member

Hi! Can I get some opinion on what the available_width should be set to? Right now it's hard-coded to 500, so I imagine that if the list gets longer, the same issue will occur again.

I think in this case we do not want to call _format_trimmed at all, leaving the string unchanged.

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.

Thanks @HolyMagician03-UMich!

Besides my comments, please add a new test to test_terminal.py to test the new behavior.

You will need to monkeypatch running_on_ci to always return False in order to ensure the test is testing the -vv flag when running on GitHub actions.

@HolyMagician03-UMich
Copy link
Contributor Author

HolyMagician03-UMich commented Apr 7, 2024

Hi! It seems that right now test_line_with_reprcrash is failing because the config object is not initialized with a option.verbose field. Would it be ok for me to modify the config class and give it a verbose of 0? It'll be similar to line 2460 - 2466 of the test_terminal.py in the previous commit, just with a verbosity value of 0.

@nicoddemus
Copy link
Member

Would it be ok for me to modify the config class and give it a verbose of 0?

You mean the stub config object used in the test? Yes please go ahead.

check("🉐🉐🉐🉐🉐\n2nd line", 80, "FAILED nodeid::🉐::withunicode - 🉐🉐🉐🉐🉐")


def test_short_summary_with_verbose(
Copy link
Member

Choose a reason for hiding this comment

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

@HolyMagician03-UMich took the liberty of simplifying this test. 👍

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.

@HolyMagician03-UMich
Copy link
Contributor Author

@nicoddemus Thank you for taking the time to review these changes!

@nicoddemus
Copy link
Member

Note to self: squash before merging.

I will leave this open for a few more days in case others want to review it.

@nicoddemus nicoddemus merged commit 089116b into pytest-dev:main Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants