From fa43d077aa1ed1c560dcf589af2da3d71c0c83d7 Mon Sep 17 00:00:00 2001 From: Enji Cooper Date: Wed, 15 May 2019 00:35:52 -0700 Subject: [PATCH] Address some of @jmmv's comments in #203 This change does the following things: * Leverages utils/cmdline/*, instead of reinventing the wheel. * Consolidates googletest_results tests/setting for clarity. * Eliminates an unnecessary variable/comments. * Removes some duplicative TODOs/fixes TODO comment formatting. * Inlines temporary variables where possible/sensible. * Moves global variables into the anonymous global namespace. * Localizes the variable initialization near its use. * Adds more helpful comments above potentially obfuscated code. * Use list/set initialization instead of using the equivalent unrolled version with `.insert()`/`.push_back()`. * Fix indentation. Signed-off-by: Enji Cooper --- engine/googletest.cpp | 38 +++--- engine/googletest_helpers.cpp | 208 ++++++++++++++---------------- engine/googletest_list.cpp | 43 +++--- engine/googletest_list_test.cpp | 90 +++++++------ engine/googletest_result.cpp | 118 ++++++++--------- engine/googletest_result.hpp | 14 +- engine/googletest_result_test.cpp | 27 ++-- 7 files changed, 242 insertions(+), 296 deletions(-) diff --git a/engine/googletest.cpp b/engine/googletest.cpp index 255793b9..4e9a7559 100644 --- a/engine/googletest.cpp +++ b/engine/googletest.cpp @@ -66,16 +66,17 @@ namespace { /// Basename of the file containing the result written by the googletest /// testcase. /// -/// TODO: use more structured output format someday, like the googletest's JSON -/// or XML format. +/// TODO: Use more structured output format someday, like the googletest's JSON +/// or XML format to avoid dealing with upstream formatting changes, as there's +/// no guarantee that the output format (which is more or less custom/freeform) +/// won't change in the future, making compatibility with all versions +/// potentially difficult to scrape for. /// /// Using either format will require pulling in a third party library and -/// understanding the schema of the format, as it stands in 1.9.0, +/// understanding the schema of the format. As it stands in 1.9.0, /// googletest doesn't document this expectation very well and instead seems /// to rely on third-party solutions for doing structured output via the /// listener interfaces. -/// -///static const char* result_name = "result.googletest"; /// Magic numbers returned by exec_list when exec(2) fails. @@ -97,13 +98,15 @@ enum list_exit_code { /// /// \param test_program The test program to execute. void -engine::googletest_interface::exec_list(const model::test_program& test_program, - const config::properties_map& /*vars*/) const +engine::googletest_interface::exec_list( + const model::test_program& test_program, + const config::properties_map& /*vars*/) const { - process::args_vector args; + process::args_vector args{ + "--gtest_color=no", + "--gtest_list_tests" + }; - args.push_back("--gtest_color=no"); - args.push_back("--gtest_list_tests"); try { process::exec_unsafe(test_program.absolute_path(), args); } catch (const process::system_error& e) { @@ -182,17 +185,10 @@ engine::googletest_interface::exec_test(const model::test_program& test_program, utils::setenv(F("TEST_ENV_%s") % (*iter).first, (*iter).second); } - process::args_vector args; - args.push_back("--gtest_color=no"); - /// TODO: use more structured output format someday, like the googletest's - /// JSON or XML format. - /// - /// Using either format will require pulling in a third party library and - /// understanding the schema of the format, as it stands in 1.9.0, - /// googletest doesn't document this expectation very well and instead seems - /// to rely on third-party solutions for doing structured output via the - /// listener interfaces. - args.push_back(F("--gtest_filter=%s") % (test_case_name)); + process::args_vector args{ + "--gtest_color=no", + F("--gtest_filter=%s") % (test_case_name) + }; process::exec(test_program.absolute_path(), args); } diff --git a/engine/googletest_helpers.cpp b/engine/googletest_helpers.cpp index b05960cb..e387049c 100644 --- a/engine/googletest_helpers.cpp +++ b/engine/googletest_helpers.cpp @@ -42,20 +42,30 @@ extern char** environ; #include #include "utils/env.hpp" +#include "utils/cmdline/options.hpp" +#include "utils/cmdline/parser.ipp" #include "utils/format/containers.ipp" #include "utils/format/macros.hpp" #include "utils/fs/path.hpp" #include "utils/optional.ipp" #include "utils/test_utils.ipp" +namespace cmdline = utils::cmdline; namespace fs = utils::fs; +using cmdline::base_option; +using cmdline::bool_option; +using cmdline::parse; +using cmdline::parsed_cmdline; +using cmdline::string_option; + +namespace fs = utils::fs; namespace { /// Prefix for all testcases. -const std::string test_suite = "Suite."; +const char *test_suite = "Suite."; /// Logs an error message and exits the test with an error code. @@ -68,9 +78,40 @@ fail(const std::string& str) std::exit(EXIT_FAILURE); } -/// An example fail message for test_check_configuration_variables() when it -/// fails. -const char fail_message1[] = ( + +/// A test scenario that validates the TEST_ENV_* variables. +static void +test_check_configuration_variables(void) +{ + std::set< std::string > vars; + char** iter; + for (iter = environ; *iter != NULL; ++iter) { + if (std::strstr(*iter, "TEST_ENV_") == *iter) { + vars.insert(*iter); + } + } + + std::set< std::string > exp_vars{ + "TEST_ENV_first=some value", + "TEST_ENV_second=some other value" + }; + + if (vars == exp_vars) { + std::cout << ( +"Note: Google Test filter = Suite.check_configuration_variables\n" +"[==========] Running 1 test from 1 test case.\n" +"[----------] Global test environment set-up.\n" +"[----------] 1 test from Suite\n" +"[ RUN ] Suite.check_configuration_variables\n" +"[ OK ] Suite.check_configuration_variables (0 ms)\n" +"[----------] 1 test from PassFailTest (0 ms total)\n" +"\n" +"[----------] Global test environment tear-down\n" +"[==========] 1 test from 1 test case ran. (1 ms total)\n" +"[ PASSED ] 1 test.\n" + ); + } else { + std::cout << ( "Note: Google Test filter = Suite.Fails\n" "[==========] Running 1 test from 1 test case.\n" "[----------] Global test environment set-up.\n" @@ -90,44 +131,7 @@ const char fail_message1[] = ( "[ FAILED ] Suite.check_configuration_variables\n" "\n" " 1 FAILED TEST\n" -); - -/// An example pass message for test_check_configuration_variables() when it -/// passes. -const char pass_message1[] = ( -"Note: Google Test filter = Suite.check_configuration_variables\n" -"[==========] Running 1 test from 1 test case.\n" -"[----------] Global test environment set-up.\n" -"[----------] 1 test from Suite\n" -"[ RUN ] Suite.check_configuration_variables\n" -"[ OK ] Suite.check_configuration_variables (0 ms)\n" -"[----------] 1 test from PassFailTest (0 ms total)\n" -"\n" -"[----------] Global test environment tear-down\n" -"[==========] 1 test from 1 test case ran. (1 ms total)\n" -"[ PASSED ] 1 test.\n" -); - -/// A test scenario that validates the TEST_ENV_* variables. -static void -test_check_configuration_variables(void) -{ - std::set< std::string > vars; - char** iter; - for (iter = environ; *iter != NULL; ++iter) { - if (std::strstr(*iter, "TEST_ENV_") == *iter) { - vars.insert(*iter); - } - } - - std::set< std::string > exp_vars; - exp_vars.insert("TEST_ENV_first=some value"); - exp_vars.insert("TEST_ENV_second=some other value"); - if (vars == exp_vars) { - std::cout << pass_message1; - } else { - std::cout << fail_message1 - << F(" Expected: %s\nFound: %s\n") % exp_vars % vars; + ) << F(" Expected: %s\nFound: %s\n") % exp_vars % vars; //std::exit(EXIT_FAILURE); } } @@ -141,8 +145,11 @@ test_crash(void) std::abort(); } -/// An example failure message for `test_fail()`. -const char fail_message2[] = ( +/// A test scenario that reports some tests as failed. +static void +test_fail(void) +{ + std::cout << ( "Note: Google Test filter = Suite.fail\n" "[==========] Running 1 test from 1 test suite.\n" "[----------] Global test environment set-up.\n" @@ -161,19 +168,17 @@ const char fail_message2[] = ( "[ FAILED ] Suite.fail\n" "\n" " 1 FAILED TEST\n" -); - -/// A test scenario that reports some tests as failed. -static void -test_fail(void) -{ - std::cout << fail_message2; + ); std::exit(EXIT_FAILURE); } -/// An example pass message for `test_pass()`. -const char pass_message2[] = ( + +/// A test scenario that passes. +static void +test_pass(void) +{ + std::cout << ( "Note: Google Test filter = Suite.pass\n" "[==========] Running 1 test from 1 test suite.\n" "[----------] Global test environment set-up.\n" @@ -185,18 +190,15 @@ const char pass_message2[] = ( "[----------] Global test environment tear-down\n" "[==========] 1 test from 1 test suite ran. (0 ms total)\n" "[ PASSED ] 1 test.\n" -); - -/// A test scenario that passes. -static void -test_pass(void) -{ - std::cout << pass_message2; + ); } -/// An example pass message for `test_pass_but_exit_failure()`. -const char pass_message3[] = ( +/// A test scenario that passes but then exits with non-zero. +static void +test_pass_but_exit_failure(void) +{ + std::cout << ( "Note: Google Test filter = Suite.pass_but_exit_failure\n" "[==========] Running 1 test from 1 test suite.\n" "[----------] Global test environment set-up.\n" @@ -208,26 +210,12 @@ const char pass_message3[] = ( "[----------] Global test environment tear-down\n" "[==========] 1 test from 1 test suite ran. (0 ms total)\n" "[ PASSED ] 1 test.\n" -); + ); -/// A test scenario that passes but then exits with non-zero. -static void -test_pass_but_exit_failure(void) -{ - std::cout << pass_message3; std::exit(70); } -/// An example incomplete output for `test_timeout`. -const char incomplete_test[] = ( -"Note: Google Test filter = Suite.incomplete\n" -"[==========] Running 1 test from 1 test suite.\n" -"[----------] Global test environment set-up.\n" -"[----------] 1 test from Suite\n" -"[ RUN ] Suite.incomplete\n" -); - /// A test scenario that times out. /// /// Note that the timeout is defined in the Kyuafile, as the TAP interface has @@ -235,7 +223,13 @@ const char incomplete_test[] = ( static void test_timeout(void) { - std::cout << incomplete_test; + std::cout << ( +"Note: Google Test filter = Suite.incomplete\n" +"[==========] Running 1 test from 1 test suite.\n" +"[----------] Global test environment set-up.\n" +"[----------] 1 test from Suite\n" +"[ RUN ] Suite.incomplete\n" + ); ::sleep(10); const fs::path control_dir = fs::path(utils::getenv("CONTROL_DIR").get()); @@ -253,7 +247,7 @@ test_timeout(void) static void usage(const char* argv0) { std::cout << "usage: " << argv0 << " " - << "[--gtest_color=*] " + << "[--gtest_color=(auto|yes|no)] " << "[--gtest_filter=POSITIVE_PATTERNS] " << "[--gtest_list_tests]" << "\n\n" @@ -281,14 +275,9 @@ main(int argc, char** argv) { using scenario_fn_t = void (*)(void); - std::map scenarios; - std::string testcase; - - const char *gtest_color_flag = "--gtest_color="; - const char *gtest_filter_flag = "--gtest_filter="; char *argv0 = argv[0]; - bool list_tests = false; + std::map scenarios; scenarios["check_configuration_variables"] = test_check_configuration_variables; @@ -298,34 +287,23 @@ main(int argc, char** argv) scenarios["pass_but_exit_failure"] = test_pass_but_exit_failure; scenarios["timeout"] = test_timeout; - /// NB: this avoids getopt_long*, because its behavior isn't portable. - int i; - for (i = 1; i < argc; i++) { - if (strcmp("--gtest_list_tests", argv[i]) == 0) { - list_tests = true; - } - /// Ignore `--gtest_color` arguments. - else if (strncmp(gtest_color_flag, argv[i], - strlen(gtest_color_flag)) == 0) { - continue; - } - /// Try looking for the test name with `--gtest_filter`. - else { - std::string filter_flag_match = - std::string(gtest_filter_flag) + test_suite; - std::string arg = std::string(argv[i]); - if (arg.find_first_of(filter_flag_match) == arg.npos) { - usage(argv0); - } - testcase = arg.erase(0, filter_flag_match.length()); - } - } + const bool_option gtest_list_tests_opt( + "gtest_list_tests", "List tests"); + const string_option gtest_color_opt( + "gtest_color", "Enable/disable color support", "auto"); + const string_option gtest_filter_opt( + "gtest_filter", "", "POSITIVE_PATTERNS"); - if (i < argc) { - usage(argv0); - } + std::vector options; + /// Ignore `--gtest_color=*`. + options.push_back(>est_color_opt); + options.push_back(>est_filter_opt); + options.push_back(>est_list_tests_opt); - if (list_tests) { + const parsed_cmdline cmdline = parse(argc, argv, options); + INV(cmdline.arguments().empty()); + + if (cmdline.has_option("gtest_list_tests")) { std::cout << test_suite << "\n"; for (auto it = scenarios.begin(); it != scenarios.end(); it++) { std::cout << " " << it->first << "\n"; @@ -333,8 +311,16 @@ main(int argc, char** argv) return EXIT_SUCCESS; } + INV(cmdline.has_option("gtest_filter")); + + auto gtest_filter_arg = cmdline.get_option("gtest_filter"); + INV(gtest_filter_arg.find_first_of(test_suite) == 0); + auto testcase = gtest_filter_arg.erase(0, strlen(test_suite)); + auto scenario = scenarios.find(testcase); if (scenario == scenarios.end()) { + /// Mimic googletest test programs by printing out a usage message when + /// a test cannot be found. usage(argv0); } diff --git a/engine/googletest_list.cpp b/engine/googletest_list.cpp index e7bb52d5..0bd85248 100644 --- a/engine/googletest_list.cpp +++ b/engine/googletest_list.cpp @@ -51,29 +51,17 @@ const std::string name_expr = "([[:alpha:][:digit:]_]+[[:alpha:][:digit:]_/]*)"; /// The separator between a test suite and a test case. const std::string testsuite_testcase_separator = "."; -/// A complete regular expression representing a line with a test suite -/// definition. -/// -/// e.g., -/// "TestSuite." -/// or -/// "TestSuite/Prefix." -/// or -/// "TestSuite/Prefix. # TypeParam = .+" -const std::string testsuite_expr = - name_expr + "\\.([[:space:]]+# TypeParam = .+)?"; - /// A complete regular expression representing a line with a test case -/// definition. -/// -/// e.g., -/// " TestCase" -/// or -/// " TestCase/0" -/// or -/// " TestCase/0 # GetParam() = 4" -const std::string testcase_expr = - " " + name_expr + "([[:space:]]+# GetParam\\(\\) = .+)?"; +/// definition, e,g., " TestCase", " TestCase/0", or +/// " TestCase/0 # GetParam() = 4". +const std::regex testcase_re( + " " + name_expr + "([[:space:]]+# GetParam\\(\\) = .+)?"); + +/// A complete regular expression representing a line with a test suite +/// definition, e,g., * "TestSuite.", "TestSuite/Prefix.", or +/// "TestSuite/Prefix. # TypeParam = .+". +const std::regex testsuite_re( + name_expr + "\\.([[:space:]]+# TypeParam = .+)?"); } // anonymous namespace @@ -88,13 +76,11 @@ const std::string testcase_expr = model::test_cases_map engine::parse_googletest_list(std::istream& input) { - std::regex testcase_re(testcase_expr), testsuite_re(testsuite_expr); - std::smatch match; - std::string line; - std::string testcase_name, test_suite; + std::string line, test_suite; model::test_cases_map_builder test_cases_builder; while (std::getline(input, line).good()) { + std::smatch match; if (std::regex_match(line, match, testcase_re)) { std::string test_case; @@ -107,9 +93,10 @@ engine::parse_googletest_list(std::istream& input) } else if (std::regex_match(line, match, testsuite_re)) { test_suite = std::string(match[1]) + testsuite_testcase_separator; + } else { + /// ignore the line; something might have used output a diagnostic + /// message to stdout, e.g., gtest_main. } - // else, ignore the line; something might have used output a diagnostic - // message to stdout, e.g., gtest_main. } const model::test_cases_map test_cases = test_cases_builder.build(); if (test_cases.empty()) { diff --git a/engine/googletest_list_test.cpp b/engine/googletest_list_test.cpp index 7a2085c1..6b157bc1 100644 --- a/engine/googletest_list_test.cpp +++ b/engine/googletest_list_test.cpp @@ -50,16 +50,12 @@ namespace units = utils::units; ATF_TEST_CASE_WITHOUT_HEAD(parse_googletest_list__invalid_testcase_definition); ATF_TEST_CASE_BODY(parse_googletest_list__invalid_testcase_definition) { - const std::string text1 = - " \n"; - const std::string text2 = - " TestcaseWithoutSuite\n"; - std::istringstream input1(text1); - std::istringstream input2(text2); + std::istringstream input1(" \n"); ATF_REQUIRE_THROW_RE(engine::format_error, "No test cases", engine::parse_googletest_list(input1)); + std::istringstream input2(" TestcaseWithoutSuite\n"); ATF_REQUIRE_THROW_RE(engine::format_error, "Invalid testcase definition: not preceded by a test suite definition", engine::parse_googletest_list(input2)); @@ -69,15 +65,13 @@ ATF_TEST_CASE_BODY(parse_googletest_list__invalid_testcase_definition) ATF_TEST_CASE_WITHOUT_HEAD(parse_googletest_list__invalid_testsuite_definition); ATF_TEST_CASE_BODY(parse_googletest_list__invalid_testsuite_definition) { - const std::string text1 = - "\n"; - const std::string text2 = - "TestSuiteWithoutSeparator\n"; - std::istringstream input1(text1); - std::istringstream input2(text2); - + std::istringstream input1("\n"); ATF_REQUIRE_THROW_RE(engine::format_error, "No test cases", engine::parse_googletest_list(input1)); + + std::istringstream input2( +"TestSuiteWithoutSeparator\n" + ); ATF_REQUIRE_THROW_RE(engine::format_error, "No test cases", engine::parse_googletest_list(input2)); } @@ -86,8 +80,7 @@ ATF_TEST_CASE_BODY(parse_googletest_list__invalid_testsuite_definition) ATF_TEST_CASE_WITHOUT_HEAD(parse_googletest_list__no_test_cases); ATF_TEST_CASE_BODY(parse_googletest_list__no_test_cases) { - const std::string text = ""; - std::istringstream input(text); + std::istringstream input(""); ATF_REQUIRE_THROW_RE(engine::format_error, "No test cases", engine::parse_googletest_list(input)); } @@ -96,9 +89,10 @@ ATF_TEST_CASE_BODY(parse_googletest_list__no_test_cases) ATF_TEST_CASE_WITHOUT_HEAD(parse_googletest_list__one_test_case); ATF_TEST_CASE_BODY(parse_googletest_list__one_test_case) { - const std::string text = - "TestSuite.\n" - " TestCase\n"; + std::string text = +"TestSuite.\n" +" TestCase\n;" + ; std::istringstream input(text); const model::test_cases_map tests = engine::parse_googletest_list(input); @@ -111,9 +105,10 @@ ATF_TEST_CASE_BODY(parse_googletest_list__one_test_case) ATF_TEST_CASE_WITHOUT_HEAD(parse_googletest_list__one_parameterized_test_case); ATF_TEST_CASE_BODY(parse_googletest_list__one_parameterized_test_case) { - const std::string text = - "TestSuite.\n" - " TestCase/0 # GetParam() = 'c'\n"; + std::string text = +"TestSuite.\n" +" TestCase/0 # GetParam() = 'c'\n" + ; std::istringstream input(text); const model::test_cases_map tests = engine::parse_googletest_list(input); @@ -126,9 +121,10 @@ ATF_TEST_CASE_BODY(parse_googletest_list__one_parameterized_test_case) ATF_TEST_CASE_WITHOUT_HEAD(parse_googletest_list__one_parameterized_test_suite); ATF_TEST_CASE_BODY(parse_googletest_list__one_parameterized_test_suite) { - const std::string text = - "TestSuite/0. # TypeParam = int\n" - " TestCase\n"; + std::string text = +"TestSuite/0. # TypeParam = int\n" +" TestCase\n" + ; std::istringstream input(text); const model::test_cases_map tests = engine::parse_googletest_list(input); @@ -141,10 +137,11 @@ ATF_TEST_CASE_BODY(parse_googletest_list__one_parameterized_test_suite) ATF_TEST_CASE_WITHOUT_HEAD(parse_googletest_list__one_parameterized_test_case_and_test_suite); ATF_TEST_CASE_BODY(parse_googletest_list__one_parameterized_test_case_and_test_suite) { - const std::string text = - "TestSuite/0. # TypeParam = int\n" - " TestCase/0 # GetParam() = \"herp\"\n" - " TestCase/1 # GetParam() = \"derp\"\n"; + std::string text = +"TestSuite/0. # TypeParam = int\n" +" TestCase/0 # GetParam() = \"herp\"\n" +" TestCase/1 # GetParam() = \"derp\"\n" + ; std::istringstream input(text); const model::test_cases_map tests = engine::parse_googletest_list(input); @@ -159,24 +156,25 @@ ATF_TEST_CASE_BODY(parse_googletest_list__one_parameterized_test_case_and_test_s ATF_TEST_CASE_WITHOUT_HEAD(parse_googletest_list__many_test_cases); ATF_TEST_CASE_BODY(parse_googletest_list__many_test_cases) { - const std::string text = - "FirstTestSuite.\n" - " ATestCase\n" - "SecondTestSuite.\n" - " AnotherTestCase\n" - "ThirdTestSuite.\n" - " _\n" - "FourthTestSuite/0. # TypeParam = std::list\n" - " TestCase\n" - "FourthTestSuite/1. # TypeParam = std::list\n" - " TestCase\n" - "FifthTestSuite.\n" - " TestCase/0 # GetParam() = 0\n" - " TestCase/1 # GetParam() = (1, 2, 3)\n" - " TestCase/2 # GetParam() = \"developers. developers\"\n" - "SixthTestSuite/0. # TypeParam = std::map\n" - " TestCase/0 # GetParam() = 0\n" - " TestCase/1 # GetParam() = (1, 2, 3)\n"; + std::string text = +"FirstTestSuite.\n" +" ATestCase\n" +"SecondTestSuite.\n" +" AnotherTestCase\n" +"ThirdTestSuite.\n" +" _\n" +"FourthTestSuite/0. # TypeParam = std::list\n" +" TestCase\n" +"FourthTestSuite/1. # TypeParam = std::list\n" +" TestCase\n" +"FifthTestSuite.\n" +" TestCase/0 # GetParam() = 0\n" +" TestCase/1 # GetParam() = (1, 2, 3)\n" +" TestCase/2 # GetParam() = \"developers. developers\"\n" +"SixthTestSuite/0. # TypeParam = std::map\n" +" TestCase/0 # GetParam() = 0\n" +" TestCase/1 # GetParam() = (1, 2, 3)\n" + ; std::istringstream input(text); const model::test_cases_map tests = engine::parse_googletest_list(input); diff --git a/engine/googletest_result.cpp b/engine/googletest_result.cpp index 18f8ce1d..ea02d80a 100644 --- a/engine/googletest_result.cpp +++ b/engine/googletest_result.cpp @@ -53,6 +53,24 @@ using utils::optional; namespace { +/// Internal string for specifying invalid output. +const std::string invalid_output_message = "invalid output"; + +/// Regular expression for disabled tests line. +const std::regex disabled_re( + R"RE((YOU HAVE [[:digit:]]+ DISABLED TESTS?))RE" +); + +/// Regular expression for starting sentinel of results block. +const std::regex starting_sentinel_re( + R"(\[[[:space:]]+RUN[[:space:]]+\][[:space:]]+[A-Za-z0-9]+\.[A-Za-z0-9]+)" +); + +/// Regular expression for ending sentinel of results block. +const std::regex ending_sentinel_re( + R"RE(\[[[:space:]]+(FAILED|OK|SKIPPED)[[:space:]]+\])RE" +); + /// Parses a test result that does not accept a reason. /// /// \param status The result status name. @@ -131,7 +149,7 @@ format_status(const process::status& status) /// Constructs a raw result with a type. /// -/// The reason and the argument are left uninitialized. +/// The reason is left uninitialized. /// /// \param type_ The type of the result. engine::googletest_result::googletest_result(const types type_) : @@ -142,38 +160,16 @@ engine::googletest_result::googletest_result(const types type_) : /// Constructs a raw result with a type and a reason. /// -/// The argument is left uninitialized. -/// /// \param type_ The type of the result. /// \param reason_ The reason for the result. -engine::googletest_result::googletest_result(const types type_, - const std::string& reason_) : +engine::googletest_result::googletest_result( + const types type_, + const std::string& reason_) : _type(type_), _reason(reason_) { } -/// Constructs a raw result with a type, an optional argument and a reason. -/// -/// \param type_ The type of the result. -/// \param argument_ The optional argument for the result. -/// \param reason_ The reason for the result. -engine::googletest_result::googletest_result(const types type_, - const utils::optional< int >& argument_, - const std::string& reason_) : - _type(type_), _argument(argument_), _reason(reason_) -{ -} - - -const std::string invalid_output_message = "invalid output"; -const std::regex disabled_re(R"RE((YOU HAVE [[:digit:]]+ DISABLED TESTS?))RE"); -const std::regex starting_sentinel_re( - R"(\[[[:space:]]+RUN[[:space:]]+\][[:space:]]+[A-Za-z0-9]+\.[A-Za-z0-9]+)"); -const std::regex ending_sentinel_re( - R"RE(\[[[:space:]]+(FAILED|OK|SKIPPED)[[:space:]]+\])RE"); - - /// Parses an input stream to extract a test result. /// /// If the parsing fails for any reason, the test result is 'broken' and it @@ -191,9 +187,6 @@ engine::googletest_result engine::googletest_result::parse(std::istream& input) { std::vector lines; - std::smatch matches; - std::string context, googletest_res, status; - bool capture_context, valid_output; do { std::string line; @@ -203,7 +196,11 @@ engine::googletest_result::parse(std::istream& input) lines.push_back(line); } while (input.good()); - valid_output = false; + bool capture_context; + bool valid_output = false; + std::smatch matches; + std::string context, status; + for (auto& line: lines) { if (regex_search(line, matches, disabled_re)) { context = matches[1]; @@ -217,7 +214,7 @@ engine::googletest_result::parse(std::istream& input) continue; } if (regex_search(line, matches, ending_sentinel_re)) { - googletest_res = matches[1]; + std::string googletest_res = matches[1]; if (googletest_res == "OK") { context = ""; status = "successful"; @@ -275,16 +272,6 @@ engine::googletest_result::type(void) const } -/// Gets the optional argument of the result. -/// -/// \return The argument of the result if present; none otherwise. -const optional< int >& -engine::googletest_result::argument(void) const -{ - return _argument; -} - - /// Gets the optional reason of the result. /// /// \return The reason of the result if present; none otherwise. @@ -341,38 +328,46 @@ engine::googletest_result::apply(const optional< process::status >& status) } INV(status); + if (_type == googletest_result::broken) { + return *this; + } + if (_type == googletest_result::failed) { + if (status.get().exited() && + status.get().exitstatus() == EXIT_FAILURE) { + return *this; + } + } else { + if (status.get().exited() && + status.get().exitstatus() == EXIT_SUCCESS) { + return *this; + } + } + switch (_type) { case googletest_result::broken: - return *this; + /// Tautilogically impossible case; however, clang 8.x doesn't realize + /// that this is not possible. + INV(false); - case googletest_result::failed: - if (status.get().exited() && status.get().exitstatus() == EXIT_FAILURE) - return *this; + case googletest_result::disabled: return googletest_result(googletest_result::broken, - "Failed test case should have reported failure but " + + "Disabled test case should have reported success but " + format_status(status.get())); - case googletest_result::disabled: - if (status.get().exited() && status.get().exitstatus() == EXIT_SUCCESS) - return *this; + case googletest_result::failed: return googletest_result(googletest_result::broken, - "Disabled test case should have reported success but " + + "Failed test case should have reported failure but " + format_status(status.get())); case googletest_result::skipped: - if (status.get().exited() && status.get().exitstatus() == EXIT_SUCCESS) - return *this; return googletest_result(googletest_result::broken, "Skipped test case should have reported success but " + format_status(status.get())); case googletest_result::successful: - if (status.get().exited() && status.get().exitstatus() == EXIT_SUCCESS) - return *this; return googletest_result(googletest_result::broken, "Passed test case should have reported success but " + format_status(status.get())); - } UNREACHABLE; @@ -389,12 +384,12 @@ engine::googletest_result::externalize(void) const case googletest_result::broken: return model::test_result(model::test_result_broken, _reason.get()); - case googletest_result::failed: - return model::test_result(model::test_result_failed, _reason.get()); - case googletest_result::disabled: return model::test_result(model::test_result_skipped, _reason.get()); + case googletest_result::failed: + return model::test_result(model::test_result_failed, _reason.get()); + case googletest_result::skipped: return model::test_result(model::test_result_skipped, _reason.get()); @@ -415,7 +410,7 @@ engine::googletest_result::externalize(void) const bool engine::googletest_result::operator==(const googletest_result& other) const { - return _type == other._type && _argument == other._argument && + return _type == other._type && _reason == other._reason; } @@ -450,13 +445,10 @@ engine::operator<<(std::ostream& output, const googletest_result& object) case googletest_result::successful: result_name = "successful"; break; } - const optional< int >& argument = object.argument(); - const optional< std::string >& reason = object.reason(); - output << F("model::test_result{type=%s, argument=%s, reason=%s}") + output << F("model::test_result{type=%s, reason=%s}") % text::quote(result_name, '\'') - % (argument ? (F("%s") % argument.get()).str() : "none") % (reason ? text::quote(reason.get(), '\'') : "none"); return output; diff --git a/engine/googletest_result.hpp b/engine/googletest_result.hpp index 23d544de..f89224d0 100644 --- a/engine/googletest_result.hpp +++ b/engine/googletest_result.hpp @@ -56,21 +56,16 @@ class googletest_result { /// List of possible types for the test case result. enum types { broken, - successful, - skipped, + disabled, failed, - disabled + skipped, + successful, }; private: /// The test case result. types _type; - /// The optional integral argument that may accompany the result. - /// - /// Should only be present if the type is expected_exit or expected_signal. - utils::optional< int > _argument; - /// A description of the test case result. /// /// Should always be present except for the passed type and sometimes with @@ -80,14 +75,11 @@ class googletest_result { public: googletest_result(const types); googletest_result(const types, const std::string&); - googletest_result(const types, const utils::optional< int >&, - const std::string&); static googletest_result parse(std::istream&); static googletest_result load(const utils::fs::path&); types type(void) const; - const utils::optional< int >& argument(void) const; const utils::optional< std::string >& reason(void) const; bool good(void) const; diff --git a/engine/googletest_result_test.cpp b/engine/googletest_result_test.cpp index 02851433..51003f9e 100644 --- a/engine/googletest_result_test.cpp +++ b/engine/googletest_result_test.cpp @@ -58,19 +58,16 @@ namespace { /// Performs a test for results::parse() that should succeed. /// /// \param exp_type The expected type of the result. -/// \param exp_argument The expected argument in the result, if any. /// \param exp_reason The expected reason describing the result, if any. /// \param text The literal input to parse; can include multiple lines. static void parse_ok_test(const engine::googletest_result::types& exp_type, - const optional< int >& exp_argument, const char* exp_reason, const char* text) { std::istringstream input(text); const engine::googletest_result actual = engine::googletest_result::parse(input); ATF_REQUIRE_EQ(exp_type, actual.type()); - ATF_REQUIRE_EQ(exp_argument, actual.argument()); if (exp_reason != NULL) { ATF_REQUIRE(actual.reason()); ATF_REQUIRE_EQ(exp_reason, actual.reason().get()); @@ -85,14 +82,13 @@ parse_ok_test(const engine::googletest_result::types& exp_type, /// \param name The name of the test case; will be prefixed with /// "googletest_result__parse__". /// \param exp_type The expected type of the result. -/// \param exp_argument The expected argument in the result, if any. /// \param exp_reason The expected reason describing the result, if any. /// \param input The literal input to parse. -#define PARSE(name, exp_type, exp_argument, exp_reason, input) \ +#define PARSE(name, exp_type, exp_reason, input) \ ATF_TEST_CASE_WITHOUT_HEAD(googletest_result__parse__ ## name); \ ATF_TEST_CASE_BODY(googletest_result__parse__ ## name) \ { \ - parse_ok_test(exp_type, exp_argument, exp_reason, input); \ + parse_ok_test(exp_type, exp_reason, input); \ } @@ -100,7 +96,7 @@ parse_ok_test(const engine::googletest_result::types& exp_type, PARSE(broken, - engine::googletest_result::broken, none, "invalid output", + engine::googletest_result::broken, "invalid output", "invalid input"); const char disabled_context[] = ( @@ -118,7 +114,7 @@ const char disabled_message[] = ( ); PARSE(disabled, - engine::googletest_result::disabled, none, disabled_context, + engine::googletest_result::disabled, disabled_context, disabled_message); const char failed_context[] = ( @@ -151,7 +147,7 @@ const char failed_message[] = ( ); PARSE(failed, - engine::googletest_result::failed, none, failed_context, + engine::googletest_result::failed, failed_context, failed_message); const char skipped_message[] = ( @@ -171,7 +167,7 @@ const char skipped_message[] = ( ); PARSE(skipped, - engine::googletest_result::skipped, none, NULL, + engine::googletest_result::skipped, NULL, skipped_message ); @@ -197,7 +193,7 @@ const char skipped_with_reason_message[] = ( ); PARSE(skipped_with_reason, - engine::googletest_result::skipped, none, skipped_with_reason_context, + engine::googletest_result::skipped, skipped_with_reason_context, skipped_with_reason_message); const char successful_message[] = ( @@ -215,7 +211,7 @@ const char successful_message[] = ( ); PARSE(successful, - engine::googletest_result::successful, none, NULL, + engine::googletest_result::successful, NULL, successful_message); const char successful_message2[] = ( @@ -233,7 +229,7 @@ const char successful_message2[] = ( ); PARSE(successful2, - engine::googletest_result::successful, none, NULL, + engine::googletest_result::successful, NULL, successful_message2); const char successful_parameterized_message[] = ( @@ -251,7 +247,7 @@ const char successful_parameterized_message[] = ( ); PARSE(successful_parameterized, - engine::googletest_result::successful, none, NULL, + engine::googletest_result::successful, NULL, successful_parameterized_message); const char successful_message_with_reason[] = ( @@ -270,7 +266,7 @@ const char successful_message_with_reason[] = ( ); PARSE(successful_with_reason, - engine::googletest_result::successful, none, NULL, + engine::googletest_result::successful, NULL, successful_message_with_reason); ATF_TEST_CASE_WITHOUT_HEAD(googletest_result__load__ok); @@ -284,7 +280,6 @@ ATF_TEST_CASE_BODY(googletest_result__load__ok) const engine::googletest_result result = engine::googletest_result::load( utils::fs::path("result.txt")); ATF_REQUIRE_EQ(engine::googletest_result::skipped, result.type()); - ATF_REQUIRE(!result.argument()); ATF_REQUIRE(result.reason()); ATF_REQUIRE_EQ(skipped_with_reason_context, result.reason().get()); }