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

Add support for outputting missing tests #8240

Merged
merged 8 commits into from
Sep 17, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/root/install/tools/route_table_check_tool.rst
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ Usage

--covall
Enables comprehensive code coverage percent calculation taking into account all the possible
asserts.
asserts. Displays missing tests.

--disable-deprecation-check
Disables the deprecation check for RouteConfiguration proto.
Expand All @@ -65,7 +65,7 @@ Output
The program exits with status EXIT_FAILURE if any test case does not match the expected route parameter
value.

If a test fails, details of the failed test cases are printed if ``-details`` flag is provided.
If a test fails, details of the failed test cases are printed if ``--details`` flag is provided.
The first field is the expected route parameter value. The second field is the actual route parameter value.
The third field indicates the parameter that is compared.
In the following example, Test_2 and Test_5 failed while the other tests
Expand Down
1 change: 1 addition & 0 deletions docs/root/intro/version_history.rst
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ Version history
* router check tool: add comprehensive coverage reporting.
* router check tool: add deprecated field check.
* router check tool: add flag for only printing results of failed tests.
* router check tool: add support for outputting missing tests in the detailed coverage report.
* server: added a post initialization lifecycle event, in addition to the existing startup and shutdown events.
* thrift_proxy: fix crashing bug on invalid transport/protocol framing
* tls: added verification of IP address SAN fields in certificates against configured SANs in the
Expand Down
21 changes: 21 additions & 0 deletions test/tools/router_check/coverage.cc
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,25 @@ double Coverage::report() {
return 100 * static_cast<double>(covered_routes_.size()) / num_routes;
}

void Coverage::printMissingTests(const std::set<std::string>& all_route_names,
const std::set<std::string>& covered_route_names) {
std::set<std::string> missing_route_names;
std::set_difference(all_route_names.begin(), all_route_names.end(), covered_route_names.begin(),
covered_route_names.end(),
std::inserter(missing_route_names, missing_route_names.end()));
for (const auto& host : route_config_.virtual_hosts()) {
for (const auto& route : host.routes()) {
if (missing_route_names.find(route.name()) != missing_route_names.end()) {
std::cout << "Missing test for host: " << host.name()
<< ", route: " << route.match().DebugString() << std::endl;
}
}
}
}

double Coverage::detailedReport() {
std::set<std::string> all_route_names;
std::set<std::string> covered_route_names;
uint64_t num_routes = 0;
for (const auto& host : route_config_.virtual_hosts()) {
for (const auto& route : host.routes()) {
Expand All @@ -62,12 +80,15 @@ double Coverage::detailedReport() {
} else {
num_routes += 1;
}
all_route_names.emplace(route.name());
}
}
double cumulative_coverage = 0;
for (auto& covered_route : covered_routes_) {
cumulative_coverage += covered_route->report();
covered_route_names.emplace(covered_route->route().routeName());
}
printMissingTests(all_route_names, covered_route_names);
return 100 * cumulative_coverage / num_routes;
}

Expand Down
3 changes: 3 additions & 0 deletions test/tools/router_check/coverage.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ class RouteCoverage : Logger::Loggable<Logger::Id::testing> {
void setHostRewriteCovered() { host_rewrite_covered_ = true; }
void setRedirectPathCovered() { redirect_path_covered_ = true; }
bool covers(const Envoy::Router::RouteEntry* route) { return &route_ == route; }
const Envoy::Router::RouteEntry& route() { return route_; }

private:
const Envoy::Router::RouteEntry& route_;
Expand Down Expand Up @@ -45,6 +46,8 @@ class Coverage : Logger::Loggable<Logger::Id::testing> {
void markRedirectPathCovered(const Envoy::Router::RouteEntry& route);
double report();
double detailedReport();
void printMissingTests(const std::set<std::string>& all_route_names,
const std::set<std::string>& covered_route_names);

private:
RouteCoverage& coveredRoute(const Envoy::Router::RouteEntry& route);
Expand Down
11 changes: 11 additions & 0 deletions test/tools/router_check/router.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "common/network/utility.h"
#include "common/protobuf/message_validator_impl.h"
#include "common/protobuf/utility.h"
#include "common/runtime/runtime_impl.h"
#include "common/stream_info/stream_info_impl.h"

#include "test/test_common/printers.h"
Expand Down Expand Up @@ -71,6 +72,7 @@ RouterCheckTool RouterCheckTool::create(const std::string& router_config_file,
auto stats = std::make_unique<Stats::IsolatedStoreImpl>();
auto api = Api::createApiForTest(*stats);
TestUtility::loadFromFile(router_config_file, route_config, *api);
assignUniqueRouteNames(route_config);

auto factory_context = std::make_unique<NiceMock<Server::Configuration::MockFactoryContext>>();
auto config = std::make_unique<Router::ConfigImpl>(route_config, *factory_context, false);
Expand All @@ -84,6 +86,15 @@ RouterCheckTool RouterCheckTool::create(const std::string& router_config_file,
std::move(api), Coverage(route_config));
}

void RouterCheckTool::assignUniqueRouteNames(envoy::api::v2::RouteConfiguration& route_config) {
Runtime::RandomGeneratorImpl random;
for (auto& host : *route_config.mutable_virtual_hosts()) {
for (auto& route : *host.mutable_routes()) {
route.set_name(random.uuid());
}
}
}

RouterCheckTool::RouterCheckTool(
std::unique_ptr<NiceMock<Server::Configuration::MockFactoryContext>> factory_context,
std::unique_ptr<Router::ConfigImpl> config, std::unique_ptr<Stats::IsolatedStoreImpl> stats,
Expand Down
5 changes: 5 additions & 0 deletions test/tools/router_check/router.h
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,11 @@ class RouterCheckTool : Logger::Loggable<Logger::Id::testing> {
std::unique_ptr<Router::ConfigImpl> config, std::unique_ptr<Stats::IsolatedStoreImpl> stats,
Api::ApiPtr api, Coverage coverage);

/**
* Set UUID as the name for each route for detecting missing tests during the coverage check.
*/
static void assignUniqueRouteNames(envoy::api::v2::RouteConfiguration& route_config);

bool compareCluster(ToolConfig& tool_config, const std::string& expected);
bool compareCluster(ToolConfig& tool_config,
const envoy::RouterCheckToolSchema::ValidationAssert& expected);
Expand Down
8 changes: 8 additions & 0 deletions test/tools/router_check/test/route_tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -86,3 +86,11 @@ FAILURE_OUTPUT=$("${PATH_BIN}" "-c" "${PATH_CONFIG}/TestRoutes.yaml" "-t" "${PAT
if [[ "${FAILURE_OUTPUT}" != *"Test_2"*"expected: [cluster1], actual: [instant-server], test type: cluster_name"* ]] || [[ "${FAILURE_OUTPUT}" == *"Test_1"* ]]; then
exit 1
fi

# Missing test results
echo "testing missing tests output test cases"
MISSING_OUTPUT=$("${PATH_BIN}" "-c" "${PATH_CONFIG}/TestRoutes.yaml" "-t" "${PATH_CONFIG}/TestRoutes.golden.proto.json" "--details" "--useproto" "--covall" 2>&1) ||
echo "${MISSING_OUTPUT:-no-output}"
if [[ "${MISSING_OUTPUT}" != *"Missing test for host: www2_staging, route: prefix: \"/\""*"Missing test for host: default, route: prefix: \"/api/application_data\""* ]]; then
exit 1
fi