Skip to content
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

[ENH] RecentFiles: Check for missing file in workflow dir #3064

Merged
merged 1 commit into from
Jul 25, 2018

Conversation

VesnaT
Copy link
Contributor

@VesnaT VesnaT commented Jun 13, 2018

Description of changes

When opening saved workflow and file widget detects missing file, check if the file is placed in the same directory as workflow is saved.

Includes
  • Code changes
  • Tests
  • Documentation

@codecov-io
Copy link

codecov-io commented Jun 14, 2018

Codecov Report

Merging #3064 into master will increase coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #3064      +/-   ##
==========================================
+ Coverage   82.35%   82.36%   +0.01%     
==========================================
  Files         335      336       +1     
  Lines       58258    58289      +31     
==========================================
+ Hits        47979    48011      +32     
+ Misses      10279    10278       -1

Copy link
Contributor

@lanzagar lanzagar left a comment

Choose a reason for hiding this comment

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

While the travis tests pass (if you also fix one of the pylint warnings it will not complain and everything will be green),
appveyor errors out on your new unit test. Can you please check why windows have a problem with it.

@VesnaT VesnaT force-pushed the recent_path branch 3 times, most recently from 56978e4 to 10b5db9 Compare July 9, 2018 12:29
)
search_paths = [("basedir", os.getcwd())]
self.assertIsNotNone(recent_path.resolve(search_paths))
os.remove("test.tab")
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like a classic trap of removing a file at the end, which might not happen if something goes wrong before the remove statement.
And here it is actually not that unusual since you have an assert above that might fail - try running this test on master.

I think there should be some utility functions for testing on temporary files somewhere... In any case some form of finally: cleanup is needed and I suggest using python's TemporaryFiles too.

@VesnaT VesnaT force-pushed the recent_path branch 2 times, most recently from efda734 to 90ecf9a Compare July 24, 2018 11:50
@VesnaT VesnaT force-pushed the recent_path branch 2 times, most recently from b3f7b66 to 3928461 Compare July 25, 2018 09:19
When opening saved workflow and file widget detects missing file, check if the file is placed in the same directory as workflow is saved.
@lanzagar lanzagar changed the title RecentFiles: Check for missing file in workflow dir [ENH] RecentFiles: Check for missing file in workflow dir Jul 25, 2018
@lanzagar lanzagar merged commit 988e7ec into biolab:master Jul 25, 2018
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.

4 participants