Skip to content

Commit

Permalink
Address some of @jmmv's comments in #203
Browse files Browse the repository at this point in the history 
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 <yaneurabeya@gmail.com>
  • Loading branch information
@ngie-eign
ngie-eign committed May 20, 2019
1 parent ff52f04 commit fa43d07
Show file tree
Hide file tree
Showing 7 changed files with 242 additions and 296 deletions.
38 changes: 17 additions & 21 deletions engine/googletest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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) {
Expand Down Expand Up @@ -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);
}

Expand Down
208 changes: 97 additions & 111 deletions engine/googletest_helpers.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,20 +42,30 @@ extern char** environ;
#include <iostream>

#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.
Expand All @@ -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"
Expand All @@ -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);
}
}
Expand All @@ -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"
Expand All @@ -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"
Expand All @@ -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"
Expand All @@ -208,34 +210,26 @@ 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
/// no means for test programs to specify this by themselves.
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());
Expand All @@ -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"
Expand Down Expand Up @@ -281,14 +275,9 @@ main(int argc, char** argv)
{
using scenario_fn_t = void (*)(void);

std::map<std::string, scenario_fn_t> 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<std::string, scenario_fn_t> scenarios;

scenarios["check_configuration_variables"] =
test_check_configuration_variables;
Expand All @@ -298,43 +287,40 @@ 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<const base_option*> options;
/// Ignore `--gtest_color=*`.
options.push_back(&gtest_color_opt);
options.push_back(&gtest_filter_opt);
options.push_back(&gtest_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";
}
return EXIT_SUCCESS;
}

INV(cmdline.has_option("gtest_filter"));

auto gtest_filter_arg = cmdline.get_option<string_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);
}

Expand Down
Loading

0 comments on commit fa43d07

Please sign in to comment.