Skip to content

Fix base_tr fixture dependency#810

Merged
podkidyshev merged 1 commit intomainfrom
ipod/780-base-tr-fixture-deps
Feb 17, 2026
Merged

Fix base_tr fixture dependency#810
podkidyshev merged 1 commit intomainfrom
ipod/780-base-tr-fixture-deps

Conversation

@podkidyshev
Copy link
Contributor

Summary

Implemets #780

Makes sure base_tr fixture doesn't use slurm_system as a dependency

Test Plan

Unit-tests are sufficient

Additional Notes

@podkidyshev podkidyshev changed the title fix base_tr fixture dependency Fix base_tr fixture dependency Feb 17, 2026
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 17, 2026

Greptile Summary

Removes the slurm_system dependency from the base_tr test fixture. The fixture now uses tmp_path directly instead of accessing the output path through slurm_system. This makes the fixture more flexible and system-agnostic, as base_tr is a general-purpose test fixture that should work with any system type (Slurm, Kubernetes, RunAI, Standalone) without requiring Slurm-specific configuration.

Changes

  • Changed base_tr fixture parameter from slurm_system: SlurmSystem to tmp_path: Path
  • Updated output_path construction from slurm_system.output_path / "tr-name" to tmp_path / "output" / "tr-name"

Confidence Score: 5/5

  • This PR is safe to merge with no risks
  • The change is a simple, well-contained refactoring that removes an unnecessary dependency. The fixture now uses tmp_path directly, which is cleaner and makes the fixture system-agnostic. All usage patterns remain compatible since tests only access base_tr.output_path, which continues to work correctly.
  • No files require special attention

Important Files Changed

Filename Overview
tests/conftest.py Removed unnecessary slurm_system dependency from base_tr fixture, making it system-agnostic by using tmp_path instead

Last reviewed commit: 747d345

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

1 file reviewed, no comments

Edit Code Review Agent Settings | Greptile

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 17, 2026

No actionable comments were generated in the recent review. 🎉


📝 Walkthrough

Walkthrough

The base_tr pytest fixture in tests/conftest.py was refactored to accept a Path parameter (tmp_path) instead of a SlurmSystem parameter. The fixture now derives the TestRun's output path from the provided temporary path rather than from the SlurmSystem instance, decoupling the fixture from SlurmSystem dependency.

Changes

Cohort / File(s) Summary
Pytest Fixture Refactoring
tests/conftest.py
Updated base_tr fixture signature to accept tmp_path: Path instead of slurm_system: SlurmSystem, with TestRun output_path now derived from tmp_path rather than SlurmSystem.output_path.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Poem

🐰 A fixture once bound to the Slurm's heavy chain,
Now dances through tmp_path, light and plain,
No system required, just paths it inspired,
My tests hop with freedom gained! 🐇✨

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: removing the slurm_system dependency from the base_tr fixture.
Description check ✅ Passed The description is related to the changeset, referencing issue #780 and the specific fixture dependency change being implemented.

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

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch ipod/780-base-tr-fixture-deps

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.


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.

@podkidyshev podkidyshev merged commit d761162 into main Feb 17, 2026
5 checks passed
@podkidyshev podkidyshev deleted the ipod/780-base-tr-fixture-deps branch February 17, 2026 10:43
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

Comments