Skip to content

test(runs): isolate runner tests from ACTL env#252

Merged
xraymemory merged 4 commits into
mainfrom
fix/runs-test-actl-env
Jun 2, 2026
Merged

test(runs): isolate runner tests from ACTL env#252
xraymemory merged 4 commits into
mainfrom
fix/runs-test-actl-env

Conversation

@xraymemory

@xraymemory xraymemory commented Jun 2, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • clear inherited ACTL/runtime pixi policy env vars in runner test fixture
  • keep runner argv tests deterministic with SAMPLEWORKS_FORCE_PIXI only

Summary by CodeRabbit

  • Tests
    • Enhanced environment cleanup in test fixtures to ensure more deterministic and reliable test assertions across different environments.

Copilot AI review requested due to automatic review settings June 2, 2026 15:33
@coderabbitai

coderabbitai Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@xraymemory, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 59 minutes and 47 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 10000b79-25c1-4c30-be29-9c8d2fa1e7fd

📥 Commits

Reviewing files that changed from the base of the PR and between 9a2f6a2 and 9583cc5.

📒 Files selected for processing (2)
  • tests/runs/conftest.py
  • tests/runs/test_runner.py
📝 Walkthrough

Walkthrough

The force_pixi_argv pytest fixture in tests/runs/conftest.py was enhanced to centralize and broaden environment variable cleanup. The fixture now removes a comprehensive set of Pixi and runtime-related environment variables using a loop, ensuring deterministic test conditions across environments, with updated documentation.

Changes

Test Fixture Cleanup

Layer / File(s) Summary
Expand env-var cleanup in force_pixi_argv
tests/runs/conftest.py
The fixture docstring is updated and environment variable cleanup is broadened to remove SAMPLEWORKS_* and related RUNTIME_PIXI/runtime-prebuilt/prepare-skip variables in a single loop via monkeypatch.delenv(..., raising=False) for more deterministic test assertions.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A fixture cleaned with gentle care,
Env vars swept from here and there,
No pixi remnants shall remain,
Tests run pure, without a stain!
Hopping forward, tests all pass,
Clean and clear, a pristine class.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'test(runs): isolate runner tests from ACTL env' clearly and specifically describes the main change: isolating runner tests from ACTL environment variables, which aligns with the PR objective of clearing inherited ACTL/runtime pixi environment variables to make tests deterministic.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/runs-test-actl-env

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR makes the tests/runs suite deterministic across developer machines and ACTL image environments by clearing inherited pixi/runtime policy environment variables in an autouse test fixture.

Changes:

  • Updates the runner test autouse fixture to delete ACTL/runtime pixi policy env vars (RUNTIME_PIXI, SAMPLEWORKS_ALLOW_RUNTIME_PIXI, SAMPLEWORKS_REQUIRE_PREBUILT_PIXI, SAMPLEWORKS_SKIP_ENV_PREPARE).
  • Keeps argv-related runner tests deterministic by relying only on SAMPLEWORKS_FORCE_PIXI=1 from the fixture.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@marcuscollins marcuscollins left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@manzuoni-astera

Copy link
Copy Markdown
Contributor

Found the remaining environment-dependent failure: on ACTL images where /home/dev/workspace/run_grid_search.py exists, the runner intentionally prefers that synced workspace script, so test_uses_baked_env_python_when_available could see /home/dev/workspace/run_grid_search.py instead of /app/run_grid_search.py. I pushed 6989aa0 to make the tests isolate WORKSPACE_GRID_SEARCH_SCRIPT to a guaranteed-missing temp path. Local lightweight checks passed; full pixi test run is unavailable on my machine because pixi is not installed.

@manzuoni-astera

Copy link
Copy Markdown
Contributor

Found the failure in https://github.com/diff-use/sampleworks/actions/runs/26837160368/job/79134059232: test_pre_jobs_run_before_main_jobs inherited SAMPLEWORKS_FORCE_PIXI=1 from the runner test fixture, so its SAMPLEWORKS_ANALYSIS_PYTHON=sys.executable override was ignored. That made runner.run() call pixi run -e analysis python -c "print(ready)", and the CI runner ran out of space populating /tmp/pixi-cache (No space left on device while downloading narwhals). Pushed 25b3b65 so this test clears SAMPLEWORKS_FORCE_PIXI and uses the explicit Python override instead of preparing a runtime pixi env.

@xraymemory xraymemory merged commit 0d20343 into main Jun 2, 2026
8 of 11 checks passed
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.

4 participants