[pre-commit.ci] pre-commit autoupdate #13733
Conversation
![pre-commit-ci[bot]](https://avatars.githubusercontent.com/in/68672?s=60&v=4){kind=small}
|
Looks like the new ruff errors are like this one: I find this a bit annoying, wouldn't feel bad about disabling this one, though it might potentially catch a real problem. |
|
I like it, after experiencing it in Rust. I have fixed that in execnet and pytest-xdist. I would vote to keep it. If nobody strongly opposes keeping it, I will fix this later. |
|
We can also enable unused-variable /unused-argument from pylint if we do |
nicoddemus
force-pushed
the
pre-commit-ci-update-config
branch
from
acfd898 to
7a0aa3d
Compare
|
Fixed the linting errors, but I'm not sure about these mypy failures: |
|
At least the first error (line 455) seems correct to me. From my reading of the code the Maybe @jakkdl is willing to take a look? If not, I can look into it more. |
|
L468 and L470 are correct errors and need to be L455 does have coverage issues, so there's a bug there! Possibly misjuggling of edit: I think it should be |
|
It seems like I don't have push permissions to the branch this PR is based on, so here's the patch: From 1b67c4bdcb1af8b96a529e837c9922b38c231f61 Mon Sep 17 00:00:00 2001
From: jakkdl <[email protected]>
Date: Fri, 19 Sep 2025 17:16:07 +0200
Subject: [PATCH] fix type failures in raises.py and add tests
---
src/_pytest/raises.py | 6 +++---
testing/python/raises_group.py | 10 ++++++++++
2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/src/_pytest/raises.py b/src/_pytest/raises.py
index 9066779a8..2a53a5a14 100644
--- a/src/_pytest/raises.py
+++ b/src/_pytest/raises.py
@@ -452,7 +452,7 @@ class AbstractRaises(ABC, Generic[BaseExcT_co]):
issubclass(origin_exc, BaseExceptionGroup)
and exc_type in (BaseException, Any)
):
- if not isinstance(exc, Exception):
+ if not issubclass(origin_exc, ExceptionGroup):
self.is_baseexception = True
return cast(type[BaseExcT_1], origin_exc)
else:
@@ -465,9 +465,9 @@ class AbstractRaises(ABC, Generic[BaseExcT_co]):
)
# unclear if the Type/ValueError distinction is even helpful here
msg = f"expected exception must be {expected}, not "
- if isinstance(exc, type):
+ if isinstance(exc, type): # type: ignore[unreachable]
raise ValueError(msg + f"{exc.__name__!r}")
- if isinstance(exc, BaseException):
+ if isinstance(exc, BaseException): # type: ignore[unreachable]
raise TypeError(msg + f"an exception instance ({type(exc).__name__})")
raise TypeError(msg + repr(type(exc).__name__))
diff --git a/testing/python/raises_group.py b/testing/python/raises_group.py
index 6b9f98201..e911ba52c 100644
--- a/testing/python/raises_group.py
+++ b/testing/python/raises_group.py
@@ -1325,6 +1325,16 @@ def test_annotated_group() -> None:
with RaisesExc(BaseExceptionGroup[BaseException]):
raise BaseExceptionGroup("", [KeyboardInterrupt()])
+ # assure AbstractRaises.is_baseexception is set properly
+ assert (
+ RaisesGroup(ExceptionGroup[Exception]).expected_type()
+ == "ExceptionGroup(ExceptionGroup)"
+ )
+ assert (
+ RaisesGroup(BaseExceptionGroup[BaseException]).expected_type()
+ == "BaseExceptionGroup(BaseExceptionGroup)"
+ )
+
def test_tuples() -> None:
# raises has historically supported one of several exceptions being raised
--
2.51.0
|
nicoddemus
force-pushed
the
pre-commit-ci-update-config
branch
4 times, most recently
from
34d79ce to
b863ca9
Compare
|
Thanks @jakkdl, applied! Also rebased the branch. |
nicoddemus
force-pushed
the
pre-commit-ci-update-config
branch
from
b863ca9 to
bd2cb21
Compare
nicoddemus
force-pushed
the
pre-commit-ci-update-config
branch
3 times, most recently
from
38eb0a6 to
96cf552
Compare
|
I tried its autofix feature and it changed to: diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index 41a0773e1..fe930b8d8 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -243,17 +243,7 @@ jobs:
continue-on-error: >-
${{
contains(
- fromJSON(
- '[
- "windows-py310-pluggy",
- "windows-py313",
- "ubuntu-py310-pluggy",
- "ubuntu-py310-freeze",
- "ubuntu-py313",
- "macos-py310",
- "macos-py313"
- ]'
- ),
+ Array,
matrix.name
)
&& true
Which is clearly incorrect. I then tried using: continue-on-error: ${{ contains(['windows-py310-pluggy','windows-py313','ubuntu-py310-pluggy','ubuntu-py310-freeze','ubuntu-py313','macos-py310','macos-py313'], matrix.name ) && true || false }}But this also fails with: I went with an ignore comment, but suggestions are welcome. |
|
Actually it seems the comment was also not welcome... @webknjaz any suggestions? |
|
@nicoddemus it must be wrapped with As for zizmor, you can add an ignore for that rule I suppose, if you insist on using this approach. I believe @woodruffw would advise the same. Though, the docs also demonstrate an example of adding an additional factor to the matrix and just using that in the conditional. This would be cleaner and easier to follow in general. I've started calling that factor Having that flag keeps data bundled in one place (the matrix definition) while the logic is disconnected and acts on that data. |
.github/workflows/test.yml
Outdated
| use_coverage: true | ||
|
|
||
| continue-on-error: >- | ||
| continue-on-error: | # zizmor: ignore[obfuscation] |
| # unclear if the Type/ValueError distinction is even helpful here | ||
| msg = f"expected exception must be {expected}, not " | ||
| if isinstance(exc, type): | ||
| if isinstance(exc, type): # type: ignore[unreachable] |
There was a problem hiding this comment.
Why not change the type in the method signature and rely on these to perform type narrowing?
There was a problem hiding this comment.
I tried to play with it but couldn't get it right TBH.
There was a problem hiding this comment.
the method is not supposed to accept these types, these error messages are explicitly for if a user is not using type checking and sending invalid types into the method.
| @pytest.fixture | ||
| def arg2(request): | ||
| pytest.raises(Exception, request.getfixturevalue, "arg1") | ||
| pytest.raises(Exception, request.getfixturevalue, "arg1") # noqa: B017 |
There was a problem hiding this comment.
For linters that don't have human-readable rule identifiers, it'd be a good idea to add a code comment explaining what the suppressions hide and when they can be dropped.
There was a problem hiding this comment.
Changed to the concrete exception (FixtureLookupError) and removed noqa. 👍
…alue zizmor complains that the previous approach was considered "obfuscation". Using a matrix value instead as suggested in #13733 (comment).
nicoddemus
force-pushed
the
pre-commit-ci-update-config
branch
from
96cf552 to
05324ab
Compare
| from py import error | ||
| from py.path import local | ||
|
|
||
| from py import error |
There was a problem hiding this comment.
This doesn't seem right. Why does ruff want inverse sort inhere?
There was a problem hiding this comment.
No idea TBH 🤷♂️
I don't care too much as long as it is consistent in different machines/computers.
…alue zizmor complains that the previous approach was considered "obfuscation". Using a matrix value instead as suggested in #13733 (comment).
nicoddemus
force-pushed
the
pre-commit-ci-update-config
branch
from
8963f13 to
b96a0b7
Compare
|
Hmm, that constexpr evaluation is definitely a bug on our part. Thanks for the ping! ( |
updates: - [github.com/astral-sh/ruff-pre-commit: v0.12.12 → v0.13.0](astral-sh/ruff-pre-commit@v0.12.12...v0.13.0) - [github.com/woodruffw/zizmor-pre-commit: v1.12.1 → v1.13.0](zizmorcore/zizmor-pre-commit@v1.12.1...v1.13.0) - [github.com/pre-commit/mirrors-mypy: v1.17.1 → v1.18.1](pre-commit/mirrors-mypy@v1.17.1...v1.18.1)
Fix typing failures after mypy 1.18 update.
…alue zizmor complains that the previous approach was considered "obfuscation". Using a matrix value instead as suggested in #13733 (comment).
nicoddemus
force-pushed
the
pre-commit-ci-update-config
branch
from
b96a0b7 to
b290db3
Compare
|
Just following up: I've fixed that consteval bug on zizmor's side in the release that's coming out now, please let me know if you hit any other problems there! |
updates: