-
Notifications
You must be signed in to change notification settings - Fork 75
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
NetCDF failing in Windows tests, blocking merging of PRs #527
Comments
For now I'm going to disable the windows tests as a requirement so we can get some of these PRs merged in. |
Lnaden
added a commit
to Lnaden/openmmtools
that referenced
this issue
Dec 13, 2021
This PR attempts to fix the issue raised in choderalab#527. From what I can tell, the tests where this is thrown always fails exactly at the same point. This PR closes the reporter file formally before the failing line occurs, so hopefully this will correctly allow Window's memory to release the file.
5 tasks
mikemhenry
added a commit
that referenced
this issue
Dec 14, 2021
This PR attempts to fix the issue raised in #527. From what I can tell, the tests where this is thrown always fails exactly at the same point. This PR closes the reporter file formally before the failing line occurs, so hopefully this will correctly allow Window's memory to release the file. Co-authored-by: Mike Henry <11765982+mikemhenry@users.noreply.github.com>
I think we can close this one. Solved in #535 . |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
For months our CI tests have been failing specifically with the windows tests complaining about issues with accessing netcdf files, such as in https://github.com/choderalab/openmmtools/runs/4155555651?check_suite_focus=true with messages like
and
So, I guess we have to ask ourselves if we want to keep supporting Windows and if so, then we want to set up environments that allow us to test this locally. Personally I use Linux as my development OS.
This is now blocking some of our current workflow to merge PRs, because we would have to manually override the windows tests if we want things to be merged.
The text was updated successfully, but these errors were encountered: