Skip to content

fix: adjust conda tests to not fail on apple silicon; fix #4040#4049

Merged
johanneskoester merged 5 commits intosnakemake:mainfrom
edbennett:4040-conda-test-compatibility
Mar 11, 2026
Merged

fix: adjust conda tests to not fail on apple silicon; fix #4040#4049
johanneskoester merged 5 commits intosnakemake:mainfrom
edbennett:4040-conda-test-compatibility

Conversation

@edbennett
Copy link
Copy Markdown
Contributor

@edbennett edbennett commented Mar 11, 2026

Update versions for the following tests such that packages are available on macos-arm64.

  • test_issue635
  • test_wrapper, test_wrapper_qsub, test_wrapper_local_git_prefix

Where the test behaviour depends on a specific old version, or where no version of a package is available, skip this test:

  • test_script (Julia is not in standard Conda channels for macOS-arm64; juliaup is, but there's no way to pin a specific version.)
  • test_conda_python_3_7_script
  • test_script_pre_py39

In one more case, that the test wasn't already skipped seems erroneous: test_conda_pin_file had the comment "the testcase only has a linux-64 pin file", but was only skipped on Windows. This is now skipped on any non-Linux OS.

QC

  • The PR contains a test case for the changes or the changes are already covered by an existing test case.
  • The documentation (docs/) is updated to reflect the changes or this is not necessary (e.g. if the change does neither modify the language nor the behavior or functionalities of Snakemake).

Summary by CodeRabbit

  • Tests
    • Added platform checks and a skip marker to skip tests on macOS Apple Silicon where required Conda packages are unavailable.
    • Replaced a Windows-only gating with a Linux-only condition for a pin-file test.
    • Applied macOS ARM skips to multiple tests.
    • Switched the test wrapper to use a different compression tool.
    • Updated the R test environment to newer r-base and r-rmarkdown versions.

update versions for some tests such that packages are available on macos-arm64.
where the test behaviour depends on a specific old version,
skip this test.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 11, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Added Linux and Apple Silicon platform checks and a macOS-ARM pytest skip marker; applied macOS-ARM and Linux-specific skips to conda-related tests; bumped R package versions in a test Conda env; and switched a test Snakefile wrapper to bgzip.

Changes

Cohort / File(s) Summary
Test infra & markers
tests/conftest.py
Imported platform; added ON_LINUX and ON_APPLE_SILICON constants and a skip_on_macos_arm pytest skip marker.
Test usage updates
tests/tests_using_conda.py
Imported ON_LINUX and skip_on_macos_arm; applied skip_on_macos_arm to multiple conda-related tests and changed one test to use pytest.mark.skipif(not ON_LINUX).
Conda environment
tests/test_issue635/envs/rmarkdown.yaml
Updated Conda dependency versions: r-base 3.6.1 → 4.5.2 and r-rmarkdown 1.17 → 2.30.
Test workflow wrapper
tests/test_wrapper/Snakefile
Replaced wrapper reference for compress_vcf from 0.42.0/bio/vcf/compress to v8.1.1/bio/bgzip.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main purpose of the PR: fixing conda tests to work on Apple Silicon and addressing issue #4040.
Description check ✅ Passed The PR description is comprehensive and complete, covering the changes made and addressing all required QC checklist items with clear explanations.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/conftest.py`:
- Around line 12-17: Replace the unreliable platform.processor() check for Apple
Silicon with platform.machine(); specifically update the ON_APPLE_SILICON
constant (used by skip_on_macos_arm) to use platform.machine().lower() ==
"arm64" so macOS ARM64 (Apple Silicon) runners are correctly detected and the
pytest.mark.skipif gate skip_on_macos_arm behaves as intended.

In `@tests/tests_using_conda.py`:
- Line 270: The test decorator references an undefined symbol ON_LINUX causing a
NameError; fix by defining or importing ON_LINUX before use (or replace the
condition inline). Locate the pytest.mark.skipif(...) line and either import a
shared constant (e.g., add an import that exposes ON_LINUX) or add a
module-level definition such as computing ON_LINUX from
sys.platform/platform.system, or change the decorator to use an equivalent
expression directly (e.g., sys.platform.startswith('linux')) so
pytest.mark.skipif can evaluate correctly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 71505c6c-2eb0-424e-aa19-844d43c818fb

📥 Commits

Reviewing files that changed from the base of the PR and between 45883c5 and 7e96f2a.

📒 Files selected for processing (4)
  • tests/conftest.py
  • tests/test_issue635/envs/rmarkdown.yaml
  • tests/test_wrapper/Snakefile
  • tests/tests_using_conda.py

@edbennett edbennett force-pushed the 4040-conda-test-compatibility branch from 15bc1d1 to 7800ef4 Compare March 11, 2026 11:58
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/tests_using_conda.py`:
- Line 276: The skip condition currently uses ON_LINUX which is true for all
Linux arches; change it to require linux-64 specifically by updating the
pytest.mark.skipif to check both ON_LINUX and the x86_64 architecture (e.g.,
import platform and use platform.machine() == "x86_64") or create a helper like
ON_LINUX_64 and use that; modify the decorator on the test (the
pytest.mark.skipif line referencing ON_LINUX) to be pytest.mark.skipif(not
(ON_LINUX and platform.machine() == "x86_64"), reason="This testcase only has a
linux-64 pin file") so the test only runs on linux-64.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0387f860-942f-4b6e-a4eb-c14b505232f3

📥 Commits

Reviewing files that changed from the base of the PR and between e1e7e35 and cc8df54.

📒 Files selected for processing (1)
  • tests/tests_using_conda.py



@skip_on_windows # the testcase only has a linux-64 pin file
@pytest.mark.skipif(not ON_LINUX, reason="This testcase only has a linux-64 pin file")
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.

⚠️ Potential issue | 🟠 Major

Gate this on linux-64, not just Linux.

The reason says the fixture is linux-64 only, but ON_LINUX is also true on Linux/aarch64. That means this test would still run on Linux ARM, where the same pin-file mismatch should make it fail.

Suggested fix
+import platform
+
-@pytest.mark.skipif(not ON_LINUX, reason="This testcase only has a linux-64 pin file")
+@pytest.mark.skipif(
+    not (ON_LINUX and platform.machine() in {"x86_64", "amd64"}),
+    reason="This testcase only has a linux-64 pin file",
+)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/tests_using_conda.py` at line 276, The skip condition currently uses
ON_LINUX which is true for all Linux arches; change it to require linux-64
specifically by updating the pytest.mark.skipif to check both ON_LINUX and the
x86_64 architecture (e.g., import platform and use platform.machine() ==
"x86_64") or create a helper like ON_LINUX_64 and use that; modify the decorator
on the test (the pytest.mark.skipif line referencing ON_LINUX) to be
pytest.mark.skipif(not (ON_LINUX and platform.machine() == "x86_64"),
reason="This testcase only has a linux-64 pin file") so the test only runs on
linux-64.

@johanneskoester johanneskoester merged commit f5b0142 into snakemake:main Mar 11, 2026
110 of 112 checks passed
@johanneskoester
Copy link
Copy Markdown
Contributor

Thanks a lot!

LKress pushed a commit to LKress/snakemake that referenced this pull request Mar 11, 2026
 (snakemake#4049)

Update versions for the following tests such that packages are available
on macos-arm64.

- `test_issue635`
- `test_wrapper`, `test_wrapper_qsub`, `test_wrapper_local_git_prefix`

Where the test behaviour depends on a specific old version, or where no
version of a package is available, skip this test:

- `test_script` (Julia is not in standard Conda channels for
macOS-arm64; `juliaup` is, but there's no way to pin a specific
version.)
- `test_conda_python_3_7_script`
- `test_script_pre_py39`

In one more case, that the test wasn't already skipped seems erroneous:
`test_conda_pin_file` had the comment "the testcase only has a linux-64
pin file", but was only skipped on Windows. This is now skipped on any
non-Linux OS.

### QC
<!-- Make sure that you can tick the boxes below. -->

* [X] The PR contains a test case for the changes or the changes are
already covered by an existing test case.
* [X] The documentation (`docs/`) is updated to reflect the changes or
this is not necessary (e.g. if the change does neither modify the
language nor the behavior or functionalities of Snakemake).


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Tests**
* Added platform checks and a skip marker to skip tests on macOS Apple
Silicon where required Conda packages are unavailable.
* Replaced a Windows-only gating with a Linux-only condition for a
pin-file test.
  * Applied macOS ARM skips to multiple tests.
  * Switched the test wrapper to use a different compression tool.
* Updated the R test environment to newer r-base and r-rmarkdown
versions.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
johanneskoester pushed a commit that referenced this pull request Mar 13, 2026
🤖 I have created a release *beep* *boop*
---


##
[9.17.0](v9.16.3...v9.17.0)
(2026-03-13)


### Features

* Allow storing snakemake metadata in files or databases
([#4012](#4012))
([dd75f31](dd75f31))
* Allow to specify comparison command per-unit test
([#3956](#3956))
([b88171c](b88171c))
* job table orderd topological when run is started
([#4018](#4018))
([75cf506](75cf506))
* lambda functions for priority in rules
([#3253](#3253))
([d2aa226](d2aa226))
* Make on... directive of modules accessible
([#4050](#4050))
([e9f2e1c](e9f2e1c))


### Bug Fixes

* adjust conda tests to not fail on apple silicon; fix
[#4040](#4040)
([#4049](#4049))
([f5b0142](f5b0142))
* allow "--containerize apptainer" to output apptainer format instead of
dockerfile ([#4030](#4030))
([f5cac30](f5cac30))
* apptainer command not recognized when singularity is absent
([#4010](#4010))
([b8162e2](b8162e2))
* capture stderr when tests fail
([#3995](#3995))
([97d74ba](97d74ba))
* **docs:** make Data-dependent conditional execution a complete example
([#4043](#4043))
([3a1d7f2](3a1d7f2))
* don't build the DAG when running unlock. Fixes
[#4000](#4000) and
[#198](#198)
([#4007](#4007))
([acf79fd](acf79fd))
* Ensure pixi tasks may be run as advertised
([#4046](#4046))
([88253c2](88253c2))
* fix checkpoint handling corner cases
([#3870](#3870) and
[#3559](#3559))
([#4015](#4015))
([63f4257](63f4257))
* issue 3642
([#4054](#4054))
([76e6fc2](76e6fc2))
* issue 3815
([#4026](#4026))
([b0eec96](b0eec96))
* logging None in shellcmd context causes error
([#4064](#4064))
([d0652cd](d0652cd))
* lookup function returns default value for empty DataFrame queries
([#4056](#4056))
([f71de97](f71de97))
* make `cache: omit-software` a rule specific property
([#4085](#4085))
([034a9e7](034a9e7))
* reduce number of tests leaving temporary files behind
([#4033](#4033))
([a3a1c97](a3a1c97))
* regression in dynamic resource handling
([#4038](#4038))
([f2c554a](f2c554a))
* somewhat shorter announce message
([#4080](#4080))
([57efc71](57efc71))


### Performance Improvements

* switch reretry with tenacity; decouple container classes (with Python
3.7 compat for old scripts) from rest of the codebase (enabling moving
to newer python versions)
([#4032](#4032))
([ffb19e7](ffb19e7))


### Documentation

* Add AI-assisted contributions policy to contributing guidelines
([#4051](#4051))
([dd70526](dd70526))
* **codebase:** Update & simplify plugin architecture section
([#4052](#4052))
([176cf63](176cf63))
* Correct workflow.source_path() description in documentation
([#4036](#4036))
([45883c5](45883c5))
* fixed wrong code example for collect() function
([#4037](#4037))
([5c85ed8](5c85ed8))
* Minor docs improvements
([#4089](#4089))
([29ea226](29ea226))
* switch to sphinx_design for tabs
([#3976](#3976))
([9674614](9674614))
* typo in the migration table breaking a pip install command
([#4024](#4024))
([66f9dda](66f9dda))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
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