Conversation
…installs of HDF5.
…l QE 6.4.1. converter with serial HDF5.
ffeecdb to
9c80758
Compare
|
Need to replace |
| @run_after('build') | ||
| @on_package_attributes(run_tests=True) | ||
| def check(self): | ||
| def check_install(self): |
There was a problem hiding this comment.
Why was this renamed? Actually, CMakePackage already runs check by default during test time, so if you change it back to check you should be able to remove the run_after and on_package_attributes.
There was a problem hiding this comment.
check did not seem to do anything anymore, so I changed it to check_install because I grepped through some packages and found an example similar to the one I had.
You are saying to get rid of the decorators and just do check and it will run after the build -- and if the build fails, it will not install. correct?
There was a problem hiding this comment.
Correct. You can test this with spack install --test=root qmcpack to make sure I'm right.
There was a problem hiding this comment.
Unfortunately, your suggested modifications are not working. check is doing nothing.
There was a problem hiding this comment.
I'm having trouble reproducing this. I tried messing with a simpler package with fewer dependencies. I tried building with and without overriding check, and with and without extending CudaPackage, but it always builds tests when I ask it to. What version of Python are you using to run Spack?
There was a problem hiding this comment.
--test=root and --run-tests should be equivalent, are you testing this on develop? If you want to run both make check and your custom unit tests, your current approach is fine, or you can add make check to your custom function and override the default check.
There was a problem hiding this comment.
I also tried combining the decorators with the custom check method and it successfully overrides the builtin check method when I use --test=root. It actually runs the tests twice, which is interesting behavior.
There was a problem hiding this comment.
--test=rootand--run-testsshould be equivalent, are you testing this on develop? If you want to run bothmake checkand your custom unit tests, your current approach is fine, or you can addmake checkto your custom function and override the defaultcheck.
yes
There was a problem hiding this comment.
Make sure you're using --test=root and not --test-root
There was a problem hiding this comment.
It does not seem to be working that way for me. If I rename the method to check instead of check_install and use --test=root or --run-tests the custom check (with the decorates present) is not run.
Just merge 'AS IS.' If I can narrow down the problem with a simple reproducer, I will file a github issue.
|
I can merge this PR as is if you want, but if there's a bug in our |
Mostly a minor update: