test: passing script

[cmeesters]

This PR creates a new test case for PR #379 - ought to work only, once the jobstep-plugin release is through bioconda.

Summary by CodeRabbit

• Tests
  • Added integration tests for Slurm executor configurations to improve test coverage and reliability.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

A new test file is added to verify the Slurm executor with the pass_command_as_script feature enabled. The test class TestPassCommandAsScript configures the Slurm executor with this setting and an initialization delay for status checks.

Changes

Cohort / File(s)	Summary
Test for pass_command_as_script feature tests/test_pass_command_as_script.py	New test class TestPassCommandAsScript that configures Slurm executor with pass_command_as_script=True and verifies executor initialization. Includes get_executor() returning "slurm" and get_executor_settings() returning ExecutorSettings(pass_command_as_script=True, init_seconds_before_status_checks=2).

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Possibly related PRs

• feat: pass a shell script to sbatch rather than the command directly #380: Implements the pass_command_as_script executor setting for the Slurm plugin that this test exercises.

Poem

🐰 A test hops into place so clean,
With scripts passed on, a welcome scene!
The Slurm executor dances with delight,
Commands flow faster, day and night! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check	❓ Inconclusive	The title 'test: passing script' is vague and generic, using non-descriptive terms that don't clearly convey what specific test functionality or behavior is being added.	Use a more descriptive title that specifies the test purpose, such as 'test: add integration test for pass_command_as_script with Slurm executor' to clearly indicate what is being tested.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ 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 test/passing_script

---

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ccde8ca and 03aef11.

📒 Files selected for processing (1)

• tests/test_pass_command_as_script.py (1 hunks)

🧰 Additional context used

🧠 Learnings (2)

📓 Common learnings

Learnt from: cmeesters
Repo: snakemake/snakemake-executor-plugin-slurm PR: 0
File: :0-0
Timestamp: 2025-01-13T09:54:22.950Z
Learning: PR #173 (adding gres resource specification) depends on PR #28 in snakemake-executor-plugin-slurm-jobstep repository, as changes were required in the cpu-settings function of the jobstep-Executor module.

Learnt from: cmeesters
Repo: snakemake/snakemake-executor-plugin-slurm PR: 187
File: .github/workflows/post_to_mastodon.yml:0-0
Timestamp: 2025-01-17T17:27:32.446Z
Learning: In the mastodon publishing workflow for snakemake-executor-plugin-slurm, the PR_TITLE environment variable is required by the post_to_mastodon.sh script and should be preserved.

Learnt from: johanneskoester
Repo: snakemake/snakemake-executor-plugin-slurm PR: 173
File: docs/further.md:96-96
Timestamp: 2025-03-10T15:20:51.829Z
Learning: PR #173 in snakemake-executor-plugin-slurm implements GPU job support by adding resources: `gres` for generic resource specifications (e.g., 'gpu:1'), `gpu`/`gpus` for specifying GPU counts, and `gpu_model`/`gpu_manufacturer` for specifying GPU types, allowing users to request GPU resources directly rather than having to use slurm_extra.

Learnt from: cmeesters
Repo: snakemake/snakemake-executor-plugin-slurm PR: 193
File: .github/workflows/post_to_mastodon.yml:26-26
Timestamp: 2025-01-20T09:13:26.443Z
Learning: In the snakemake-executor-plugin-slurm repository, release PRs follow the naming pattern "chore(main): release X.Y.Z" where X.Y.Z is the version number.

Learnt from: cmeesters
Repo: snakemake/snakemake-executor-plugin-slurm PR: 221
File: tests/tests.py:62-70
Timestamp: 2025-06-24T12:41:31.837Z
Learning: In the snakemake-executor-plugin-slurm efficiency report feature, the executor is responsible for creating the efficiency_report_path directory when generating efficiency reports. Tests should not manually check for directory existence but should rely on the executor to properly create necessary directories as part of its normal operation.

📚 Learning: 2025-09-09T15:55:55.669Z

Learnt from: cmeesters
Repo: snakemake/snakemake-executor-plugin-slurm PR: 0
File: :0-0
Timestamp: 2025-09-09T15:55:55.669Z
Learning: In the Snakemake SLURM executor plugin, Snakemake submits itself as a job using the --wrap parameter in the sbatch command, rather than creating or submitting a separate job.sh script file. The actual implementation uses `call += f' --wrap="{exec_job}"'` where exec_job is the formatted snakemake execution command.

Applied to files:

• tests/test_pass_command_as_script.py

🧬 Code graph analysis (1)

tests/test_pass_command_as_script.py (1)

snakemake_executor_plugin_slurm/__init__.py (1)

• ExecutorSettings (129-302)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: testing

🔇 Additional comments (3)

tests/test_pass_command_as_script.py (3)

1-4: Imports and base types are minimal and appropriate

The imports cleanly pull in just the test harness and settings base + concrete type needed for this integration test; no unused or redundant imports here.

---

7-17: Test class structure and naming clearly convey intent

Using TestPassCommandAsScript with TestWorkflowsLocalStorageBase, __test__ = True, and get_executor returning "slurm" matches the existing testing pattern and makes the scenario under test obvious.

---

18-23: Executor settings correctly target the pass_command_as_script path

Returning a concrete ExecutorSettings with pass_command_as_script=True and a reduced init_seconds_before_status_checks=2 focuses this test on the new stdin-script submission behavior while keeping other defaults intact and validation-friendly. This aligns well with the prior behavior that used sbatch --wrap and now needs explicit coverage for the alternative script-via-stdin path. Based on learnings, this looks consistent with how the executor is intended to be configured for this feature.

Please double‑check against the current ExecutorSettings docs that no additional flags are required for the jobstep plugin’s pass‑as‑script mode, and that a 2‑second init delay is stable on your CI Slurm test cluster.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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