Skip to content

fix rx-shout integration test for new uv project structure#6470

Merged
masenf merged 9 commits intomainfrom
masenf/rx-shout-test
May 8, 2026
Merged

fix rx-shout integration test for new uv project structure#6470
masenf merged 9 commits intomainfrom
masenf/rx-shout-test

Conversation

@masenf
Copy link
Copy Markdown
Collaborator

@masenf masenf commented May 7, 2026

No description provided.

@masenf masenf requested a review from a team as a code owner May 7, 2026 21:35
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 7, 2026

Greptile Summary

This PR updates the rx-shout integration test job to work with the new uv-based pyproject.toml project structure (replacing the old requirements.txt flow), and removes the unconditional psutil dependency from wait_for_listening_port.py on non-Windows platforms.

  • Integration test job: Instead of stripping a reflex== pin from requirements.txt and running uv pip install, the job now appends a [tool.uv.sources] override to pyproject.toml and calls uv sync --prerelease=allow, then uses uv run --project ... --no-sync to launch the integration script.
  • _pid_exists helper: On Linux/macOS the function now uses the standard POSIX os.kill(pid, 0) pattern with ProcessLookupError/PermissionError handling, only falling back to psutil on Windows where os.kill accepts only CTRL_*_EVENT signals.

Confidence Score: 4/5

The workflow logic is sound but a stray tab character on line 190 of the YAML file could prevent the workflow from parsing correctly under a strict YAML parser.

The approach of patching pyproject.toml and using uv sync is correct for the new project structure, and the os.kill change in wait_for_listening_port.py is a clean POSIX improvement. The one concern is the tab-indented comment on line 190 of integration_tests.yml — it is invalid per the YAML spec, and if GitHub's parser ever tightens up or a pre-commit lint step runs, the entire workflow file would be rejected.

.github/workflows/integration_tests.yml — line 190 has a tab character where spaces are required by the YAML spec.

Important Files Changed

Filename Overview
.github/workflows/integration_tests.yml Rewrites the rx-shout integration job to use uv's pyproject.toml/sources model; one line has a tab character for indentation which is invalid YAML.
scripts/wait_for_listening_port.py Replaces unconditional psutil usage with os.kill(pid, 0) on non-Windows, eliminating the psutil runtime dependency on Linux/macOS CI runners.

Reviews (2): Last reviewed commit: "remove long comment" | Re-trigger Greptile

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented May 7, 2026

Merging this PR will not alter performance

✅ 24 untouched benchmarks
⏩ 2 skipped benchmarks1


Comparing masenf/rx-shout-test (f7d442c) with main (2df5344)

Open in CodSpeed

Footnotes

  1. 2 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

masenf and others added 7 commits May 7, 2026 14:58
avoid case of mixed installation from local/pypi for
reflex subpackages
os.kill(pid, 0) is the stdlib equivalent everywhere except Windows,
where psutil is already a transitive runtime dep of reflex. Drops the
manual uv pip install psutil from the rx-shout job, which was landing
in the wrong venv anyway because setup-uv activates the repo-root venv.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
because we're installing reflex in an editable way, it will use the local
specification's from reflex's pyproject.toml when it installs.
@adhami3310
Copy link
Copy Markdown
Member

@greptile

Comment thread .github/workflows/integration_tests.yml Outdated
@masenf masenf merged commit a10252b into main May 8, 2026
68 of 69 checks passed
@masenf masenf deleted the masenf/rx-shout-test branch May 8, 2026 00:11
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