Add test phase to the SIRF project.
https://github.com/CCPPETMR/SIRF/pull/58
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.
@KrisThielemans requested changes on this pull request.
thanks. Move the tests to src/xGadgetron/pGadgetron/tests/CMakeLists.txt
(and same for STIR). That's where we know what tests are available. (add an add_directory
in the level above).
@paskino pushed 1 commit.
—
You are receiving this because you are subscribed to this thread.
View it on GitHub or mute the thread.
@KrisThielemans requested changes on this pull request.
some clean-up needed
In src/xSTIR/pSTIR/tests/CMakeLists.txt:
> +# +# This file is part of the CCP PETMR Synergistic Image Reconstruction Framework (SIRF) SuperBuild. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0.txt +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +#=========================================================================
forgot to add the actual test?
In src/xSTIR/pSTIR/CMakeLists.txt:
> @@ -40,3 +40,5 @@ if(BUILD_PYTHON) INSTALL(FILES "${CMAKE_CURRENT_BINARY_DIR}/pystir.py" pStir.py DESTINATION "${PYTHON_DEST}") endif(BUILD_PYTHON) + +ADD_SUBDIRECTORY(tests)
move inside the if, to avoid this to be included if Python-stuff not built
In src/xGadgetron/pGadgetron/tests/CMakeLists.txt:
> +# This file is part of the CCP PETMR Synergistic Image Reconstruction Framework (SIRF) SuperBuild. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0.txt +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +#========================================================================= +ENABLE_TESTING()
no need for this as in top-level (probably should be there only)
In CMakeLists.txt:
> @@ -74,15 +74,15 @@ endif() #### enable support for ctest ENABLE_TESTING() -include(FindPythonInterp) -add_test(NAME MR_FULLY_SAMPLED - COMMAND ${PYTHON_EXECUTABLE} src/xGadgetron/pGadgetron/tests/fully_sampled.py) +#include(FindPythonInterp)
I prefer not to have the commented-out stuff here...
@paskino pushed 1 commit.
—
You are receiving this because you are subscribed to this thread.
View it on GitHub or mute the thread.
@paskino pushed 2 commits.
—
You are receiving this because you are subscribed to this thread.
View it on GitHub or mute the thread.
I have moved the tests in src/xGadgetron/pGadgetron/tests/CMakeLists.txt
and src/xSTIR/pSTIR/tests/CMakeLists.txt
For some reasons I did not figure out yet, the STIR tests are not found.
@KrisThielemans commented on this pull request.
In CMakeLists.txt:
> @@ -74,6 +74,16 @@ endif() #### enable support for ctest ENABLE_TESTING() +#include(FindPythonInterp)
let's remove these comments
looks good but a few comments remaining.
no clue why it doesn't find the pStir test. sorry.
@paskino pushed 1 commit.
—
You are receiving this because you are subscribed to this thread.
View it on GitHub or mute the thread.
I checked today and it's fine. The problem I was facing yesterday was due to the fact that cmake didn't find STIR, hence the pSTIR tests were not run. It's all fine when built from the SuperBuild which sets all variables.
Once this is merged, we may merge the ctest branch on SuperBuild.
great. thanks.
Would you be able to put the enable_testing() in the main file, you've just commented it out :-; and remove it from the others? That looks a bit cleaner to me.
@paskino pushed 1 commit.
—
You are receiving this because you are subscribed to this thread.
View it on GitHub or mute the thread.
perfect. ready to accept? Prefer a merge or a squash-merge? (see email)
I guess squash-merge
@paskino pushed 1 commit.
—
You are receiving this because you are subscribed to this thread.
View it on GitHub or mute the thread.
but the same doc says
Enables testing for this directory and below.
it doesn't make sense to me (and it isn't needed in STIR). @paskino, are you sure we need this?
By the way, you will need to merge master onto here for final testing. @evgueni-ovtchinnikov fixed filenames and "case" of directory, so this might affect this PR.
@paskino pushed 1 commit.
—
You are receiving this because you are subscribed to this thread.
View it on GitHub or mute the thread.
@paskino pushed 1 commit.
—
You are receiving this because you are subscribed to this thread.
View it on GitHub or mute the thread.
good to know! I actually struggled to find that variable! All right, I'll fix it next week then.
@paskino pushed 2 commits.
—
You are receiving this because you are subscribed to this thread.
View it on GitHub or mute the thread.
@KrisThielemans approved this pull request.
Merged #58.
@casperdcl commented on this pull request.
In src/xGadgetron/pGadgetron/tests/CMakeLists.txt:
> +# This file is part of the CCP PETMR Synergistic Image Reconstruction Framework (SIRF) SuperBuild. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0.txt
+# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +#========================================================================= +include(FindPythonInterp)
@KrisThielemans I'm assuming this is overriding travis' PYTHON_EXECUTABLE
@casperdcl commented on this pull request.
In src/xSTIR/pSTIR/tests/CMakeLists.txt:
> +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0.txt
+# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +#========================================================================= +include(FindPythonInterp) +add_test(NAME PET_TEST1 COMMAND ${PYTHON_EXECUTABLE} test1.py WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/src/xSTIR/pSTIR/tests/ )
same here...
@paskino commented on this pull request.
In src/xGadgetron/pGadgetron/tests/CMakeLists.txt:
> +# This file is part of the CCP PETMR Synergistic Image Reconstruction Framework (SIRF) SuperBuild. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0.txt
+# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +#========================================================================= +include(FindPythonInterp)
why would this happen only on one of the systems in the travis matrix?