Add OuputVerbosity and use it in assertions #11473

plannigan · 2023-10-01T14:22:47Z

OuputVerbosity is a new class that allows for retrieving a verbosity level for a specific output type. If nothing is specifically configured, the application wide verbose value is used.

The first use of this functionality is verbosity_assertions which can be used to make the assertion reports output at the most verbose level while still showing a single character for each test case when progress is being reported.

Addresses #11387

plannigan · 2023-10-01T14:27:13Z

src/_pytest/config/__init__.py

+        return f"verbosity_{output_type}"
+
+    @staticmethod
+    def add_ini(parser: "Parser", output_type: str, help: str) -> None:


Since many things will add these type of settings, I thought it would be helpful to have this function to ensure it is dong consistently. I'm not completely in love with this level of indirection. But it is necessary since config settings are added to the Parser before the Config object exists.

Good idea. 👍

plannigan · 2023-10-01T14:29:24Z

testing/test_assertion.py

-    class Config:
-        def getoption(self, name):
-            if name == "verbose":
+def mock_config(verbose: int = 0, assertion_override: Optional[int] = None):


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.)

plannigan · 2023-10-21T14:18:35Z

Since this is my first contribution, I'm not sure the best way to get a review for this PR that has been up for 3 weeks. @nicoddemus & @Zac-HD, you both interacted with the issue. Would one of you be able to review this code?

nicoddemus · 2023-10-21T17:28:50Z

Since this is my first contribution, I'm not sure the best way to get a review for this PR that has been up for 3 weeks

Sorry @plannigan, you are not supposed to do anything else other than opening the PR: we dropped the ball for not reviewing it earlier (I personally have been busy). When that happens a ping (like you did) is appreciated, thank you!

I will try my best to review this in the next couple of days. 👍

nicoddemus

Great work @plannigan, and sorry again for the delay!

The implementation is good, however I think we might need some more interactions given this introduce a new API, which must be done carefully and with care.

doc/en/how-to/output.rst

src/_pytest/assertion/__init__.py

nicoddemus · 2023-10-23T11:14:47Z

src/_pytest/config/__init__.py

+        return f"verbosity_{output_type}"
+
+    @staticmethod
+    def add_ini(parser: "Parser", output_type: str, help: str) -> None:


Good idea. 👍

nicoddemus · 2023-10-23T11:34:43Z

src/_pytest/config/__init__.py

+        """Application wide verbosity level."""
+        return cast(int, self._config.option.verbose)
+
+    def verbosity_for(self, output_type: str) -> int:


I think if one passes an invalid output_type at the moment, we will get an error (due to self._config.getini).

We probably want to have this classs accessible to plugins and users, via config.output_verbosity, so we need to think about its API carefully as we will need to support this officially moving on.

Some points:

In tandem with pytest's direction regarding type safety, I think we should use an Enum with the output types currently available, something along the lines of:

class VerbosityType(Enum): """...""" Global = "global" Assertions = "assertions"

Instead of a simple output_type: str parameter/value.

The class name is OutputVerbosity and the public methods are verbosity and verbosity_for, which read a bit redundant: confg.output_verbosity.verbosity, confg.output_verbosity.get_verbosity_for. I suggest we use simply get, defaulting to the Global verbosity:

def get(self, type: VerbosityType = VerbosityType.Global) -> int: ...

Which reads well I think:

config.output_verbosity.get() config.output_verbosity.get(VerbosityType.Assertions)

We need to properly document both OutputVerbosity and VerbosityType in the reference docs (including some usage examples, but possibly we can write those in the class docstring). We also need to expose VerbosityType in the pytest namespace.

Given OutputVerbosity is public, we should make OutputVerbosity.addini private, as it is intended for internal use only.

I'm not married to the names above, so suggestions and bikeshedding are welcome.

I opted for str because I was thinking that plugins might want also register types, like any other config setting. Which would mean that it wouldn't be possible to statically define all of the types. However, reflecting on that, I'm not sure that level of extensibility is necessary. It is likely better for pytest to say "here are N buckets that a plugin can utilize" and that way users of multiple plugins that leverage this functionality will see consistent behavior without needing to enable M settings.

That is fair. It also works nicely with the enum suggestion.

Sure

I had it as public for the reasons described in 1, but it makes sense to make it private based on that change.

changelog/11387.feature.rst

bluetech

I'm not able to do a full review ATM, but left some quick comments.

doc/en/how-to/output.rst

bluetech · 2023-10-25T12:49:04Z

doc/en/how-to/output.rst

+Fine gain verbosity
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+In addition to specifying the application wide verbosity level, it is possible to control specific aspects independently.


Since we only have one aspect at the moment, it seems premature to write the prose in a general/list format.

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.

doc/en/how-to/output.rst

doc/en/reference/reference.rst

src/_pytest/assertion/truncate.py

bluetech · 2023-10-25T12:54:56Z

src/_pytest/config/__init__.py

@@ -1662,6 +1663,54 @@ def _warn_about_skipped_plugins(self) -> None:
            )


+class OutputVerbosity:


Why OutputVerbosity and not just Verbosity? I can't think of a verbosity that is not related to output, so it seems redundant.

I'm open tweaking the name. I generally default to more explicit names.

src/_pytest/config/__init__.py

Co-authored-by: Ran Benita <[email protected]> Co-authored-by: Bruno Oliveira <[email protected]>

* Use enum instead of string * Change method name to simple get * Make type registering internal

plannigan · 2023-11-05T21:00:55Z

I believe I have addressed the technical comments and documentation requests.

I'm not sure why the doc building step is failing. The error don't seem to be related to the lines I changed. If someone with more experience with sphinx can take a look, I would appreciate it.

nicoddemus · 2023-11-07T21:45:53Z

I'm not sure why the doc building step is failing

The problem was that you used ^^^ for the heading in your new session, while in the rest of the document we used ~~~ for that level of heading. I pushed a fix. 👍

bluetech

Thanks for the update @plannigan, my comments:

The OutputVerbosity and VerbosityType seem more complicated than necessary to me. What is the advantage over doing it the direct way, i.e. adding the ini directly and maybe adding a getter on Config?
If we stick with OutputVerbosity, let's make it just Verbosity -- the Output part is entirely redundant IMO.
We should have at least one end-to-end test (pytester.runpytest test) which shows this working, i.e. changing the output vs. not setting the ini.

nicoddemus · 2023-11-08T10:55:56Z

i.e. adding the ini directly

I think this is to handle the fallback to default verbosity when the setting is defined in the ini file.

and maybe adding a getter on Config?

Can you clarify what you mean here, do you mean adding:

Config.get_verbosity(VerbosityType) -> int
Config.get_verbosity(Literal["assertion"]) -> int
Config.get_assertion_verbosity() -> int

bluetech · 2023-11-08T11:17:26Z

Can you clarify what you mean here, do you mean adding:

Yes, any of these would be fine by me.

nicoddemus · 2023-11-08T12:05:09Z

One nice effect of implementing some functionality directly in Config as suggested by @bluetech is that it would cut down on the number of things exposed publicly, specially 2) in my suggestion above.

Using Config.get_verbosity(str) we can by default return the default verbosity if an unknown verbosity is passed , to make the life of plugin authors easier: using a VerbosityType enum has many advantages, but one disadvantage is that plugin authors that need to support multiple pytest versions need to do some juggling to see if they have access to VerbosityType.X.

Suppose in the future we add VerbosityType.Rubles in pytest 9.0, plugins would have to juggle a bit to support pytest 7.x+ up to 9:

if hasattr(config, "output_verbosity") and hasattr(pytest, "VerbosityType") and hasattr(pytest.VerbosityType, "Rubles"):
    rubles_verbosity = config.output_verbosity.get(pytest.VerbosityType, Rubles)
else:
    rubles_verbosity = config.option.verbose

While using option 2) above users can go with:

if hasattr(config, "get_verbosity"):
    rubles_verbosity = config.get_verbosity("rubles")
else:
    rubles_verbosity = config.option.verbose

changelog/11387.feature.rst

testing/test_assertion.py

plannigan · 2023-11-12T00:06:19Z

Looks like one of the jobs failed with a file system error in an area I didn't touch. I don't have permissions to retry the job.

nicoddemus · 2023-11-12T13:07:38Z

Looks like one of the jobs failed with a file system error in an area I didn't touch. I don't have permissions to retry the job.

Started it again. 👍

bluetech

Haven't had time for a full review again, but left some comments, please take a look.

src/_pytest/config/__init__.py

bluetech · 2023-11-12T21:45:49Z

src/_pytest/config/__init__.py

+        return f"verbosity_{verbosity_type}"
+
+    @staticmethod
+    def _add_ini(parser: "Parser", verbosity_type: str, help: str) -> None:


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.

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.

src/_pytest/config/__init__.py

testing/test_assertion.py

* don't use implementaiton for setting name * str() of path object is not needed

bluetech

Thanks for the update @plannigan, please see my comment on _KNOWN_VERBOSITY_TYPES. Other than that it looks good!

doc/en/reference/reference.rst

src/_pytest/assertion/truncate.py

bluetech · 2023-11-17T08:52:36Z

src/_pytest/config/__init__.py

+        assert isinstance(global_level, int)
+        if (
+            verbosity_type is None
+            or verbosity_type not in Config._KNOWN_VERBOSITY_TYPES


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.

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.

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.

src/_pytest/config/__init__.py

bluetech

LGTM! I know this feature will make a lot of people happy :)

I'll let @nicoddemus do the final review/merge.

nicoddemus · 2023-11-19T14:55:21Z

Thanks everyone!

Add OuputVerbosity and use it in assertions

c93bc1e

plannigan commented Oct 1, 2023

View reviewed changes

plannigan added 4 commits October 1, 2023 10:44

Add test cases to confirm implementation of mock_config()

64f7277

Fix typo

12cb693

Merge branch 'main' into fine-grain-verbosity

5b36f86

Merge branch 'main' into fine-grain-verbosity

cb9729b

nicoddemus requested changes Oct 23, 2023

View reviewed changes

bluetech reviewed Oct 25, 2023

View reviewed changes

plannigan and others added 6 commits October 31, 2023 14:03

Apply documentation suggestions

ec76cf5

Co-authored-by: Ran Benita <[email protected]> Co-authored-by: Bruno Oliveira <[email protected]>

Adjust verbosity level retrival

933742a

* Use enum instead of string * Change method name to simple get * Make type registering internal

Expose VerbosityType

8e38fc0

Assert isinstance intead of cast

cfb1654

Improve documentation of functionality

4d7ceb8

Merge branch 'main' into fine-grain-verbosity

855f579

plannigan requested review from nicoddemus and bluetech November 5, 2023 20:43

Fix docs

d2529ed

nicoddemus added 3 commits November 7, 2023 19:14

Improve docs

c5cc08a

Remove unused _option_name_fmt

1e91814

Remove OutputVerbosity.verbose

b8714de

nicoddemus force-pushed the fine-grain-verbosity branch from 3f2b440 to b8714de Compare November 7, 2023 22:22

nicoddemus approved these changes Nov 7, 2023

View reviewed changes

bluetech requested changes Nov 8, 2023

View reviewed changes

Merge branch 'main' into fine-grain-verbosity

192b62d

nicoddemus reviewed Nov 11, 2023

View reviewed changes

changelog/11387.feature.rst Outdated Show resolved Hide resolved

testing/test_assertion.py Show resolved Hide resolved

nicoddemus and others added 4 commits November 11, 2023 12:22

Update changelog/11387.feature.rst

1baca27

User fine grained verbosity for saferepr max size

21d8111

Add end to end test of assertion fine grained verbosity

675584f

Merge branch 'main' into fine-grain-verbosity

9580f4b

plannigan requested a review from bluetech November 12, 2023 13:12

bluetech requested changes Nov 12, 2023

View reviewed changes

plannigan added 5 commits November 12, 2023 20:07

More explict names

b47b203

Test case for unknown verbosity type

91bfcab

Improve documentation of get_verbosity() and configuraiton

5d2183d

Clean up end to end test

6ea1fe5

* don't use implementaiton for setting name * str() of path object is not needed

Merge branch 'main' into fine-grain-verbosity

8fdfc1e

plannigan requested a review from bluetech November 13, 2023 14:03

bluetech requested changes Nov 17, 2023

View reviewed changes

plannigan added 4 commits November 18, 2023 07:03

Mark constant as final

e663889

Check parser for ini name instead of manually listing known types

d943866

Merge branch 'main' into fine-grain-verbosity

b91a467

Restore whitespace changes

59e39c8

plannigan requested a review from bluetech November 18, 2023 12:44

bluetech approved these changes Nov 18, 2023

View reviewed changes

nicoddemus approved these changes Nov 19, 2023

View reviewed changes

nicoddemus merged commit 9dc1fc4 into pytest-dev:main Nov 19, 2023
22 checks passed

plannigan mentioned this pull request Nov 25, 2023

Output verbosity for test cases #11639

Closed

plannigan deleted the fine-grain-verbosity branch November 25, 2023 20:15

This was referenced Feb 6, 2024

Add 'assert_truncate_level' to show the full diff without -vv #10588

Closed

Add distinct option to show full (assertion) diffs always #3962

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add OuputVerbosity and use it in assertions #11473

Add OuputVerbosity and use it in assertions #11473

plannigan commented Oct 1, 2023

plannigan Oct 1, 2023

nicoddemus Oct 23, 2023

plannigan Oct 1, 2023

plannigan commented Oct 21, 2023

nicoddemus commented Oct 21, 2023 •

edited

Loading

nicoddemus left a comment

nicoddemus Oct 23, 2023

nicoddemus Oct 23, 2023 •

edited

Loading

plannigan Oct 24, 2023

bluetech left a comment

bluetech Oct 25, 2023

plannigan Nov 5, 2023

bluetech Oct 25, 2023

plannigan Nov 5, 2023

plannigan commented Nov 5, 2023

nicoddemus commented Nov 7, 2023

bluetech left a comment

nicoddemus commented Nov 8, 2023

bluetech commented Nov 8, 2023

nicoddemus commented Nov 8, 2023 •

edited

Loading

plannigan commented Nov 12, 2023

nicoddemus commented Nov 12, 2023

bluetech left a comment

bluetech Nov 12, 2023

plannigan Nov 13, 2023

bluetech left a comment

bluetech Nov 17, 2023

plannigan Nov 17, 2023

nicoddemus Nov 17, 2023

bluetech left a comment

nicoddemus commented Nov 19, 2023

		@@ -1662,6 +1663,54 @@ def _warn_about_skipped_plugins(self) -> None:
		)


		class OutputVerbosity:

Add OuputVerbosity and use it in assertions #11473

Add OuputVerbosity and use it in assertions #11473

Conversation

plannigan commented Oct 1, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

plannigan commented Oct 21, 2023

nicoddemus commented Oct 21, 2023 • edited Loading

nicoddemus left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicoddemus Oct 23, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bluetech left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

plannigan commented Nov 5, 2023

nicoddemus commented Nov 7, 2023

bluetech left a comment

Choose a reason for hiding this comment

nicoddemus commented Nov 8, 2023

bluetech commented Nov 8, 2023

nicoddemus commented Nov 8, 2023 • edited Loading

plannigan commented Nov 12, 2023

nicoddemus commented Nov 12, 2023

bluetech left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bluetech left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bluetech left a comment

Choose a reason for hiding this comment

nicoddemus commented Nov 19, 2023

nicoddemus commented Oct 21, 2023 •

edited

Loading

nicoddemus Oct 23, 2023 •

edited

Loading

nicoddemus commented Nov 8, 2023 •

edited

Loading