From f6407a4e0fd101c255a039d01052f35b3fe55ed8 Mon Sep 17 00:00:00 2001 From: Dominik Charousset Date: Tue, 29 Oct 2019 14:52:09 +0100 Subject: [PATCH 1/4] Add initial scaffold for unit testing via doctest --- CMakeLists.txt | 7 ++++ configure | 4 +++ src/3rdparty | 2 +- src/CMakeLists.txt | 22 +++++++++++++ src/broker/Data.cc | 21 ++++++++++++ src/main.cc | 80 ++++++++++++++++++++++++++++++++++++++++------ 6 files changed, 126 insertions(+), 10 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index c786f5ee59..3235ad04ba 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -19,6 +19,13 @@ include(cmake/FindClangTidy.cmake) ######################################################################## ## Project/Build Configuration + +if (ENABLE_ZEEK_UNIT_TESTS) + enable_testing() +else () + add_definitions(-DDOCTEST_CONFIG_DISABLE) +endif () + if ( ENABLE_CCACHE ) find_program(CCACHE_PROGRAM ccache) diff --git a/configure b/configure index e2670eed5d..dac759a7c7 100755 --- a/configure +++ b/configure @@ -47,6 +47,7 @@ Usage: $0 [OPTION]... [VAR=VALUE]... --enable-jemalloc link against jemalloc --enable-static-broker build Broker statically (ignored if --with-broker is specified) --enable-static-binpac build binpac statically (ignored if --with-binpac is specified) + --enable-cpp-tests build Zeek's C++ unit tests --disable-zeekctl don't install ZeekControl --disable-auxtools don't build or install auxiliary tools --disable-python don't try to build python bindings for Broker @@ -245,6 +246,9 @@ while [ $# -ne 0 ]; do --enable-static-binpac) append_cache_entry BUILD_STATIC_BINPAC BOOL true ;; + --enable-cpp-tests) + append_cache_entry ENABLE_ZEEK_UNIT_TESTS BOOL true + ;; --disable-zeekctl) append_cache_entry INSTALL_ZEEKCTL BOOL false ;; diff --git a/src/3rdparty b/src/3rdparty index ceaa634637..6411b6cc3d 160000 --- a/src/3rdparty +++ b/src/3rdparty @@ -1 +1 @@ -Subproject commit ceaa6346378cbeda8ce4e2be95f75b60865d113e +Subproject commit 6411b6cc3df2c647aff9af887e0ee52e55915885 diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index e32c739e9d..98813f9da0 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -433,3 +433,25 @@ add_clang_tidy_files(${MAIN_SRCS}) # (*.pac.cc) files, and most of the generated code for BIFs (not including # *.bif.register.cc) create_clang_tidy_target() + +######################################################################## +## CTest setup. + +# Scan all .cc files for TEST_CASE macros and generate CTest targets. +if (ENABLE_ZEEK_UNIT_TESTS) + file(GLOB_RECURSE all_cc_files "*.cc") + set(test_cases "") + foreach (cc_file ${all_cc_files}) + file (STRINGS ${cc_file} test_case_lines REGEX "TEST_CASE") + foreach (line ${test_case_lines}) + string(REGEX REPLACE "TEST_CASE\\(\"(.+)\"\\)" "\\1" test_case "${line}") + list(APPEND test_cases "${test_case}") + endforeach () + endforeach () + list(LENGTH test_cases num_test_cases) + MESSAGE(STATUS "-- Found ${num_test_cases} test cases for CTest") + foreach (test_case ${test_cases}) + add_test(NAME "\"${test_case}\"" + COMMAND zeek --testing "--test-case=${test_case}") + endforeach () +endif () diff --git a/src/broker/Data.cc b/src/broker/Data.cc index 6bba3b6bb2..0929961dc8 100644 --- a/src/broker/Data.cc +++ b/src/broker/Data.cc @@ -1,5 +1,6 @@ #include "Data.h" #include "File.h" +#include "3rdparty/doctest.h" #include "broker/data.bif.h" #include @@ -35,6 +36,16 @@ static broker::port::protocol to_broker_port_proto(TransportProto tp) } } +TEST_CASE("converting Zeek to Broker protocol constants") + { + CHECK_EQ(to_broker_port_proto(TRANSPORT_TCP), broker::port::protocol::tcp); + CHECK_EQ(to_broker_port_proto(TRANSPORT_UDP), broker::port::protocol::udp); + CHECK_EQ(to_broker_port_proto(TRANSPORT_ICMP), + broker::port::protocol::icmp); + CHECK_EQ(to_broker_port_proto(TRANSPORT_UNKNOWN), + broker::port::protocol::unknown); + } + TransportProto bro_broker::to_bro_port_proto(broker::port::protocol tp) { switch ( tp ) { @@ -50,6 +61,16 @@ TransportProto bro_broker::to_bro_port_proto(broker::port::protocol tp) } } +TEST_CASE("converting Broker to Zeek protocol constants") + { + using bro_broker::to_bro_port_proto; + CHECK_EQ(to_bro_port_proto(broker::port::protocol::tcp), TRANSPORT_TCP); + CHECK_EQ(to_bro_port_proto(broker::port::protocol::udp), TRANSPORT_UDP); + CHECK_EQ(to_bro_port_proto(broker::port::protocol::icmp), TRANSPORT_ICMP); + CHECK_EQ(to_bro_port_proto(broker::port::protocol::unknown), + TRANSPORT_UNKNOWN); + } + struct val_converter { using result_type = Val*; diff --git a/src/main.cc b/src/main.cc index 1bfaa5882d..eed67b0b66 100644 --- a/src/main.cc +++ b/src/main.cc @@ -60,6 +60,9 @@ extern "C" { #include "3rdparty/sqlite3.h" +#define DOCTEST_CONFIG_IMPLEMENT +#include "3rdparty/doctest.h" + Brofiler brofiler; #ifndef HAVE_STRSEP @@ -401,16 +404,75 @@ int main(int argc, char** argv) { std::set_new_handler(bro_new_handler); + auto copy_bro_args = [argc, argv] + { + bro_argc = argc; + bro_argv = new char* [argc]; + + for ( int i = 0; i < argc; i++ ) + bro_argv[i] = copy_string(argv[i]); + }; + +#ifndef DOCTEST_CONFIG_DISABLE + + doctest::Context context; + + // When running unit tests, the first argument on the command line must be + // --testing, followed by doctest options. Optionally, users can use "--" as + // separator to pass Zeek options afterwards: + // + // zeek --testing [doctest-options] -- [zeek-options] + + if (argc > 1 && strcmp(argv[1], "--testing") == 0) + { + + // Deal with CLI arguments (copy Zeek part over to bro_argv). + auto is_separator = [](const char* cstr) + { + return strcmp(cstr, "--") == 0; + }; + auto first = argv; + auto last = argv + argc; + auto separator = std::find_if(first, last, is_separator); + + if ( separator == last ) + { + bro_argc = 1; + bro_argv = new char*[1]; + bro_argv[0] = copy_string(argv[0]); + } + else + { + auto first_zeek_arg = std::next(separator); + bro_argc = 1 + std::distance(first_zeek_arg, last); + bro_argv = new char*[bro_argc]; + bro_argv[0] = copy_string(argv[0]); + auto bro_argv_iter = std::next(bro_argv); + for ( auto i = first_zeek_arg; i != last; ++i ) + *bro_argv_iter++ = copy_string(*i); + } + + context.applyCommandLine(std::distance(first, separator), argv); + + // Run queries/tests and exit. + return context.run(); + + } + else + { + copy_bro_args(); + } + +#else // DOCTEST_CONFIG_DISABLE + + copy_bro_args(); + +#endif // DOCTEST_CONFIG_DISABLE + double time_start = current_time(true); brofiler.ReadStats(); - bro_argc = argc; - bro_argv = new char* [argc]; - - for ( int i = 0; i < argc; i++ ) - bro_argv[i] = copy_string(argv[i]); - name_list interfaces; name_list read_files; name_list rule_files; @@ -509,7 +571,7 @@ int main(int argc, char** argv) #endif int op; - while ( (op = getopt_long(argc, argv, opts, long_opts, &long_optsind)) != EOF ) + while ( (op = getopt_long(bro_argc, bro_argv, opts, long_opts, &long_optsind)) != EOF ) switch ( op ) { case 'a': parse_only = true; @@ -721,7 +783,7 @@ int main(int argc, char** argv) plugin_mgr->SearchDynamicPlugins(bro_plugin_path()); - if ( optind == argc && + if ( optind == bro_argc && read_files.length() == 0 && interfaces.length() == 0 && ! id_name && ! command_line_policy && ! print_plugins ) @@ -730,7 +792,7 @@ int main(int argc, char** argv) // Process remaining arguments. X=Y arguments indicate script // variable/parameter assignments. X::Y arguments indicate plugins to // activate/query. The remainder are treated as scripts to load. - while ( optind < argc ) + while ( optind < bro_argc ) { if ( strchr(argv[optind], '=') ) params.push_back(argv[optind++]); From 8c848079fb9fd67b27fc7c2b5639a1bda277e3d2 Mon Sep 17 00:00:00 2001 From: Dominik Charousset Date: Tue, 12 Nov 2019 17:05:17 +0100 Subject: [PATCH 2/4] Fix submodule reference for doctest --- src/3rdparty | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/3rdparty b/src/3rdparty index 6411b6cc3d..5cace37278 160000 --- a/src/3rdparty +++ b/src/3rdparty @@ -1 +1 @@ -Subproject commit 6411b6cc3df2c647aff9af887e0ee52e55915885 +Subproject commit 5cace372787c87f7326887326f188c1d245d2faf From 773adab76bd16104e102ea1cac3fdf8177e3e5bc Mon Sep 17 00:00:00 2001 From: Dominik Charousset Date: Thu, 14 Nov 2019 09:15:50 +0100 Subject: [PATCH 3/4] Integrate review feedback --- CMakeLists.txt | 1 + src/CMakeLists.txt | 2 +- src/main.cc | 65 +++++++++++++++++++++++++--------------------- 3 files changed, 37 insertions(+), 31 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 3235ad04ba..64072ca629 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -22,6 +22,7 @@ include(cmake/FindClangTidy.cmake) if (ENABLE_ZEEK_UNIT_TESTS) enable_testing() + add_definitions(-DDOCTEST_CONFIG_SUPER_FAST_ASSERTS) else () add_definitions(-DDOCTEST_CONFIG_DISABLE) endif () diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 98813f9da0..2c47f7dd3b 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -452,6 +452,6 @@ if (ENABLE_ZEEK_UNIT_TESTS) MESSAGE(STATUS "-- Found ${num_test_cases} test cases for CTest") foreach (test_case ${test_cases}) add_test(NAME "\"${test_case}\"" - COMMAND zeek --testing "--test-case=${test_case}") + COMMAND zeek --test "--test-case=${test_case}") endforeach () endif () diff --git a/src/main.cc b/src/main.cc index eed67b0b66..00f0cf4cc6 100644 --- a/src/main.cc +++ b/src/main.cc @@ -153,7 +153,10 @@ bool bro_dns_fake() void usage(int code = 1) { fprintf(stderr, "zeek version %s\n", zeek_version()); + fprintf(stderr, "usage: %s [options] [file ...]\n", prog); + fprintf(stderr, "usage: %s --test [doctest-options] -- [options] [file ...]\n", prog); + fprintf(stderr, " | policy file, or read stdin\n"); fprintf(stderr, " -a|--parse-only | exit immediately after parsing scripts\n"); fprintf(stderr, " -b|--bare-mode | don't load scripts from the base/ directory\n"); @@ -195,6 +198,7 @@ void usage(int code = 1) fprintf(stderr, " -n|--idmef-dtd | specify path to IDMEF DTD file\n"); #endif + fprintf(stderr, " --test | run unit tests ('--test -h' for help, only when compiling with ENABLE_ZEEK_UNIT_TESTS)\n"); fprintf(stderr, " $ZEEKPATH | file search path (%s)\n", bro_path().c_str()); fprintf(stderr, " $ZEEK_PLUGIN_PATH | plugin search path (%s)\n", bro_plugin_path()); fprintf(stderr, " $ZEEK_PLUGIN_ACTIVATE | plugins to always activate (%s)\n", bro_plugin_activate()); @@ -404,26 +408,13 @@ int main(int argc, char** argv) { std::set_new_handler(bro_new_handler); - auto copy_bro_args = [argc, argv] - { - bro_argc = argc; - bro_argv = new char* [argc]; - - for ( int i = 0; i < argc; i++ ) - bro_argv[i] = copy_string(argv[i]); - }; - -#ifndef DOCTEST_CONFIG_DISABLE - - doctest::Context context; - // When running unit tests, the first argument on the command line must be - // --testing, followed by doctest options. Optionally, users can use "--" as + // --test, followed by doctest options. Optionally, users can use "--" as // separator to pass Zeek options afterwards: // - // zeek --testing [doctest-options] -- [zeek-options] + // zeek --test [doctest-options] -- [zeek-options] - if (argc > 1 && strcmp(argv[1], "--testing") == 0) + if (argc > 1 && strcmp(argv[1], "--test") == 0) { // Deal with CLI arguments (copy Zeek part over to bro_argv). @@ -452,23 +443,31 @@ int main(int argc, char** argv) *bro_argv_iter++ = copy_string(*i); } - context.applyCommandLine(std::distance(first, separator), argv); +#ifdef DOCTEST_CONFIG_DISABLE - // Run queries/tests and exit. + fprintf(stderr, "ERROR: C++ unit tests are disabled for this build.\n" + " Please re-compile with ENABLE_ZEEK_UNIT_TESTS " + "to run the C++ unit tests.\n"); + return EXIT_FAILURE; + +#else + + doctest::Context context; + context.applyCommandLine(std::distance(first, separator), argv); return context.run(); +#endif + } else { - copy_bro_args(); + bro_argc = argc; + bro_argv = new char* [argc]; + + for ( int i = 0; i < argc; i++ ) + bro_argv[i] = copy_string(argv[i]); } -#else // DOCTEST_CONFIG_DISABLE - - copy_bro_args(); - -#endif // DOCTEST_CONFIG_DISABLE - double time_start = current_time(true); brofiler.ReadStats(); @@ -531,6 +530,7 @@ int main(int argc, char** argv) #endif {"pseudo-realtime", optional_argument, 0, 'E'}, + {"test", no_argument, 0, '#'}, {0, 0, 0, 0}, }; @@ -711,6 +711,11 @@ int main(int argc, char** argv) break; #endif + case '#': + fprintf(stderr, "ERROR: --test only allowed as first argument.\n"); + usage(1); + break; + case 0: // This happens for long options that don't have // a short-option equivalent. @@ -794,12 +799,12 @@ int main(int argc, char** argv) // activate/query. The remainder are treated as scripts to load. while ( optind < bro_argc ) { - if ( strchr(argv[optind], '=') ) - params.push_back(argv[optind++]); - else if ( strstr(argv[optind], "::") ) - requested_plugins.insert(argv[optind++]); + if ( strchr(bro_argv[optind], '=') ) + params.push_back(bro_argv[optind++]); + else if ( strstr(bro_argv[optind], "::") ) + requested_plugins.insert(bro_argv[optind++]); else - add_input_file(argv[optind++]); + add_input_file(bro_argv[optind++]); } push_scope(nullptr, nullptr); From 885707d66689b87af572c4602f3ae6bbc619b8ef Mon Sep 17 00:00:00 2001 From: Dominik Charousset Date: Thu, 14 Nov 2019 09:19:26 +0100 Subject: [PATCH 4/4] Add doctest license and copyright --- COPYING.3rdparty | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/COPYING.3rdparty b/COPYING.3rdparty index 52b69deecb..06183fd114 100644 --- a/COPYING.3rdparty +++ b/COPYING.3rdparty @@ -583,3 +583,29 @@ The original source file comes with this licensing statement: This Source Code Form is subject to the terms of the Mozilla Public License, v. 2.0. If a copy of the MPL was not distributed with this file, You can obtain one at http://mozilla.org/MPL/2.0/. + +============================================================================== + +%%% src/3rdparty/doctest.h + +============================================================================== + +Copyright (c) 2016-2019 Viktor Kirilov + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE.