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

Tests: Migrate to Catch 2.4.0+ #712

Merged
merged 2 commits into from
Jan 2, 2019
Merged

Conversation

ax3l
Copy link
Member

@ax3l ax3l commented Dec 11, 2018

Migrate from Boost.Test to Catch2 for:

  • compile time
  • stability
  • better native compiler support
  • automated, modern code coverage and tooling on mainline repo
  • no C++98 work-around legacy with a modern code base
  • no long-term bugs (e.g. on OSX) and ominous valgrind issues
  • triggers no warnings (as long as we didn't migrate to CMake's CUDA language feature)
  • no dependencies (fully self-contained: just needs C++11 & a stdlib, easily ship-able when vendored)

Uses catch2 in a way where most implementation parts are only compiled once in a .cpp file for the main() and then linked against the test cases that pull only part of the header. Quite nifty and we could later on speed it up even more with PCHs for the tests, e.g. via cotire.

Related to #481.

  • If you like, I can vendor a fixed single-header release into an alpaka third-party dir so one does not even need to pre-install Catch2 at all. I did this before in other libs and would add a switch that allows switching to an external dependency as well: Vendor Catch2: v2.5.0 #718
  • If you like, we can enable some Catch2 fast-compile options on CI only, making runtime crashes a bit less detailed but one would likely debug local anyway.
  • Compile times on AppVeyor look significantly improved (develop before) and Travis-CI (develop before) seems very well too, even including the initial docker image rebuild.

Debugger Options

In case you want to break into the debugger on failures, run an individual test executable with

gdb --args ./testName -a -b

as -h will show: -a aborts on the first error and -b breaks into the debugger.

Developers Install of Catch2

If we don't vendor a single-header version directly in Alpaka yet, which I would say we can do, install catch 2.4.0+ for yourself with user-level package managers. For example:

# (Linux/OSX)
spack install catch
spack load catch

or via

# (Win/Linux/OSX)
conda install -c schrodinger catch2

or via conan from bincrafters (Win/Linux/OSX)

or via vcpkg (Windows)

or via aur (Arch Linux)

or just quickly build it form source and extend your CMAKE_PREFIX_PATH

@ax3l ax3l added this to the Version 0.4.0 milestone Dec 11, 2018
@ax3l ax3l requested a review from BenjaminW3 December 11, 2018 17:32
@ax3l ax3l force-pushed the topic-catch2 branch 12 times, most recently from 7d54048 to 18ec939 Compare December 11, 2018 23:24
script/travis/docker_install.sh Outdated Show resolved Hide resolved
@ax3l ax3l changed the title [WIP] Tests: Migrate to Catch 2.3.0+ Tests: Migrate to Catch 2.3.0+ Dec 12, 2018
@ax3l ax3l force-pushed the topic-catch2 branch 2 times, most recently from ad3220f to d58835c Compare December 12, 2018 09:43
@ax3l ax3l changed the title Tests: Migrate to Catch 2.3.0+ Tests: Migrate to Catch 2.4.0+ Dec 12, 2018
@ax3l
Copy link
Member Author

ax3l commented Dec 12, 2018

Switched to Catch 2.4.0+ (instead of 2.3.0+) now since I saw in tests and changelog that runtime improved further in this release.

@ax3l ax3l force-pushed the topic-catch2 branch 2 times, most recently from 1acfd74 to fb03c38 Compare December 12, 2018 13:13
alpakaConfig.cmake Outdated Show resolved Hide resolved
@ax3l ax3l force-pushed the topic-catch2 branch 5 times, most recently from f321768 to 2d905b0 Compare December 18, 2018 12:18
@ax3l
Copy link
Member Author

ax3l commented Dec 18, 2018

The problem is likely caused by the way that we still manually add our compiler flags for C++ instead of using target properties. I "linked"/included the external library now more old school to avoid populating the std flags twice.

@ax3l ax3l force-pushed the topic-catch2 branch 2 times, most recently from f37291e to c2eaae3 Compare December 18, 2018 14:11
@ax3l
Copy link
Member Author

ax3l commented Dec 18, 2018

Is it ok if we switch to a newer CMake, e.g. 3.11+ ? Improves handling of imported targets a lot and also (3.8+) adds better control over target properties/features.

@ax3l ax3l mentioned this pull request Dec 18, 2018
@ax3l ax3l force-pushed the topic-catch2 branch 4 times, most recently from 4a9aa33 to 02216ba Compare December 20, 2018 07:42
Migrate from Boost.Test to Catch2 for:

- compile time
- stability
- better native compiler support
- smaller dependencies
@BenjaminW3
Copy link
Member

After thinking about it some time I would prefer to include the testing framework within the project. I do not see any advantage of requiring the user to download the framework. Do you see any?
The user should still be able to set a different path to the test framework but the default should be the one included in the repo.
Should we use a submodule for this?

@ax3l
Copy link
Member Author

ax3l commented Dec 24, 2018

Sure thing! Catch2 comes as a single header, either we check it in as such or download it on the fly with cmake. I am not a big fan of submodules for their usebility outside of pure meta repos.

@ax3l
Copy link
Member Author

ax3l commented Jan 1, 2019

@BenjaminW3 can we please merge this before the other CI changes come in? I would like to avoid rebasing this a lot.
I can add a vendored catch2 header later on in a follow-up (I have the PR ready).

- AppVeyor
- Travis-CI
@ax3l
Copy link
Member Author

ax3l commented Jan 2, 2019

Final CI compile-time comparison (similar to here): recent develop vs. PR with caches up
(3:40h vs. 3:03h).

Catch2 fast compile not yet activated and not yet vendored into our source.

catch2

@ax3l
Copy link
Member Author

ax3l commented Jan 2, 2019

@BenjaminW3 hurray, CI finished :)

@BenjaminW3 BenjaminW3 merged commit 9032197 into alpaka-group:develop Jan 2, 2019
@ax3l ax3l deleted the topic-catch2 branch January 2, 2019 10:59
@ax3l
Copy link
Member Author

ax3l commented Jan 31, 2019

@BenjaminW3 Catch 2.6.0 now contains a TEMPLATE_PRODUCT_TEST_CASE macro as we had it before :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants