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

Update terminal.py to fix #11777 #12192

Merged
merged 12 commits into from
Apr 10, 2024
Merged

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.

@@ -2443,6 +2448,43 @@ def markup(self, word: str, **markup: str):
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
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Verbosity setting does not affect truncation of sequences when comparing their length
2 participants