Skip to content

Commit

Permalink
Revert collapsable-if and multiple-with-statements changes
Browse files Browse the repository at this point in the history
  • Loading branch information
CoolCat467 committed Oct 23, 2023
1 parent e55b1e3 commit 76d7413
Show file tree
Hide file tree
Showing 20 changed files with 252 additions and 236 deletions.
2 changes: 2 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ extend-ignore = [
'F405', # undefined-local-with-import-star-usage
'E402', # module-import-not-at-top-of-file (usually OS-specific)
'E501', # line-too-long
'SIM102', # collapsible-if (good idea, but ends up squashing things together, making them seem related - which they aren't)
'SIM117', # multiple-with-statements (messes up lots of context-based stuff and looks bad)
]

include = ["*.py", "*.pyi", "**/pyproject.toml"]
Expand Down
5 changes: 1 addition & 4 deletions trio/_core/_multierror.py
Original file line number Diff line number Diff line change
Expand Up @@ -328,10 +328,7 @@ def catch(
return MultiErrorCatcher(handler)


if TYPE_CHECKING: # noqa: SIM108
_ExceptionGroup = ExceptionGroup[Exception]
else:
_ExceptionGroup = ExceptionGroup
_ExceptionGroup = ExceptionGroup[Exception] if TYPE_CHECKING else ExceptionGroup


class NonBaseMultiError(MultiError, _ExceptionGroup):
Expand Down
10 changes: 3 additions & 7 deletions trio/_core/_run.py
Original file line number Diff line number Diff line change
Expand Up @@ -792,19 +792,15 @@ def cancel_called(self) -> bool:
cancelled, then :attr:`cancelled_caught` is usually more
appropriate.
"""
if (
self._cancel_status is not None
or not self._has_been_entered
and not self._cancel_called
and current_time() >= self._deadline
):
if self._cancel_status is not None or not self._has_been_entered:
# Scope is active or not yet entered: make sure cancel_called
# is true if the deadline has passed. This shouldn't
# be able to actually change behavior, since we check for
# deadline expiry on scope entry and at every checkpoint,
# but it makes the value returned by cancel_called more
# closely match expectations.
self.cancel()
if not self._cancel_called and current_time() >= self._deadline:
self.cancel()
return self._cancel_called


Expand Down
2 changes: 1 addition & 1 deletion trio/_core/_tests/test_ki.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ def protected_manager():
with protected_manager():
assert not _core.currently_ki_protected()

with pytest.raises(KeyError): # noqa: SIM117 # multiple-with-statements
with pytest.raises(KeyError):
# This is the one that used to fail
with protected_manager():
raise KeyError
Expand Down
29 changes: 8 additions & 21 deletions trio/_core/_tests/test_multierror.py
Original file line number Diff line number Diff line change
Expand Up @@ -283,9 +283,7 @@ def noop(_):

# Simple pass-through of all exceptions
m = make_tree()
with pytest.raises( # noqa: SIM117 # multiple-with-statements
MultiError
) as excinfo:
with pytest.raises(MultiError) as excinfo:
with pytest.warns(TrioDeprecationWarning), MultiError.catch(lambda exc: exc):
raise m
assert excinfo.value is m
Expand All @@ -308,9 +306,7 @@ def simple_filter(exc):
return RuntimeError()
return exc

with pytest.raises( # noqa: SIM117 # multiple-with-statements
MultiError
) as excinfo:
with pytest.raises(MultiError) as excinfo:
with pytest.warns(TrioDeprecationWarning), MultiError.catch(simple_filter):
raise make_tree()
new_m = excinfo.value
Expand All @@ -328,19 +324,15 @@ def simple_filter(exc):
# check preservation of __cause__ and __context__
v = ValueError()
v.__cause__ = KeyError()
with pytest.raises( # noqa: SIM117 # multiple-with-statements
ValueError
) as excinfo:
with pytest.raises(ValueError) as excinfo:
with pytest.warns(TrioDeprecationWarning), MultiError.catch(lambda exc: exc):
raise v
assert isinstance(excinfo.value.__cause__, KeyError)

v = ValueError()
context = KeyError()
v.__context__ = context
with pytest.raises( # noqa: SIM117 # multiple-with-statements
ValueError
) as excinfo:
with pytest.raises(ValueError) as excinfo:
with pytest.warns(TrioDeprecationWarning), MultiError.catch(lambda exc: exc):
raise v
assert excinfo.value.__context__ is context
Expand All @@ -360,9 +352,7 @@ def catch_RuntimeError(exc):
else:
return exc

with pytest.warns( # noqa: SIM117 # multiple-with-statements
TrioDeprecationWarning
):
with pytest.warns(TrioDeprecationWarning):
with MultiError.catch(catch_RuntimeError):
raise MultiError([v, distractor])
assert excinfo.value.__context__ is context
Expand Down Expand Up @@ -392,7 +382,7 @@ def simple_filter(exc):

try:
gc.set_debug(gc.DEBUG_SAVEALL)
with pytest.raises(MultiError): # noqa: SIM117 # multiple-with-statements
with pytest.raises(MultiError):
# covers MultiErrorCatcher.__exit__ and _multierror.copy_tb
with pytest.warns(TrioDeprecationWarning), MultiError.catch(simple_filter):
raise make_multi()
Expand Down Expand Up @@ -450,12 +440,9 @@ def run_script(name: str) -> subprocess.CompletedProcess[bytes]:

env = dict(os.environ)
print("parent PYTHONPATH:", env.get("PYTHONPATH"))
if ( # pragma: no cover # noqa: SIM108 # if-else-block-instead-of-if-exp
"PYTHONPATH" in env
):
pp = []
if "PYTHONPATH" in env: # pragma: no cover
pp = env["PYTHONPATH"].split(os.pathsep)
else:
pp = []
pp.insert(0, str(trio_path))
pp.insert(0, str(script_path.parent))
env["PYTHONPATH"] = os.pathsep.join(pp)
Expand Down
125 changes: 67 additions & 58 deletions trio/_core/_tests/test_run.py
Original file line number Diff line number Diff line change
Expand Up @@ -619,11 +619,12 @@ async def test_basic_timeout(mock_clock: _core.MockClock) -> None:

async def test_cancel_scope_nesting() -> None:
# Nested scopes: if two triggering at once, the outer one wins
with _core.CancelScope() as scope1, _core.CancelScope() as scope2: # noqa: SIM117 # multiple-with-statements
with _core.CancelScope() as scope3:
scope3.cancel()
scope2.cancel()
await sleep_forever()
with _core.CancelScope() as scope1:
with _core.CancelScope() as scope2:
with _core.CancelScope() as scope3:
scope3.cancel()
scope2.cancel()
await sleep_forever()
assert scope3.cancel_called
assert not scope3.cancelled_caught
assert scope2.cancel_called
Expand All @@ -632,17 +633,18 @@ async def test_cancel_scope_nesting() -> None:
assert not scope1.cancelled_caught

# shielding
with _core.CancelScope() as scope1, _core.CancelScope() as scope2:
scope1.cancel()
with pytest.raises(_core.Cancelled):
await _core.checkpoint()
with pytest.raises(_core.Cancelled):
await _core.checkpoint()
scope2.shield = True
await _core.checkpoint()
scope2.cancel()
with pytest.raises(_core.Cancelled):
with _core.CancelScope() as scope1:
with _core.CancelScope() as scope2:
scope1.cancel()
with pytest.raises(_core.Cancelled):
await _core.checkpoint()
with pytest.raises(_core.Cancelled):
await _core.checkpoint()
scope2.shield = True
await _core.checkpoint()
scope2.cancel()
with pytest.raises(_core.Cancelled):
await _core.checkpoint()

# if a scope is pending, but then gets popped off the stack, then it
# isn't delivered
Expand All @@ -655,12 +657,13 @@ async def test_cancel_scope_nesting() -> None:

# Regression test for https://github.com/python-trio/trio/issues/1175
async def test_unshield_while_cancel_propagating() -> None:
with _core.CancelScope() as outer, _core.CancelScope() as inner:
outer.cancel()
try:
await _core.checkpoint()
finally:
inner.shield = True
with _core.CancelScope() as outer:
with _core.CancelScope() as inner:
outer.cancel()
try:
await _core.checkpoint()
finally:
inner.shield = True
assert outer.cancelled_caught and not inner.cancelled_caught


Expand Down Expand Up @@ -699,14 +702,16 @@ async def sleep_until_cancelled(scope: _core.CancelScope) -> None:
await _core.checkpoint()
assert scope.cancel_called
assert not scope.cancelled_caught
with pytest.raises(RuntimeError) as exc_info, scope:
pass # pragma: no cover
with pytest.raises(RuntimeError) as exc_info:
with scope:
pass # pragma: no cover
assert "single 'with' block" in str(exc_info.value)

# Can't reenter
with _core.CancelScope() as scope:
with pytest.raises(RuntimeError) as exc_info, scope:
pass # pragma: no cover
with pytest.raises(RuntimeError) as exc_info:
with scope:
pass # pragma: no cover
assert "single 'with' block" in str(exc_info.value)

# Can't enter from multiple tasks simultaneously
Expand All @@ -720,8 +725,9 @@ async def enter_scope() -> None:
nursery.start_soon(enter_scope, name="this one")
await wait_all_tasks_blocked()

with pytest.raises(RuntimeError) as exc_info, scope:
pass # pragma: no cover
with pytest.raises(RuntimeError) as exc_info:
with scope:
pass # pragma: no cover
assert "single 'with' block" in str(exc_info.value)
nursery.cancel_scope.cancel()

Expand All @@ -740,8 +746,9 @@ async def test_cancel_scope_misnesting() -> None:
inner = _core.CancelScope()
with ExitStack() as stack:
stack.enter_context(outer)
with inner, pytest.raises(RuntimeError, match="still within its child"):
stack.close()
with inner:
with pytest.raises(RuntimeError, match="still within its child"):
stack.close()
# No further error is raised when exiting the inner context

# If there are other tasks inside the abandoned part of the cancel tree,
Expand All @@ -752,8 +759,9 @@ async def task1() -> None:

# Even if inside another cancel scope
async def task2() -> None:
with _core.CancelScope(), pytest.raises(_core.Cancelled):
await sleep_forever()
with _core.CancelScope():
with pytest.raises(_core.Cancelled):
await sleep_forever()

with ExitStack() as stack:
stack.enter_context(_core.CancelScope())
Expand Down Expand Up @@ -889,10 +897,9 @@ async def main() -> None:
task._schedule_points = "hello!" # type: ignore
await _core.checkpoint()

with ignore_coroutine_never_awaited_warnings(), pytest.raises(
_core.TrioInternalError
):
_core.run(main)
with ignore_coroutine_never_awaited_warnings():
with pytest.raises(_core.TrioInternalError):
_core.run(main)


async def test_spawn_system_task() -> None:
Expand Down Expand Up @@ -1407,25 +1414,26 @@ def slow_abort(raise_cancel: _core.RaiseCancelT) -> _core.Abort:
await _core.checkpoint()
record.append("done")

with _core.CancelScope() as outer1, _core.CancelScope() as outer2:
async with _core.open_nursery() as nursery:
# So we have a task blocked on an operation that can't be
# aborted immediately
nursery.start_soon(slow_aborter)
await wait_all_tasks_blocked()
assert record == ["sleeping"]
# And then we cancel it, so the abort callback gets run
outer1.cancel()
assert record == ["sleeping", "abort-called"]
# In fact that happens twice! (This used to cause the abort
# callback to be run twice)
outer2.cancel()
assert record == ["sleeping", "abort-called"]
# But then before the abort finishes, the task gets shielded!
nursery.cancel_scope.shield = True
# Now we wait for the task to finish...
# The cancellation was delivered, even though it was shielded
assert record == ["sleeping", "abort-called", "cancelled", "done"]
with _core.CancelScope() as outer1:
with _core.CancelScope() as outer2:
async with _core.open_nursery() as nursery:
# So we have a task blocked on an operation that can't be
# aborted immediately
nursery.start_soon(slow_aborter)
await wait_all_tasks_blocked()
assert record == ["sleeping"]
# And then we cancel it, so the abort callback gets run
outer1.cancel()
assert record == ["sleeping", "abort-called"]
# In fact that happens twice! (This used to cause the abort
# callback to be run twice)
outer2.cancel()
assert record == ["sleeping", "abort-called"]
# But then before the abort finishes, the task gets shielded!
nursery.cancel_scope.shield = True
# Now we wait for the task to finish...
# The cancellation was delivered, even though it was shielded
assert record == ["sleeping", "abort-called", "cancelled", "done"]


async def test_task_tree_introspection() -> None:
Expand All @@ -1439,7 +1447,7 @@ async def parent(

assert tasks["parent"].child_nurseries == []

async with _core.open_nursery() as nursery1: # noqa: SIM117 # multiple-with-statements
async with _core.open_nursery() as nursery1:
async with _core.open_nursery() as nursery2:
assert tasks["parent"].child_nurseries == [nursery1, nursery2]

Expand Down Expand Up @@ -2522,8 +2530,9 @@ async def test_cancel_scope_no_cancellederror() -> None:
a Cancelled exception, it will NOT set the ``cancelled_caught`` flag.
"""

with pytest.raises(ExceptionGroup), _core.CancelScope() as scope:
scope.cancel()
raise ExceptionGroup("test", [RuntimeError(), RuntimeError()])
with pytest.raises(ExceptionGroup):
with _core.CancelScope() as scope:
scope.cancel()
raise ExceptionGroup("test", [RuntimeError(), RuntimeError()])

assert not scope.cancelled_caught
2 changes: 1 addition & 1 deletion trio/_highlevel_socket.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ def __init__(self, socket: SocketType):
async def send_all(self, data: bytes | bytearray | memoryview) -> None:
if self.socket.did_shutdown_SHUT_WR:
raise trio.ClosedResourceError("can't send data after sending EOF")
with self._send_conflict_detector: # noqa: SIM117 # multiple-with-statements
with self._send_conflict_detector:
with _translate_socket_errors_to_stream_errors():
with memoryview(data) as data:
if not data:
Expand Down
17 changes: 8 additions & 9 deletions trio/_tests/test_dtls.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,10 +118,7 @@ async def test_handshake_over_terrible_network(
# avoid spurious timeouts on slow machines
autojump_clock.autojump_threshold = 0.001

async with dtls_echo_server() as ( # noqa: SIM117 # multiple-with-statements
_,
address,
):
async with dtls_echo_server() as (_, address):
async with trio.open_nursery() as nursery:

async def route_packet(packet: UDPPacket) -> None:
Expand Down Expand Up @@ -289,8 +286,9 @@ async def null_handler(_: object) -> None: # pragma: no cover


async def test_dtls_over_dgram_only() -> None:
with trio.socket.socket() as s, pytest.raises(ValueError):
DTLSEndpoint(s)
with trio.socket.socket() as s:
with pytest.raises(ValueError):
DTLSEndpoint(s)


async def test_double_serve() -> None:
Expand Down Expand Up @@ -823,9 +821,10 @@ def route_packet(packet: UDPPacket) -> None:

fn.route_packet = route_packet # type: ignore[assignment] # TODO add type annotations for FakeNet

with endpoint() as client_endpoint, trio.move_on_after(10):
client = client_endpoint.connect(address, client_ctx)
await client.do_handshake()
with endpoint() as client_endpoint:
with trio.move_on_after(10):
client = client_endpoint.connect(address, client_ctx)
await client.do_handshake()


async def test_association_replaced_while_handshake_running(
Expand Down
Loading

0 comments on commit 76d7413

Please sign in to comment.