-
Notifications
You must be signed in to change notification settings - Fork 64
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
Migrate test suite to GoogleTest framework #119
Changes from 9 commits
8f802af
742f529
fad3a15
3c8b754
f963ca8
6d16a79
9f1b342
15cb73c
e5e4f70
4ba22d0
eeeb3ee
a2f0d76
0334d72
1fb6bf1
5ddbdee
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
# SPDX-License-Identifier: MIT | ||
# | ||
# SPDX-FileCopyrightText: Copyright (c) 2019-2023 Lars Melchior and contributors | ||
|
||
set(CPM_DOWNLOAD_VERSION 0.38.6) | ||
set(CPM_HASH_SUM "11c3fa5f1ba14f15d31c2fb63dbc8628ee133d81c8d764caad9a8db9e0bacb07") | ||
|
||
if(CPM_SOURCE_CACHE) | ||
set(CPM_DOWNLOAD_LOCATION "${CPM_SOURCE_CACHE}/cpm/CPM_${CPM_DOWNLOAD_VERSION}.cmake") | ||
elseif(DEFINED ENV{CPM_SOURCE_CACHE}) | ||
set(CPM_DOWNLOAD_LOCATION "$ENV{CPM_SOURCE_CACHE}/cpm/CPM_${CPM_DOWNLOAD_VERSION}.cmake") | ||
else() | ||
set(CPM_DOWNLOAD_LOCATION "${CMAKE_BINARY_DIR}/cmake/CPM_${CPM_DOWNLOAD_VERSION}.cmake") | ||
endif() | ||
|
||
# Expand relative path. This is important if the provided path contains a tilde (~) | ||
get_filename_component(CPM_DOWNLOAD_LOCATION ${CPM_DOWNLOAD_LOCATION} ABSOLUTE) | ||
|
||
file(DOWNLOAD | ||
https://github.com/cpm-cmake/CPM.cmake/releases/download/v${CPM_DOWNLOAD_VERSION}/CPM.cmake | ||
${CPM_DOWNLOAD_LOCATION} EXPECTED_HASH SHA256=${CPM_HASH_SUM} | ||
) | ||
|
||
include(${CPM_DOWNLOAD_LOCATION}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
option(RTNEURAL_TEST_REPORTS "Output test reports to XML files" OFF) | ||
|
||
macro(rtneural_setup_testing) | ||
include(CTest) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was under the impression that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe you're correct on both counts :) A |
||
enable_testing() | ||
add_custom_target(rtneural_test COMMAND ctest -C ${Configuration} --output-on-failure) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice! |
||
CPMAddPackage("gh:google/[email protected]") | ||
endmacro() | ||
|
||
function(rtneural_add_test) | ||
|
||
set(one_val_args TARGET) | ||
set(multi_val_args SOURCES DEPENDENCIES) | ||
cmake_parse_arguments(arg "" "${one_val_args}" "${multi_val_args}" ${ARGN}) | ||
|
||
add_executable(${arg_TARGET} ${arg_SOURCES}) | ||
target_link_libraries(${arg_TARGET} PUBLIC gtest_main gmock ${arg_DEPENDENCIES}) | ||
target_compile_definitions(${arg_TARGET} PRIVATE RTNEURAL_ROOT_DIR="${CMAKE_SOURCE_DIR}/") | ||
|
||
if(RTNEURAL_TEST_REPORTS) | ||
set(test_cmd_args --gtest_output=xml:${arg_TARGET}_report.xml) | ||
endif() | ||
|
||
add_test( | ||
NAME ${arg_TARGET} | ||
WORKING_DIRECTORY ./ | ||
COMMAND ${arg_TARGET} ${test_cmd_args}) | ||
|
||
add_dependencies(rtneural_test ${arg_TARGET}) | ||
|
||
endfunction() |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,24 +1,9 @@ | ||
include_directories(../RTNeural) | ||
|
||
add_executable(rtneural_tests tests.cpp) | ||
target_link_libraries(rtneural_tests LINK_PUBLIC RTNeural) | ||
target_compile_definitions(rtneural_tests PRIVATE RTNEURAL_ROOT_DIR="${CMAKE_SOURCE_DIR}/") | ||
|
||
add_test(NAME "RTNeural_Bad_Model_Test" COMMAND $<TARGET_FILE:rtneural_tests> bad_model) | ||
add_test(NAME "RTNeural_Model_Test" COMMAND $<TARGET_FILE:rtneural_tests> model) | ||
add_test(NAME "RTNeural_Sample_Rate_RNN_Test" COMMAND $<TARGET_FILE:rtneural_tests> sample_rate_rnn) | ||
add_test(NAME "RTNeural_Torch_Test" COMMAND $<TARGET_FILE:rtneural_tests> torch) | ||
add_test(NAME "RTNeural_Util_Test" COMMAND $<TARGET_FILE:rtneural_tests> util) | ||
add_test(NAME "RTNeural_Conv1D_Test" COMMAND $<TARGET_FILE:rtneural_tests> conv1d) | ||
add_test(NAME "RTNeural_Conv2D_Test" COMMAND $<TARGET_FILE:rtneural_tests> conv2d) | ||
add_test(NAME "RTNeural_Dense_Test" COMMAND $<TARGET_FILE:rtneural_tests> dense) | ||
add_test(NAME "RTNeural_GRU_Test" COMMAND $<TARGET_FILE:rtneural_tests> gru) | ||
add_test(NAME "RTNeural_GRU_1D_Test" COMMAND $<TARGET_FILE:rtneural_tests> gru_1d) | ||
add_test(NAME "RTNeural_LSTM_Test" COMMAND $<TARGET_FILE:rtneural_tests> lstm) | ||
add_test(NAME "RTNeural_LSTM_1D_Test" COMMAND $<TARGET_FILE:rtneural_tests> lstm_1d) | ||
add_subdirectory(unit) | ||
add_subdirectory(functional) | ||
|
||
option(RTNEURAL_CODE_COVERAGE "Build RTNeural tests with code coverage flags" OFF) | ||
if(RTNEURAL_CODE_COVERAGE) | ||
include(${CMAKE_CURRENT_SOURCE_DIR}/../cmake/EnableCoverageFlags.cmake) | ||
enable_coverage_flags(rtneural_tests) | ||
enable_coverage_flags(rtneural_test_unit) | ||
enable_coverage_flags(rtneural_test_functional) | ||
endif() |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
# RTNeural Testing | ||
|
||
RTNeural tests are configured with CMake's [CTest](https://cmake.org/cmake/help/book/mastering-cmake/chapter/Testing%20With%20CMake%20and%20CTest.html) and developed with the [GoogleTest framework](https://github.com/google/googletest). Tests are split into two categories: | ||
|
||
1. unit tests, and | ||
2. functional tests. | ||
Comment on lines
+5
to
+6
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is great! I wonder if it might make sense to further split up the function tests into "layer" tests and "model" tests... at the moment we only have one model test, but I'm hoping to add more. Anyway, that's a change we can make later if we want to. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Glad you like it! On splitting the functional tests into tests for layers vs models, I'm not sure. I'd be happy to jump on a call to chat about it with you at some point, if you like! I would consider whether the layer tests are simple enough to be written as unit tests - they're usually simpler, easier to reason about, and less brittle. But every project has different needs and challenges, so there's no one-size-fits-all :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, that's a good point. I'd like to do a bit of hacking myself just to get a sense of what might work, but yeah, no rush or anything, just putting some thoughts out there. |
||
|
||
A unit test should (ideally) be small, and will usually verify that one piece of | ||
logic in an individual component is working correctly. Functional tests operate | ||
at a higher level, and should check that the combination of multiple components | ||
(or multiple pieces of logic) are operating together as expected. | ||
|
||
GoogleTest has excellent [documentation](https://google.github.io/googletest/), | ||
which is highly recommended reading if you're new to testing or new to the | ||
framework. | ||
|
||
Happy testing! | ||
|
||
## Running tests | ||
|
||
### Full test suite | ||
|
||
To build and run all tests, make sure that you have passed `-DBUILD_TESTS=ON` | ||
to your `cmake` command. You then have two options: | ||
|
||
1. build all targets as normal, then run the test suite by invoking `ctest` in your build folder, or | ||
2. build the `rtneural_test` target, which should also run the test suite. | ||
|
||
### All unit tests | ||
|
||
To build and run only the unit tests, | ||
|
||
1. build the `rtneural_test_unit` target, and | ||
2. run the executable in your build folder at `./tests/unit/rtneural_test_unit` | ||
|
||
### All functional tests | ||
|
||
To build and run only the functional tests, | ||
|
||
1. build the `rtneural_test_functional` target, and | ||
2. run the executable in your build folder at `./tests/functional/rtneural_test_functional` | ||
|
||
### A subset of the tests | ||
|
||
Running a sub-set of the tests within a test target like `rtneural_test_unit` can be done by passing | ||
a filter string to the `--gtest_filter` command-line argument, e.g. | ||
|
||
```sh | ||
./tests/unit/rtneural_test_unit --gtest_filter="*Activation*" | ||
``` | ||
|
||
There are a wealth of other useful command-line options that can be found by passing the `--help` | ||
argument to the test executable. | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I would prefer to leave the include path as
#include <RTNeural/RTNeural.h>
, just to avoid having a breaking change for end-users, and then we could update the tests to use that include path also?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For sure - sounds great. Totally agree on avoiding a breaking change. I'll update the tests and revert the include path!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one's done, and I've updated the PR description to match!