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

fix os.symlink call for gather_templates --structure_paths - use abspath #74

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

rafwiewiora
Copy link
Member

Fixes #73

(that's adding a trailing new line at the end in the diff)

@jchodera
Copy link
Member

jchodera commented Apr 8, 2016

Can you add a corresponding test to catch the original problem this was designed to solve?

@jchodera
Copy link
Member

jchodera commented Apr 8, 2016

To get the travis tests to run, you'll have to change this line from Miniconda-latest-Linux to Miniconda2-latest-Linux.

@jchodera
Copy link
Member

jchodera commented Apr 8, 2016

Nevermind, I fixed this in #75

@rafwiewiora
Copy link
Member Author

re test: there was one already (https://github.com/choderalab/ensembler/blob/master/ensembler/tests/test_initproject.py#L101) but that uses the absolute path, hence works.

If I want to test using a relative path - do I have guarantee that the current folder will always be the folder in which the test script sits? Not sure how Travis works with that kind of thing.

@jchodera
Copy link
Member

If I want to test using a relative path - do I have guarantee that the current folder will always be the folder in which the test script sits? Not sure how Travis works with that kind of thing.

I would write a test that does the following:

  • Create a temporary directory with tempfile.mkdtemp()
  • Store the original (current) working directory
  • Change to the temporary directory
  • copy from structure_paths = [get_installed_resource_filename(os.path.join('tests', 'resources'))]
  • run the test using the local directory
  • Make sure to restore the original working directory even if an exception is thrown

@rafwiewiora
Copy link
Member Author

Thanks! Test added.

@jchodera
Copy link
Member

We're still working on fixing tests---sorry! See #80

@rafwiewiora
Copy link
Member Author

No problem!

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