-
Notifications
You must be signed in to change notification settings - Fork 29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add missing handover keyword #117
Conversation
Codecov Report
@@ Coverage Diff @@
## master #117 +/- ##
=======================================
Coverage 98.21% 98.21%
=======================================
Files 12 12
Lines 951 951
Branches 174 174
=======================================
Hits 934 934
Misses 10 10
Partials 7 7
Flags with carried forward coverage won't be shown. Click here to find out more.
|
I've squashed all of the commits starting with "test". Basically, the snakefmt/tests/test_snakefmt.py Lines 56 to 70 in 91603a5
was actually ignored, and thus we didn't reformat it and the test failed. I fixed this by explicitly setting the tempdir for that test to |
tests/test_snakefmt.py
Outdated
@@ -54,14 +54,14 @@ def test_stdinAsSrc_WritesToStdout(self, cli_runner): | |||
assert actual.output == expected_output | |||
|
|||
def test_src_dir_arg_files_modified_inplace(self, cli_runner): | |||
with tempfile.TemporaryDirectory() as tmpdir: | |||
with tempfile.TemporaryDirectory(dir="/tmp") as tmpdir: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you remind me why this is being done?
What if /tmp
does not exist on the host machine the tests run on? Running this with dir="/not_there"
fails with FileNotFoundError
on my machine
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's basically trying to create a directory structure to test that we are searching for snakefiles and ignoring things that should be ignored etc.
I used /tmp
as it seems likely to exist in most places and will be auto cleaned up by the OS usually. I can't use the normal temp dir because as I mentioned above, the Mac CI tests fail.
Happy to change the behaviour to something else though if you have a better idea?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would a good alternative be the cwd? So like .
, as that is guaranteed to exist?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
everything else looks fine to me so approved
Part of the reason #116 was failing tests was a missing
handover
keyword that was added insnakemake
v6.2.0. I've added that in and updated the lock file due to some other issues alluded to in #116I've also pre-emptively bumped the version so that when this is merged onto master we can just tag a release on github directly.