Skip to content
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

Enable ruff's flake8-trio rule #2947

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ select = [
"RUF", # Ruff-specific rules
"SIM", # flake8-simplify
"TCH", # flake8-type-checking
"TRIO", # flake8-trio
"UP", # pyupgrade
"W", # Warning
"YTT", # flake8-2020
Expand Down
12 changes: 6 additions & 6 deletions src/trio/_core/_tests/test_guest_mode.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ def done_callback(outcome: Outcome[T]) -> None:

def test_guest_trivial() -> None:
async def trio_return(in_host: InHost) -> str:
await trio.sleep(0)
await trio.lowlevel.checkpoint()
return "ok"

assert trivial_guest_run(trio_return) == "ok"
Expand Down Expand Up @@ -149,7 +149,7 @@ def test_guest_is_initialized_when_start_returns() -> None:

async def trio_main(in_host: InHost) -> str:
record.append("main task ran")
await trio.sleep(0)
await trio.lowlevel.checkpoint()
assert trio.lowlevel.current_trio_token() is trio_token
return "ok"

Expand All @@ -164,7 +164,7 @@ def after_start() -> None:
@trio.lowlevel.spawn_system_task
async def early_task() -> None:
record.append("system task ran")
await trio.sleep(0)
await trio.lowlevel.checkpoint()

res = trivial_guest_run(trio_main, in_host_after_start=after_start)
assert res == "ok"
Expand Down Expand Up @@ -396,7 +396,7 @@ def do_abandoned_guest_run() -> None:
async def abandoned_main(in_host: InHost) -> None:
in_host(lambda: 1 / 0)
while True:
await trio.sleep(0)
await trio.lowlevel.checkpoint()

with pytest.raises(ZeroDivisionError):
trivial_guest_run(abandoned_main)
Expand Down Expand Up @@ -472,7 +472,7 @@ async def trio_main() -> str:

# Make sure we have at least one tick where we don't need to go into
# the thread
await trio.sleep(0)
await trio.lowlevel.checkpoint()

from_trio.put_nowait(0)

Expand Down Expand Up @@ -540,7 +540,7 @@ async def crash_in_run_loop(in_host: InHost) -> None:

async def crash_in_io(in_host: InHost) -> None:
m.setattr("trio._core._run.TheIOManager.get_events", None)
await trio.sleep(0)
await trio.lowlevel.checkpoint()

with pytest.raises(trio.TrioInternalError):
trivial_guest_run(crash_in_io)
Expand Down
2 changes: 1 addition & 1 deletion src/trio/_tests/test_scheduler_determinism.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ async def scheduler_trace() -> tuple[tuple[str, int], ...]:
async def tracer(name: str) -> None:
for i in range(50):
trace.append((name, i))
await trio.sleep(0)
await trio.lowlevel.checkpoint()

async with trio.open_nursery() as nursery:
for i in range(5):
Expand Down
8 changes: 5 additions & 3 deletions src/trio/_tests/test_threads.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
import pytest
import sniffio

from trio import lowlevel

from .. import (
CancelScope,
CapacityLimiter,
Expand Down Expand Up @@ -165,7 +167,7 @@ def external_thread_fn() -> None:
thread.start()
print("waiting")
while thread.is_alive():
await sleep(0.01)
await lowlevel.checkpoint()
Copy link
Contributor

@A5rocks A5rocks Feb 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's probably better to use sleep here because otherwise we'll be busy looping I think?

Like this isn't a matter of "trio just needs another cycle or two of the available tasks" this is "we need to wait for the thread to start and do its stuff"

This happens 2 other places too.

Copy link
Member

@jakkdl jakkdl Feb 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is also a bug in ruffs implementation, flake8-trio only triggers when the literal is exactly 0

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wait no, ruff gave TRIO110, which suggests to await an Event instead of looping a sleep. Resolving that by replacing with a checkpoint is ... definitely not the way to go.
Maybe I should widen TRIO110 to also catch checkpoints to make sure people don't confuse them and thinks that because it stops warning on checkpoint that it "solves" it

Copy link
Contributor Author

@CoolCat467 CoolCat467 Feb 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I manually changed the 0.02 to a checkpoint because an error was being raised from the sleep there, mostly because the Event part didn't make a lot of sense to me. Should have just done a noqa really.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah an Event could plausibly not be possible or make sense in a context, in which case noqa would be the solution :)

print("waited, joining")
thread.join()
print("done")
Expand Down Expand Up @@ -535,7 +537,7 @@ async def run_thread(event: Event) -> None:
# check below won't fail due to scheduling issues. (It could still
# fail if too many threads are let through here.)
while state.parked != MAX or c.statistics().tasks_waiting != MAX:
await sleep(0.01) # pragma: no cover
await lowlevel.checkpoint() # pragma: no cover
# Then release the threads
gate.set()

Expand All @@ -546,7 +548,7 @@ async def run_thread(event: Event) -> None:
# finish before checking that all threads ran. We can do this
# using the CapacityLimiter.
while c.borrowed_tokens > 0:
await sleep(0.01) # pragma: no cover
await lowlevel.checkpoint() # pragma: no cover

assert state.ran == COUNT
assert state.running == 0
Expand Down
Loading