Deprecate `RaisesGroup`

[AbduazizZiyodov]

Closes #3326 by deprecating RaisesGroup where pytest alternative recommended from now on.

[AbduazizZiyodov]

I've got couple of questions.

• Version should be 0.31.0 or 0.32.0 ?
• Is that OK to warn about deprecation during __init__ or do I have to move this into __enter__ ?
• What can we do about the existing tests that uses RaisesGroup, can just "replace" them with pytest alternative ?

(I'll include newsfragment later)

[codecov[bot]]

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 94.30300%. Comparing base (84fbcb7) to head (5be7c99).

:x: Your project check has failed because the head coverage (94.30300%) is below the target coverage (100.00000%). You can increase the head coverage or adjust the target coverage.

:exclamation: There is a different number of reports uploaded between BASE (84fbcb7) and HEAD (5be7c99). Click for more details.

HEAD has 3 uploads less than BASE

Flag	BASE (84fbcb7)	HEAD (5be7c99)
3.11	5	4
Windows	15	14
3.12	5	4

Additional details and impacted files

@@                 Coverage Diff                  @@
##                 main       #3337         +/-   ##
====================================================
- Coverage   100.00000%   94.30300%   -5.69701%     
====================================================
  Files             125         125                 
  Lines           19586       19835        +249     
  Branches         1342        1384         +42     
====================================================
- Hits            19586       18705        -881     
- Misses              0        1102       +1102     
- Partials            0          28         +28     

Files with missing lines	Coverage Δ
src/trio/testing/_raises_group.py	29.01408% <100.00000%> (-70.98592%)	:arrow_down:

... and 43 files with indirect coverage changes

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[A5rocks]

I've got couple of questions.

• version should be the next Trio version. Since we're including a deprecation, this is a "minor" change, it's 0.32.0.
• personally I think this should warn on attribute access! See _deprecate.deprecate_attributes. This way, someone even just going trio.RaisesGroup would get the warning without having to initialize it
• yeah. I think the idiomatic replacement is pytest.raises_group? (Maybe the deprecation should link that instead of the class)

[AbduazizZiyodov]

@A5rocks

personally I think this should warn on attribute access! See _deprecate.deprecate_attributes. This way, someone even just going trio.RaisesGroup would get the warning without having to initialize it

Makes sense. However even if I deprecate it on trio/__init__.py like this:

_deprecate.deprecate_attributes(
    __name__,
    {
        "RaisesGroup": _deprecate.DeprecatedAttribute(
            "RaisesGroup",
            version="0.32.0",
            issue=3326,
            instead="See https://docs.pytest.org/en/stable/reference/reference.html#pytest.RaisesGroup",
        )
    },
)

On trio.testing.RaisesGroup or just using RaisesGroup (via from trio.testing import) won't show any warnings. I guess we're trying to warn on even occurrence of RaisesGroup (even tried on testing/__init__.py). Am I missing something ? Thanks.

[A5rocks]

I think the deprecation should go in trio.testing?

I'm probably misunderstanding what you're saying cause it sounds like you tried that...

[A5rocks]

Note that this is how the usage looks like (it's a bit complicated because you need the actual underlying object, but under a different name)

deprecate_attributes(__name__, {
    "RaisesGroup": DeprecatedAttribute(_RaisesGroup, "0.32.0", ...)
})

[jakkdl]

note that Matcher (and AbstractMatcher I suppose) also needs to be deprecated. The tests that test RaisesGroup/Matcher itself should suppress the deprecation warning, while all other tests that make use of them should be updated to use the equivalent ones in pytest.

[AbduazizZiyodov]

@jakkdl

1. Regarding to Matcher:

    with RaisesGroup(
        Matcher(_core.BrokenResourceError, match="^Parking lot broken by"),
    ):
        async with _core.open_nursery() as nursery:
            nursery.start_soon(bad_parker, lot, cs)
            await wait_all_tasks_blocked()

            nursery.start_soon(lot.park)
            await wait_all_tasks_blocked()

            cs.cancel()

How should I supposed to write this via pytest alternatives, specifically Matcher ? We can't pass Matcher to pytest.RaisesGroup as I see. I did search for Matcher from pytest docs, however I'm not sure whether it does same job.

2. Should we suppress warnings (for the modules/files that actually test trio's RaisesGroup as you mentioned) like this (in pyproject.toml):

[tool.pytest.ini_options]
filterwarnings = [
  # ...
  'ignore::trio.TrioDeprecationWarning:trio._tests.test_testing_raisesgroup'
]

From my point of view, files that should suppress deprecations are:

• trio._tests.test_testing_raisesgroup.py
• trio._tests.type_tests.raisesgroup.py ?

Anything else ?

[A5rocks]

@AbduazizZiyodov I might be misunderstanding things but:

1. maybe just deprecate it? we don't need to provide a direct translation to pytest. (I think the direct translation is RaisesExc, though)
2. suppress warnings per test case. I think there's examples elsewhere for how to apply a warning filter on a specific test case.

[jakkdl]

1. yeah for tests that make use of Matcher outside those files the correct replacement is RaisesExc. It got renamed when moved to pytest.

[A5rocks]

@AbduazizZiyodov just bumping since it would be nice to get this by the next release. (which will be... I don't know yet, maybe New Year's would be a nice time?)

[AbduazizZiyodov]

@A5rocks Yep, we can do that (till new year's).