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.
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 OuputVerbosity and use it in assertions #11473
Add OuputVerbosity and use it in assertions #11473
Changes from 17 commits
c93bc1e
64f7277
12cb693
5b36f86
cb9729b
ec76cf5
933742a
8e38fc0
cfb1654
4d7ceb8
855f579
d2529ed
c5cc08a
1e91814
b8714de
50c0b93
192b62d
1baca27
21d8111
675584f
9580f4b
b47b203
91bfcab
5d2183d
6ea1fe5
8fdfc1e
e663889
d943866
b91a467
59e39c8
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Since we only have one aspect at the moment, it seems premature to write the prose in a general/list format.
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.
My intention is to implement some of the other verbosity types I identified once this PR is merged. A different suggestion was to add a note that only one is currently supported. That seems sufficient to me if "life happens" and it takes me some time to get to adding other types.
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 it would be better to avoid this check, so that plugins can add their own verbosity types. We would instead check if the ini exists.
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 like the idea of checking the ini data instead (one less thing to manually manage).
In a previous conversation, there was a conversation about plugins creating their own levels. I think I'm still in the "pytest can enumerate a few buckets" camp. @nicoddemus Do you have any thoughts? If we did allow plugins to create types,
_add_verbosity_ini
would have to be made public and some more documentation updates.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 we can keep it simple for now, making this internal only; if the need comes later, we can discuss how to make this public -- better to err on the side of caution, as it is easy to make something public later, rather than making something public and then regretting it later.
Check warning on line 1666 in src/_pytest/config/__init__.py
Codecov / codecov/patch
src/_pytest/config/__init__.py#L1666
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.
Same here, the name
_add_ini
is too general now.Also, IMO this function doesn't add much value, I think it would be better to reduce the indirection and inline it. But since it's private then it's OK if you want to keep it.
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 prefer to keep it because it will avoid duplication as more types are added. It also ensures that the same default is used across verbosity types. If someone adds an Nth type, but uses "global", then
get_verbosity()
will raise an exception.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.
With the move away from
getoption()
, this needed to get a bit more complicated. (In the long term, it would probably be beneficial move the code base to a consistent way to access the verbosity level.)