test: symlink to directory "has older modification time" than target#3782
Closed
thekswenson wants to merge 1 commit into
Closed
test: symlink to directory "has older modification time" than target#3782thekswenson wants to merge 1 commit into
thekswenson wants to merge 1 commit into
Conversation
Contributor
📝 WalkthroughWalkthroughAdds a new Snakemake test workflow that creates a directory with a file and a symlinked directory, plus an expected-results artifact representing the symlink target path. Changes
Sequence Diagram(s)sequenceDiagram
participant Snakemake
participant rule_all as rule all (dir2/D)
participant link_to_D as rule link_to_D (dir2/D → ../dir1/D)
participant make_file as rule make_file (dir1/D, dir1/D/file.txt)
Snakemake->>rule_all: Build target dir2/D
rule_all->>link_to_D: Require input dir1/D
link_to_D->>make_file: Require output dir1/D
make_file-->>link_to_D: Create dir1/D and dir1/D/file.txt
link_to_D-->>rule_all: Create symlink dir2/D → ../dir1/D
rule_all-->>Snakemake: Target satisfied
note over link_to_D: Symlink created via Path(...).symlink_to("../dir1/D")
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/test_github_issue3687/Snakefile (2)
1-1: Remove unused import.The
shutilmodule is imported but never used in this workflow.Apply this diff to remove the unused import:
-import shutil from pathlib import Path
28-28: Prefer Path operations for constructing the symlink target.The current string formatting approach (
f'../{input}') is fragile. Consider using Path operations for more robust path construction.Apply this diff to use Path operations:
- Path(str(output)).symlink_to(f'../{input}') + target = Path('..') / str(input) + Path(str(output)).symlink_to(target)Or, for an even more explicit approach:
- Path(str(output)).symlink_to(f'../{input}') + # Create symlink with relative path from dir2/D to dir1/D + target = Path('..').joinpath(str(input)) + Path(str(output)).symlink_to(target)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tests/test_github_issue3687/Snakefile(1 hunks)tests/test_github_issue3687/expected-results/dir2/D(1 hunks)
🔇 Additional comments (3)
tests/test_github_issue3687/Snakefile (2)
4-6: LGTM!The
rule allcorrectly specifies the final target.
9-16: Verify directory outputs are created before therun:blockPlease run this simplified script in
tests/test_github_issue3687:#!/bin/bash # Clean any previous output rm -rf dir1 # Execute workflow quietly snakemake --cores 1 --forceall --quiet || exit 1 # Check existence of directory and file if [[ -d dir1/D && -f dir1/D/file.txt ]]; then echo "SUCCESS: Snakemake created the directory and file before run block" else echo "ERROR: Parent directory not created automatically" exit 1 fiIf you see the error, add before touching:
run: + Path(output.file).parent.mkdir(parents=True, exist_ok=True) Path(output.file).touch()tests/test_github_issue3687/expected-results/dir2/D (1)
1-1: LGTM!The expected result correctly specifies the symlink target path that matches the Snakefile's
link_to_Drule output.
Comment on lines
+19
to
+28
| rule link_to_D: | ||
| """ Create a symlink to dir1/D in dir2. """ | ||
| input: | ||
| 'dir1/D' | ||
|
|
||
| output: | ||
| directory('dir2/D') | ||
|
|
||
| run: | ||
| Path(str(output)).symlink_to(f'../{input}') |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Verify that parent directory exists before creating the symlink.
Path.symlink_to() will raise FileNotFoundError if the parent directory (dir2) does not exist. Verify that Snakemake creates the parent directory before executing the run: block. If not, it must be created explicitly.
Run the following script to confirm whether the parent directory is created automatically:
If the verification fails, apply this diff to create the parent directory explicitly:
run:
+ Path(str(output)).parent.mkdir(parents=True, exist_ok=True)
Path(str(output)).symlink_to(f'../{input}')🏁 Script executed:
#!/bin/bash
# Description: Test if the symlink creation executes without errors
# Navigate to test directory and attempt to run the workflow
cd tests/test_github_issue3687
snakemake --cores 1 --forceall 2>&1 | tee execution.log
# Check if there are FileNotFoundError exceptions during symlink creation
if grep -q "FileNotFoundError.*dir2" execution.log; then
echo "ERROR: Parent directory dir2 not created automatically"
exit 1
else
echo "SUCCESS: Symlink created without FileNotFoundError"
fiLength of output: 139
🏁 Script executed:
#!/bin/bash
cd tests/test_github_issue3687
# Run workflow and capture all output
snakemake --cores 1 --forceall > execution.log 2>&1 || true
# Check for FileNotFoundError
if grep -q "FileNotFoundError" execution.log; then
echo "ERROR: Parent directory dir2 not created automatically"
exit 1
else
echo "No FileNotFoundError detected."
fi
# Inspect resulting filesystem
echo "Contents of test_github_issue3687:"
ls -R .
echo
echo "Check 'dir2' existence:"
if [ -d dir2 ]; then
echo "dir2 exists"
else
echo "dir2 missing"
fi
echo
echo "Check 'dir2/D':"
if [ -L dir2/D ]; then
echo "dir2/D exists and is a symlink -> $(readlink dir2/D)"
else
if [ -d dir2/D ]; then
echo "dir2/D exists as a directory"
else
echo "dir2/D missing"
fi
fiLength of output: 303
🏁 Script executed:
#!/bin/bash
# Inspect Snakefile for working directory or path prefixes
grep -nE "workdir|configfile" tests/test_github_issue3687/Snakefile || echo "No workdir/configfile directives"
echo "----- Snakefile content -----"
sed -n '1,200p' tests/test_github_issue3687/SnakefileLength of output: 1486
Pre-create parent dir2 and use pathlib for symlink target
- In the
run:block, before callingsymlink_to, add:out = Path(output[0]) out.parent.mkdir(parents=True, exist_ok=True)
- Replace
Path(str(output)).symlink_to(f'../{input}')with:out.symlink_to(Path(input[0]).relative_to(out.parent))
🤖 Prompt for AI Agents
In tests/test_github_issue3687/Snakefile around lines 19 to 28, the run block
currently calls symlink_to without ensuring the parent directory exists and uses
string interpolation for the target; change it to create a Path for output[0],
call mkdir(parents=True, exist_ok=True) on its parent to pre-create dir2, build
the symlink target as the Path of input[0] made relative to the output parent,
and call symlink_to with that Path so the symlink is created reliably and
portably.
Contributor
Author
|
I'm closing this because I realize that I put it on the main branch. Will create a PR with a new branch now. |
johanneskoester
added a commit
that referenced
this pull request
Jun 1, 2026
…3782) (#3784) <!--Add a description of your PR here--> This is a test case for the problem described in #3687. ### 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. * [ ] 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 an integration test that verifies a workflow creating a file and a symlinked directory target, ensuring reruns are triggered when inputs change (skipped on Windows). * **Bug Fixes** * Improved handling of directory vs. symlink timestamps and touch behavior, and clarified related error messages for more reliable filesystem workflows. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Johannes Köster <johannes.koester@tu-dortmund.de> Co-authored-by: Filipe G. Vieira <1151762+fgvieira@users.noreply.github.com> Co-authored-by: hwrn <hwrn.aou@sjtu.edu.cn> Co-authored-by: Aoran Hu <48747984+Hocnonsense@users.noreply.github.com>
johanneskoester
pushed a commit
that referenced
this pull request
Jun 1, 2026
🤖 I have created a release *beep* *boop* --- ## [9.22.0](v9.21.1...v9.22.0) (2026-06-01) ### Features * add semantic helper functions choose_file/choose_folder/choose_tmp and allow all semantic helper functions to be used when parsing resources ([#3820](#3820)) ([d3c2386](d3c2386)) * add temp to default pathvars ([#4108](#4108)) ([917fb11](917fb11)) * add workflow info to log ([#4079](#4079)) ([e40e15b](e40e15b)) * support python 3.14 ([#3739](#3739)) ([7e3be0c](7e3be0c)) ### Bug Fixes * Adapt for dataclasses._MISSING_TYPE replaced with sentinel in Python 3.15 ([#4211](#4211)) ([b7fb8df](b7fb8df)) * ensure that storage is not actually retrieved with --touch ([#4212](#4212)) ([2726cae](2726cae)) * symlink to directory "has older modification time" than target ([#3782](#3782)) ([#3784](#3784)) ([41903d8](41903d8)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a test case for the problem described in #3687.
QC
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