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

Write tests for the issue #535 fix. #1

Merged
merged 3 commits into from
May 3, 2016

Conversation

ftsamis
Copy link

@ftsamis ftsamis commented May 1, 2016

Wrote 3 tests, all of them use the tardis_configv1_ascii_density_abund.yml configuration file which uses external files for both the structure, and the abundances. Given that these files are specified as filenames without paths in the configuration, the following tests (which @yeganer proposed) should verify that the changes introduced in tardis-sn#543 work as expected.

Test if the config_reader can properly read the configuration when:

  1. The working directory is the same as the configuration file and a filename without path is provided.
  2. The working directory is the parent directory of the configuration file and a filename with a relative path is provided.
  3. The working directory is whatever it was when py.test started and a filename with an absolute path is provided.

I used no asserts since pytest considers a test failed if an exception is raised while running it, and the config_reader would raise one if it could not find any of the files. Though this is something I'm not sure if I did right.

@ftsamis
Copy link
Author

ftsamis commented May 2, 2016

Fixed Travis failing, please review!

@lukeshingles lukeshingles merged commit d831c28 into lukeshingles:fix-535 May 3, 2016
@yeganer
Copy link

yeganer commented May 4, 2016

I was about to comment on the PR but It's merged now. Please see my comment
on PR tardis-sn#543.

Luke Shingles [email protected] schrieb am Di., 3. Mai 2016 um
10:46 Uhr:

Merged #1 #1.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#1 (comment)

lukeshingles pushed a commit that referenced this pull request Oct 6, 2017
Streamline the OMP implementation
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.

3 participants