Skip to content

fix(hook): rewrite pytest under uv run - #2416

Merged
aeppling merged 1 commit into
rtk-ai:developfrom
BrokkAi:fix/hook-uv-run-pytest
Jun 18, 2026
Merged

aeppling merged 1 commit into
rtk-ai:developfrom
BrokkAi:fix/hook-uv-run-pytest

Conversation

@jbellis

@jbellis jbellis commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Summary

Teach hook rewrite detection to preserve uv run for the two common pytest forms:

  • uv run pytest ...
  • uv run python -m pytest ...

These now rewrite to:

  • uv run rtk pytest ...

[This covers almost 10% of the "uv run" commands that Codex has executed on my machine]

Test plan

  • [X ] cargo fmt --all && cargo clippy --all-targets && cargo test

Added/kept coverage for:

  • uv run pytest ...
  • PYTHONPATH=. uv run pytest ...
  • uv run python -m pytest ...
  • non-pytest uv run ... commands remain unrevised
  • unsupported uv run option-bearing forms remain unrevised

@aeppling aeppling self-assigned this Jun 16, 2026
@aeppling

Copy link
Copy Markdown
Contributor

Hey @jbellis !

Thanks for this PR, clean direction and code, lgtm.

Possible enhancement

Generalize the allowlist

Right now strip_uv_run_pytest_prefix hardcodes the two pytest forms. But uv run is conceptually the same as the wrappers RTK already strips generically (poetry run, bundle exec, docker exec, the doc comment in this file even lists them): strip the prefix, recurse, re-prepend if the inner command rewrote.

If uv run is handled by that same mechanism instead of a pytest-specific helper, every tool RTK already filters gets covered under uv run for free

@aeppling

aeppling commented Jun 18, 2026 •

Copy link
Copy Markdown
Contributor

Hey @jbellis ready to merge , just waiting in case you want to go a little further with the review.

Waiting for your call on this !

@jbellis
jbellis force-pushed the fix/hook-uv-run-pytest branch from 17f0f59 to 34441dd Compare June 18, 2026 08:53
@jbellis

jbellis commented Jun 18, 2026

Copy link
Copy Markdown
Contributor Author

Wow, apologies for missing this, that's a much better solution. Force-pushed rather than cluttering the history.

@aeppling

Copy link
Copy Markdown
Contributor

@jbellis All look good to me

Thanks for this clean contribution !

Will be available in next release, should land by the end of the week

@aeppling
aeppling merged commit c8722bd into rtk-ai:develop Jun 18, 2026
11 checks passed
@aeppling aeppling mentioned this pull request Jun 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants