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

Cleanup tests #175

Merged
merged 1 commit into from Nov 24, 2019
Merged

Cleanup tests #175

merged 1 commit into from Nov 24, 2019

Conversation

@wesbarnett
Copy link
Collaborator

wesbarnett commented Nov 22, 2019

Submission Checklist

  • Run unit tests
  • Declare copyright holder and open-source license: see below

Summary

Cleans up test data after tests are run. Fixes #174.

Alternatively a temporary directory could be used to store the files instead of this PR.

Copyright and Licensing

Please list the copyright holder for the work you are submitting (this will be you or your assignee, such as a university or company): American Express

By submitting this pull request, the copyright holder is agreeing to license the submitted work under the following licenses:

@mitzimorris

This comment has been minimized.

Copy link
Member

mitzimorris commented Nov 22, 2019

would a cleaner solution be to add a "cleanup" task to the CmdStan makefile - specifically in make/program?

note that stan-dev/cmdstan#769 fixes the Cmdstan makefiles so that in addition to the exe, .hpp, and .o files, there is also a dependencies (.d) file generated as well.

the pytest fixture could call make directly make clean-program <stan-program>

@mitzimorris

This comment has been minimized.

Copy link
Member

mitzimorris commented Nov 22, 2019

filed CmdStan issue - stan-dev/cmdstan#772

would be happy to approve this as is, but after next release, we'll need to change this.
@wesbarnett - what to do you think?

@wesbarnett

This comment has been minimized.

Copy link
Collaborator Author

wesbarnett commented Nov 22, 2019

I do think that is a much cleaner solution to have some kind of make clean-program in cmdstan as you stated that could be called. So I'm fine with this PR being merged as a temp solution or not merging it and just waiting for the cmdstan change. Either is good for me. Thanks for allowing me to contribute!

Copy link
Member

mitzimorris left a comment

lgtm

@mitzimorris mitzimorris merged commit c8c3963 into stan-dev:master Nov 24, 2019
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.