Unit testing cmake #2268

jngrad · 2018-09-17T18:51:46Z

Unit tests for CMake output files: espresso binaries, Python bindings, Doxygen documentation and Sphinx documentation. Part of the make check workflow. If we ever need a shell-independent solution, there is kward/shunit2.

Regarding #2245, the install test checks make install DESTDIR=/some/dir, yet for make install we need write rights for /usr/local/lib, which the end user doesn't have without sudo. We could either:

make the CMake tests exclusive to the CI workflow, testing make install becomes trivial
keep the make check workflow as-is and add a CI-specific target check_cmake_CI to test make install

Solution 2 allows external developers to run CMake tests with make check, while solution 1 requires them to find the CI-specific CMake target and adapt it to run without sudo rights.

Note: the CMake variables PYTHON_FOUND and other *_FOUND variables defined in /CMakeLists.txt seem out of scope in /testsuite/cmake/CMakeLists.txt. Is there any way to import them? At the moment I re-declared them in /testsuite/cmake/CMakeLists.txt (can break if we update version numbers in /CMakeLists.txt):

find_package(PythonInterp REQUIRED)
find_package(Doxygen)
find_package(Sphinx 1.6.6)

* Create /testsuite/cmake/CMakeLists.txt using /testsuite/python/CMakeLists.txt * Split CMake unit tests in 4 files * Handle the setup() function inside CMake with add_custom_target(), for example: `add_custom_target(setup_doxygen COMMAND make doc WORKING_DIRECTORY ${CMAKE_BINARY_DIR})`

mkuron · 2018-09-17T19:17:43Z

for make install we need write rights for /usr/local/lib

You could change the destination to some other place that‘s writable, like /tmp. There is a CMake variable for that.

hmenke · 2018-09-17T21:24:04Z

cmake -DCMAKE_INSTALL_PREFIX=/tmp/espresso ..
make
make install

KaiSzuttor · 2018-09-18T07:23:55Z

testsuite/CMakeLists.txt

-add_dependencies(check_python pypresso python_test_data check_python_serial)
-
-add_dependencies(check check_python)
+add_subdirectory(python)


we lost git history here... better: git mv CMakeLists.txt python

I see no difference between testsuite/CMakeLists.txt in upstream and testsuite/python/CMakeLists.txt in this PR. Commit c273d64 shows everything was renamed with git mv. Commit 8e80916 applied the changes by junghans to preserve git history.

ah git is not good in tracking moves...

KaiSzuttor · 2018-09-18T11:58:31Z

the *FOUND variables are available in the subdirectories, for PythonInterp it's PYTHONINTERP_FOUND

jngrad · 2018-09-18T13:52:25Z

@KaiSzuttor: thanks, I see the issue with *_FOUND variables now
@mkuron, @hmenke: good idea! CI now uses /tmp to test the standard installation procedure. This test is disabled for the user, as it would collide with the normal installation workflow (CMake environment variables set by the user would be erased). Is it necessary to add a cleanup function, or is /tmp automatically cleaned after CI?

KaiSzuttor · 2018-09-18T17:56:09Z

We use docker containers / VM, so you dont have to worry about /tmp

codecov · 2018-09-18T19:19:25Z

Codecov Report

Merging #2268 into python will decrease coverage by <1%.
The diff coverage is n/a.

@@          Coverage Diff           @@
##           python   #2268   +/-   ##
======================================
- Coverage      71%     71%   -1%     
======================================
  Files         377     377           
  Lines       18946   18925   -21     
======================================
- Hits        13585   13526   -59     
- Misses       5361    5399   +38

Impacted Files	Coverage Δ
...rc/core/utils/serialization/CUDA_particle_data.hpp	`50% <0%> (-50%)`	⬇️
src/core/constraints/Constraint.hpp	`66% <0%> (-34%)`	⬇️
src/core/short_range_loop.hpp	`85% <0%> (-15%)`	⬇️
src/core/EspressoSystemInterface.cpp	`83% <0%> (-9%)`	⬇️
src/core/PartCfg.hpp	`91% <0%> (-9%)`	⬇️
src/core/lattice.cpp	`82% <0%> (-7%)`	⬇️
src/core/object-in-fluid/membrane_collision.hpp	`14% <0%> (-4%)`	⬇️
src/core/domain_decomposition.cpp	`88% <0%> (-4%)`	⬇️
src/core/Vector.hpp	`89% <0%> (-3%)`	⬇️
src/core/algorithm/verlet_ia.hpp	`97% <0%> (-3%)`	⬇️
... and 18 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3059d2d...5812159. Read the comment docs.

jngrad · 2018-09-18T19:27:04Z

Good to know:

depending on the linux distribution, Python modules are installed in pythonX.Y/dist-packages or pythonX.Y/site-packages
mpiexec and its siblings from the Open MPI suite can stall indefinitely when running a bash script containing two subprocesses with the syntax (set -e; python -m ...). Ctrl+C does not terminate the process, Ctrl+Z hangs the process but does not terminate it, such that you cannot cleanup files created by the unit tests until you SIGTERM mpiexec in htop. I've disabled MPI for CMake tests.

codecov · 2018-09-18T19:27:55Z

Codecov Report

Merging #2268 into python will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff           @@
##           python   #2268   +/-   ##
======================================
  Coverage      71%     71%           
======================================
  Files         380     380           
  Lines       18975   18975           
======================================
  Hits        13588   13588           
  Misses       5387    5387

Impacted Files	Coverage Δ
src/core/grid_based_algorithms/lb.hpp	`71% <0%> (ø)`	⬆️
src/core/lattice.cpp	`89% <0%> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3c195e3...71fa626. Read the comment docs.

jngrad · 2018-09-18T19:30:04Z

The CMake tests are now included in /maintainer/CI/build_cmake.sh and are running in CI, however Sphinx needs some troubleshooting. Depending on the container, either Sphinx extensions are missing:

Could not import extension sphinxcontrib.bibtex (exception: No module named bibtex)
make[7]: *** [doc/sphinx/CMakeFiles/sphinx] Error 1

or it cannot load a Python module:

Warning, treated as error:
autodoc: failed to import module 'espressomd.visualization_mayavi'; the following exception was raised:
No module named 'particle_data'
make[7]: *** [doc/sphinx/CMakeFiles/sphinx] Error 2
doc/sphinx/CMakeFiles/sphinx.dir/build.make:57: recipe for target 'doc/sphinx/CMakeFiles/sphinx' failed

KaiSzuttor · 2018-09-18T19:39:36Z

The latter is an issue of install as sphinx does not seem to find necessary files. I’m also not so sure if we want to test sphinx and doxygen for the install target since we deploy both with each merge via the CI.

jngrad · 2018-09-19T08:33:37Z

Agreed, let's just test installation. Now most containers pass CI, however maxset-python3, clang:6.0 and intel:15 seem unable to copy python files into the build directory, despite being configured with $with_python_interface set to true.

jngrad · 2018-09-22T18:22:33Z

Alright, it was just a race condition in the Makefile for two containers. I also found an interaction with Clang's UBSan that prevents python from importing the espressomd module after installation in /tmp:

File "/tmp/espresso-unit-tests/lib/python2.7/dist-packages/espressomd/__init__.py", line 23, in <module>
    import espressomd._init
ImportError: /builds/espressomd/espresso/build/src/core/grid_based_algorithms/liblbgpu_cuda.so: undefined symbol: __ubsan_vptr_type_cache

Not really sure why a shared object in the build directory would be called at that point (linking error?), nor why the error doesn't show up before installation. I get a different pointer error when compiling espresso with UBSan on my Ubuntu machine. UBSan is currently enabled only in container clang-6.0; I've changed the CMake test to skip the import test when UBSan is enabled.

Then, code coverage stopped working for container cuda-maxset (yet unit tests pass).

…into unit-test-cmake

KaiSzuttor · 2018-09-25T06:48:52Z

maybe we should try to get the test pass also with UBSan. @mkuron can you please have a look?

mkuron · 2018-09-25T08:09:03Z

You need the LD_PRELOAD from the pypresso script in order to inject ubsan into python itself.

jngrad · 2018-09-25T09:20:37Z

Using pypresso instead of python2 exits the interpreter on my machine with UndefinedBehaviorSanitizer: failed to parse suppressions instead of raising warnings. I tried removing lines in /tools/ubsan-suppressions.txt to check if one of them caused the parsing error, but the message did not change (except when the file is completely empty, causing a read error).

KaiSzuttor · 2018-09-25T14:44:19Z

@jngrad could you please have a look at my changes?

jngrad · 2018-09-25T15:06:00Z

Looks good to me.

testsuite/cmake/test_doxygen.sh

testsuite/cmake/test_sphinx.sh

KaiSzuttor · 2018-09-26T09:33:07Z

@jngrad i don't understand the interplay between your cmake tests and the failing coverage commands, any idea?

jngrad · 2018-09-26T09:41:29Z

I've got the same coverage command issue in #2280 despite only changing comment lines in that PR.

jngrad · 2018-09-26T10:11:43Z

Our GitLab instance crashed on Saturday. It automatically restarted afterwards, then a few hours later I got the coverage command issue here (57d8d12) and on the other PR (8fedeb9). The issue seems random, as your commit 31b76f8 passed while my next commit 71fa626 did not (although I just re-started the failed test with container cuda-shanchen and it now passes).

jngrad · 2018-09-26T12:49:25Z

Maybe this is relevant: I used git commit --amend --no-edit; git push --force in this PR and a similar rebase operation the other PR, while CI was still running in both cases. I'll halt any pending CI before push --force in the future.

fweik · 2018-09-28T12:59:08Z

@KaiSzuttor @jngrad What is the status on this? You should merge this soon to avoid merge conflicts.

Unit testing cmake

jngrad added 5 commits September 11, 2018 19:17

Bash unit testing framework

d3a36d0

Add tests for CMake: installation, documentation

a7672f3

Reorganize testsuite folder

c273d64

Merge

8e80916

Fix path

82538bb

KaiSzuttor reviewed Sep 18, 2018

View reviewed changes

fweik added this to the Espresso 4.0.1 milestone Sep 18, 2018

fweik added CMake DevOps labels Sep 18, 2018

Disable MPI for CMake unit testing

3f7ba35

jngrad force-pushed the unit-test-cmake branch 3 times, most recently from 0d760c3 to bb89f74 Compare September 18, 2018 17:10

jngrad force-pushed the unit-test-cmake branch 2 times, most recently from 5a65077 to 8e2833a Compare September 18, 2018 18:29

Add installation test in CI script

5812159

jngrad force-pushed the unit-test-cmake branch from 8e2833a to 5812159 Compare September 18, 2018 18:47

jngrad added 2 commits September 19, 2018 09:25

Remove Open MPI support

1ec2d08

Remove Doxygen/Sphinx unit tests

eed9856

jngrad force-pushed the unit-test-cmake branch from dbc3e86 to 7cac5fd Compare September 22, 2018 14:16

jngrad added 2 commits September 22, 2018 18:45

Fix multithreading issue

3e4bdbd

Improve Bash unit testing reliability

57d8d12

jngrad force-pushed the unit-test-cmake branch 2 times, most recently from 7638520 to 57d8d12 Compare September 22, 2018 17:25

KaiSzuttor added 2 commits September 24, 2018 17:59

maintainer: include coverage report for unit_tests.

ebfb920

Merge branch 'unit-test-cmake' of https://github.com/jngrad/espresso …

9fedc32

…into unit-test-cmake

KaiSzuttor added 2 commits September 25, 2018 15:35

cmake: call check of installation with pypresso

e1fb96d

Merged espressomd/python.

31b76f8

KaiSzuttor suggested changes Sep 26, 2018

View reviewed changes

testsuite/cmake/test_doxygen.sh Outdated Show resolved Hide resolved

testsuite/cmake/test_sphinx.sh Outdated Show resolved Hide resolved

KaiSzuttor approved these changes Sep 26, 2018

View reviewed changes

Remove unused CMake tests

71fa626

jngrad force-pushed the unit-test-cmake branch from b3e264a to 71fa626 Compare September 26, 2018 08:02

KaiSzuttor approved these changes Sep 28, 2018

View reviewed changes

KaiSzuttor merged commit c1d850e into espressomd:python Sep 28, 2018

RudolfWeeber pushed a commit to RudolfWeeber/espresso that referenced this pull request Oct 17, 2018

Merge pull request espressomd#2268 from jngrad/unit-test-cmake

a6fb1f7

Unit testing cmake

jngrad deleted the unit-test-cmake branch January 18, 2022 12:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unit testing cmake #2268

Unit testing cmake #2268

jngrad commented Sep 17, 2018

mkuron commented Sep 17, 2018

hmenke commented Sep 17, 2018

KaiSzuttor Sep 18, 2018

jngrad Sep 18, 2018

KaiSzuttor Sep 18, 2018

KaiSzuttor commented Sep 18, 2018

jngrad commented Sep 18, 2018 •

edited

Loading

KaiSzuttor commented Sep 18, 2018

codecov bot commented Sep 18, 2018

jngrad commented Sep 18, 2018

codecov bot commented Sep 18, 2018 •

edited

Loading

jngrad commented Sep 18, 2018

KaiSzuttor commented Sep 18, 2018

jngrad commented Sep 19, 2018

jngrad commented Sep 22, 2018

KaiSzuttor commented Sep 25, 2018

mkuron commented Sep 25, 2018

jngrad commented Sep 25, 2018

KaiSzuttor commented Sep 25, 2018

jngrad commented Sep 25, 2018

KaiSzuttor commented Sep 26, 2018

jngrad commented Sep 26, 2018

jngrad commented Sep 26, 2018

jngrad commented Sep 26, 2018

fweik commented Sep 28, 2018

Unit testing cmake #2268

Unit testing cmake #2268

Conversation

jngrad commented Sep 17, 2018

mkuron commented Sep 17, 2018

hmenke commented Sep 17, 2018

KaiSzuttor Sep 18, 2018

Choose a reason for hiding this comment

jngrad Sep 18, 2018

Choose a reason for hiding this comment

KaiSzuttor Sep 18, 2018

Choose a reason for hiding this comment

KaiSzuttor commented Sep 18, 2018

jngrad commented Sep 18, 2018 • edited Loading

KaiSzuttor commented Sep 18, 2018

codecov bot commented Sep 18, 2018

Codecov Report

jngrad commented Sep 18, 2018

codecov bot commented Sep 18, 2018 • edited Loading

Codecov Report

jngrad commented Sep 18, 2018

KaiSzuttor commented Sep 18, 2018

jngrad commented Sep 19, 2018

jngrad commented Sep 22, 2018

KaiSzuttor commented Sep 25, 2018

mkuron commented Sep 25, 2018

jngrad commented Sep 25, 2018

KaiSzuttor commented Sep 25, 2018

jngrad commented Sep 25, 2018

KaiSzuttor commented Sep 26, 2018

jngrad commented Sep 26, 2018

jngrad commented Sep 26, 2018

jngrad commented Sep 26, 2018

fweik commented Sep 28, 2018

jngrad commented Sep 18, 2018 •

edited

Loading

codecov bot commented Sep 18, 2018 •

edited

Loading