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

Document how to run the test suite #39

Closed
drewish opened this issue Apr 7, 2015 · 17 comments
Closed

Document how to run the test suite #39

drewish opened this issue Apr 7, 2015 · 17 comments
Assignees

Comments

@drewish
Copy link
Contributor

drewish commented Apr 7, 2015

I'm working on my first contribution and managed to get a simple test case built to demonstrate the problem. I'm going to create a branch that fixes the issue but I'd like to run the test suite to ensure there are no regressions. After reading the various INSTALL.md files it's not clear to me how to build and run the test.

@bo0ts
Copy link
Member

bo0ts commented Apr 7, 2015

👍 A quick workaround to test an individual package on a Unix system:

# make sure CGAL_DIR is set, have a configured build 
# tree with all the options (especially CMAKE_BUILD_TYPE=Release)
export CGAL_DIR=/path/to/cgal/build
# make sure you have the CGAL scripts in your path
export PATH="$PATH;/path/to/cgal/Scripts/developer_scripts;/path/to/cgal/Scripts/scripts"
# go to your package
cd /path/to/cgal/package/test/package
# run the testsuite
cgal_test_with_cmake
# Repeat for the examples folder

This should give you an idea how to test a single package and maybe some obvious dependencies.

@lrineau
Copy link
Member

lrineau commented Apr 16, 2015

@drewish Have you tried what is suggested by @bo0ts?

@drewish
Copy link
Contributor Author

drewish commented Apr 16, 2015

@lrineau i did. i guess i was expecting more of a test suite rather than having to manually run each of the test programs. i was able to run them but without knowing the expected output it wasn't obvious if they were doing what they were supposed to do. i ended up just making sure the affected ones would compile.

@lrineau
Copy link
Member

lrineau commented Apr 16, 2015

Actually the CGAL testsuite runs every night from a release tarball. The set of scripts, see in Scripts/developers_scripts, are made to run in the tarball flat layout, and not in the modular layout of the Git repository.

@bo0ts
Copy link
Member

bo0ts commented Apr 17, 2015

@drewish The few commands I gave you should be able to compile, run and confirm the correctness of the tests without you needing to know that. In general, a test will be considered correct based on the process exit code and if it prints the word ERROR to stdout. How to invoke a test/example binary for testing is a bit more complicated but you hopefully shouldn't need to know that when using cgal_test_with_cmake.

@drewish
Copy link
Contributor Author

drewish commented Apr 20, 2015

Good to know I should just be looking for errors.

I tried these instructions on another computer (using the master branch) and ended up getting some errors when trying to test the Straight_skeleton_2 tests or examples:

In file included from /Users/amorton/projects/cgal/Straight_skeleton_2/test/Straight_skeleton_2/offset_bug.cpp:1:
In file included from /Users/amorton/projects/cgal/Cartesian_kernel/include/CGAL/Cartesian.h:28:
In file included from /Users/amorton/projects/cgal/Cartesian_kernel/include/CGAL/Cartesian/Cartesian_base.h:28:
In file included from /Users/amorton/projects/cgal/Kernel_23/include/CGAL/basic.h:29:
/Users/amorton/projects/cgal/Installation/include/CGAL/config.h:43:4: error: The test-suite needs no NDEBUG defined
#  error The test-suite needs no NDEBUG defined
   ^
1 error generated.

I can include more of the compiler output if that's helpful.

@bo0ts
Copy link
Member

bo0ts commented Apr 20, 2015

No, the output is sufficient. That is one of the peculiarities of our
testsuite. It can only run when NDEBUG is defined (meaning that assertions
are enabled). The compilation flags are taken from the CMake build
(CMakeCache.txt) found in CGAL_DIR. I assume you configured that build
with CMAKE_BUILD_TYPE=Release resulting in a build with no debugging. For
simple testing purposes I would recommend changing that variable to the
value Debug. For this:

cmake $CGAL_DIR
cmake -DCMAKE_BUILD_TYPE=Debug . # or use ccmake, cmake-gui etc
make clean && make # rebuild

No rinse and repeat the testing process. You probably need to remove the
CMakeCache.txt to delete the cache generated by cgal_test_with_cmake.
Thanks for staying with me through this.

@lrineau
Copy link
Member

lrineau commented Apr 20, 2015

I wonder why <CGAL/config.h> is complaining about NDEBUG instead of "#undef-ing" it. I think that header should warn about NDEBUG if it is defined at the same time CGAL_TESTSUITE is defined, and then it should undefine the macro.

(Note for @bo0ts: It seems that GitHub does not format using Markdown when the answer is send by mail.)

@bo0ts
Copy link
Member

bo0ts commented Apr 20, 2015

I think the current behavior is OK. If you #undef NDEBUG you cannot be sure if <assert> or any other debug header has already been included or not. The current implementation is more robust.

@lrineau
Copy link
Member

lrineau commented Apr 20, 2015

As the CGAL testsuite is on our repository, we could enforce that CGAL/config.h is always included first in CGAL tests and examples.

@bo0ts
Copy link
Member

bo0ts commented Apr 20, 2015

IMO too much effort to actually have a test for this and not enough returns. In examples it also would be misleading for users, because they would believe they always have to include CGAL/config.h even if this is not the case and we don't want them to believe that.

@lrineau
Copy link
Member

lrineau commented Apr 20, 2015

In example that is easy: most if not all CGAL headers also include CGAL/config.h first.

Philipp Moeller notifications@github.com a écrit :

IMO too much effort to actually have a test for this and not enough returns. In examples it also would be misleading for users, because they would believe they always have to include CGAL/config.h even if this is not the case and we don't want them to believe that.


Reply to this email directly or view it on GitHub.

@bo0ts
Copy link
Member

bo0ts commented Apr 20, 2015

Which brings us back to the problem that

#include <boost/assert.hpp> // imagine any other assertion include here
#include <CGAL/config.h> // CGAL/config.h

will be broken, because it now depends on the include order and boost.assert is going to behave differently from the CGAL assertions. The current practice is OK IMO, maybe we should add the options to enable assertions in automated builds, but this is anoher issue. Anyway, this is getting to far of topic. Let's continue this discussion elsewhere if you think it is necessary.

@lrineau
Copy link
Member

lrineau commented Apr 20, 2015

The current behavior is completely OK. I was trying to find out if we can improve it, to avoid what Andrew has reported at #39 (comment).

But it that would create more confusion, let's forget that proposal.

@strk
Copy link

strk commented Jun 15, 2015

Having a "check" Makefile target could be useful for many (it's a standard GNU target).

@bo0ts
Copy link
Member

bo0ts commented Jun 15, 2015

@strk Yes, but currently this is far from feasible as it would entail creating a release and running the testsuite on it. We are trying to get their though.

@sloriot
Copy link
Member

sloriot commented Nov 23, 2015

See #504

@sloriot sloriot self-assigned this Jan 18, 2016
@sloriot sloriot closed this as completed Jan 18, 2016
MaelRL added a commit to MaelRL/cgal that referenced this issue Mar 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants