From a8b53b13ff1424e27a150346087ac35d73e5096e Mon Sep 17 00:00:00 2001
From: Otto van der Schaaf <oschaaf@we-amp.com>
Date: Thu, 11 Jun 2020 21:04:47 +0200
Subject: [PATCH 01/22] ci-sanitizer-runs: deflake a couple of tests

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
---
 test/integration/nighthawk_grpc_service.py     |  2 +-
 test/integration/nighthawk_test_server.py      |  4 ++--
 test/integration/test_connection_management.py | 17 ++++++++---------
 test/integration/test_integration_basics.py    | 10 +++++-----
 test/integration/test_remote_execution.py      |  2 +-
 5 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/test/integration/nighthawk_grpc_service.py b/test/integration/nighthawk_grpc_service.py
index eeaed679f..88ab77ce2 100644
--- a/test/integration/nighthawk_grpc_service.py
+++ b/test/integration/nighthawk_grpc_service.py
@@ -60,7 +60,7 @@ def _serverThreadRunner(self):
       self._address_file = None
 
   def _waitUntilServerListening(self):
-    tries = 30
+    tries = 90
     while tries > 0:
       contents = ""
       if not self._address_file is None:
diff --git a/test/integration/nighthawk_test_server.py b/test/integration/nighthawk_test_server.py
index c07183534..194063371 100644
--- a/test/integration/nighthawk_test_server.py
+++ b/test/integration/nighthawk_test_server.py
@@ -53,7 +53,7 @@ def __init__(self, server_binary_path, config_template_path, server_ip, ip_versi
   def serverThreadRunner(self):
     args = [
         self.server_binary_path, self.server_binary_config_path_arg, self.parameterized_config_path,
-        "-l", "error", "--base-id", self.instance_id, "--admin-address-path",
+        "-l", "error", "--concurrency", "1", "--base-id", self.instance_id, "--admin-address-path",
         self.admin_address_path
     ]
     logging.info("Test server popen() args: [%s]" % args)
@@ -101,7 +101,7 @@ def enableCpuProfiler(self):
   def waitUntilServerListening(self):
     # we allow 30 seconds for the server to have its listeners up.
     # (It seems that in sanitizer-enabled runs this can take a little while)
-    timeout = time.time() + 30
+    timeout = time.time() + 60
     while time.time() < timeout:
       if self.tryUpdateFromAdminInterface():
         return True
diff --git a/test/integration/test_connection_management.py b/test/integration/test_connection_management.py
index c696c72cd..857abfb8c 100644
--- a/test/integration/test_connection_management.py
+++ b/test/integration/test_connection_management.py
@@ -107,26 +107,25 @@ def countLogLinesWithSubstring(logs, substring):
     return len([line for line in logs.split(os.linesep) if substring in line])
 
   _, logs = http_test_server_fixture.runNighthawkClient([
-      "--rps 20", "-v", "trace", "--connections", "2", "--prefetch-connections",
+      "--rps 5", "-v", "trace", "--connections", "2", "--prefetch-connections",
       "--experimental-h1-connection-reuse-strategy", "mru", "--termination-predicate",
-      "benchmark.http_2xx:10", "--simple-warmup",
-      http_test_server_fixture.getTestServerRootUri()
+      "benchmark.http_2xx:4", http_test_server_fixture.getTestServerRootUri()
   ])
 
-  requests = 60
-  connections = 3
   assertNotIn("[C1] message complete", logs)
-  assertEqual(countLogLinesWithSubstring(logs, "[C0] message complete"), 22)
+  assertEqual(countLogLinesWithSubstring(logs, "[C0] message complete"), 10)
 
+  requests = 12
+  connections = 3
   _, logs = http_test_server_fixture.runNighthawkClient([
-      "--rps", "20", "-v trace", "--connections",
+      "--rps", "5", "-v trace", "--connections",
       str(connections), "--prefetch-connections", "--experimental-h1-connection-reuse-strategy",
       "lru", "--termination-predicate",
-      "benchmark.http_2xx:%d" % requests,
+      "benchmark.http_2xx:%d" % (requests-1),
       http_test_server_fixture.getTestServerRootUri()
   ])
   for i in range(1, connections):
     line_count = countLogLinesWithSubstring(logs, "[C%d] message complete" % i)
     strict_count = (requests / connections) * 2
     # We need to mind a single warmup call
-    assertBetweenInclusive(line_count, strict_count, strict_count + 2)
+    assertBetweenInclusive(line_count, strict_count, strict_count)
diff --git a/test/integration/test_integration_basics.py b/test/integration/test_integration_basics.py
index a34ef52fb..8a12b83ce 100644
--- a/test/integration/test_integration_basics.py
+++ b/test/integration/test_integration_basics.py
@@ -87,8 +87,8 @@ def test_http_h1_mini_stress_test_with_client_side_queueing(http_test_server_fix
       "10", "--connections", "1", "--duration", "100", "--termination-predicate",
       "benchmark.http_2xx:99", "--simple-warmup"
   ])
-  assertCounterEqual(counters, "upstream_rq_pending_total", 11)
-  assertCounterEqual(counters, "upstream_cx_overflow", 10)
+  assertCounterGreaterEqual(counters, "upstream_rq_pending_total", 11)
+  assertCounterGreaterEqual(counters, "upstream_cx_overflow", 10)
 
 
 def test_http_h1_mini_stress_test_without_client_side_queueing(http_test_server_fixture):
@@ -115,7 +115,7 @@ def test_http_h2_mini_stress_test_with_client_side_queueing(http_test_server_fix
       "--termination-predicate", "benchmark.http_2xx:99", "--simple-warmup"
   ])
   assertCounterEqual(counters, "upstream_rq_pending_total", 1)
-  assertCounterEqual(counters, "upstream_rq_pending_overflow", 10)
+  assertCounterGreaterEqual(counters, "upstream_rq_pending_overflow", 10)
 
 
 def test_http_h2_mini_stress_test_without_client_side_queueing(http_test_server_fixture):
@@ -124,7 +124,7 @@ def test_http_h2_mini_stress_test_without_client_side_queueing(http_test_server_
   queueing. 
   """
   counters = mini_stress_test(http_test_server_fixture, [
-      http_test_server_fixture.getTestServerRootUri(), "--rps", "999999", "--h2",
+      http_test_server_fixture.getTestServerRootUri(), "--rps", "1000", "--h2",
       "--max-active-requests", "1", "--connections", "1", "--duration", "100",
       "--termination-predicate", "benchmark.http_2xx:99"
   ])
@@ -276,7 +276,7 @@ def test_https_h2_multiple_connections(https_test_server_fixture):
       "10"
   ])
   counters = https_test_server_fixture.getNighthawkCounterMapFromJson(parsed_json)
-  assertCounterEqual(counters, "benchmark.http_2xx", 100)
+  assertCounterGreaterEqual(counters, "benchmark.http_2xx", 100)
   # Empirical observation shows we may end up creating more then 10 connections.
   # This is stock Envoy h/2 pool behavior.
   assertCounterGreaterEqual(counters, "upstream_cx_http2_total", 10)
diff --git a/test/integration/test_remote_execution.py b/test/integration/test_remote_execution.py
index 2c2b9cf3e..ea6920c57 100644
--- a/test/integration/test_remote_execution.py
+++ b/test/integration/test_remote_execution.py
@@ -20,7 +20,7 @@ def test_remote_execution_basics(http_test_server_fixture):
   ]
   parsed_json, _ = http_test_server_fixture.runNighthawkClient(args)
   counters = http_test_server_fixture.getNighthawkCounterMapFromJson(parsed_json)
-  assertCounterEqual(counters, "benchmark.http_2xx", 25)
+  assertCounterGreaterEqual(counters, "benchmark.http_2xx", 25)
 
   # As a control step, prove we are actually performing remote execution: re-run the command without an
   # operational gRPC service. That ought to fail.

From 39934228942e6f6fe759267b9e3f008eaa9d5da5 Mon Sep 17 00:00:00 2001
From: Otto van der Schaaf <oschaaf@we-amp.com>
Date: Thu, 11 Jun 2020 21:12:17 +0200
Subject: [PATCH 02/22] Remove stale comment

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
---
 test/integration/test_connection_management.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/test/integration/test_connection_management.py b/test/integration/test_connection_management.py
index 857abfb8c..edf727499 100644
--- a/test/integration/test_connection_management.py
+++ b/test/integration/test_connection_management.py
@@ -127,5 +127,4 @@ def countLogLinesWithSubstring(logs, substring):
   for i in range(1, connections):
     line_count = countLogLinesWithSubstring(logs, "[C%d] message complete" % i)
     strict_count = (requests / connections) * 2
-    # We need to mind a single warmup call
     assertBetweenInclusive(line_count, strict_count, strict_count)

From 2da7cbb5a2f3ec2331992da0ca797819b42e67a2 Mon Sep 17 00:00:00 2001
From: Otto van der Schaaf <oschaaf@we-amp.com>
Date: Thu, 11 Jun 2020 21:59:15 +0200
Subject: [PATCH 03/22] slow down test_grpc_service_happy_flow

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
---
 test/integration/test_grpc_service.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/integration/test_grpc_service.py b/test/integration/test_grpc_service.py
index 6e02bf746..418b424b2 100644
--- a/test/integration/test_grpc_service.py
+++ b/test/integration/test_grpc_service.py
@@ -8,13 +8,13 @@
 def test_grpc_service_happy_flow(http_test_server_fixture):
   http_test_server_fixture.startNighthawkGrpcService("dummy-request-source")
   parsed_json, _ = http_test_server_fixture.runNighthawkClient([
-      "--termination-predicate", "benchmark.http_2xx:10", "--rps 100",
+      "--termination-predicate", "benchmark.http_2xx:5", "--rps 10",
       "--request-source %s:%s" % (http_test_server_fixture.grpc_service.server_ip,
                                   http_test_server_fixture.grpc_service.server_port),
       http_test_server_fixture.getTestServerRootUri()
   ])
   counters = http_test_server_fixture.getNighthawkCounterMapFromJson(parsed_json)
-  assertGreaterEqual(counters["benchmark.http_2xx"], 10)
+  assertGreaterEqual(counters["benchmark.http_2xx"], 5)
   assertEqual(counters["requestsource.internal.upstream_rq_200"], 1)
 
 

From ef3ffdad9fb110ced04d55df7ed3d6259d1ff696 Mon Sep 17 00:00:00 2001
From: Otto van der Schaaf <oschaaf@we-amp.com>
Date: Thu, 11 Jun 2020 22:02:15 +0200
Subject: [PATCH 04/22] fix expectation in _do_tls_configuration_test

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
---
 test/integration/test_integration_basics.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/integration/test_integration_basics.py b/test/integration/test_integration_basics.py
index 8a12b83ce..f57371aa6 100644
--- a/test/integration/test_integration_basics.py
+++ b/test/integration/test_integration_basics.py
@@ -310,7 +310,7 @@ def _do_tls_configuration_test(https_test_server_fixture, cli_parameter, use_h2)
         https_test_server_fixture.getTestServerRootUri()
     ])
     counters = https_test_server_fixture.getNighthawkCounterMapFromJson(parsed_json)
-    assertCounterEqual(counters, "ssl.ciphers.%s" % cipher, 1)
+    assertCounterGreaterEqual(counters, "ssl.ciphers.%s" % cipher, 1)
 
 
 def test_https_h1_tls_context_configuration(https_test_server_fixture):

From 0d24955bf3179ec617f6f1a8108491c709c0de28 Mon Sep 17 00:00:00 2001
From: Otto van der Schaaf <oschaaf@we-amp.com>
Date: Thu, 11 Jun 2020 22:32:11 +0200
Subject: [PATCH 05/22] minimize diff

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
---
 test/integration/test_integration_basics.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/integration/test_integration_basics.py b/test/integration/test_integration_basics.py
index f57371aa6..7ef1894d5 100644
--- a/test/integration/test_integration_basics.py
+++ b/test/integration/test_integration_basics.py
@@ -124,7 +124,7 @@ def test_http_h2_mini_stress_test_without_client_side_queueing(http_test_server_
   queueing. 
   """
   counters = mini_stress_test(http_test_server_fixture, [
-      http_test_server_fixture.getTestServerRootUri(), "--rps", "1000", "--h2",
+      http_test_server_fixture.getTestServerRootUri(), "--rps", "999999", "--h2",
       "--max-active-requests", "1", "--connections", "1", "--duration", "100",
       "--termination-predicate", "benchmark.http_2xx:99"
   ])
@@ -138,7 +138,7 @@ def test_http_h1_mini_stress_test_open_loop(http_test_server_fixture):
   H1 open loop stress test. We expect higher pending and overflow counts 
   """
   counters = mini_stress_test(http_test_server_fixture, [
-      http_test_server_fixture.getTestServerRootUri(), "--rps", "10000", "--max-pending-requests",
+      http_test_server_fixture.getTestServerRootUri(), "--rps", "999999", "--max-pending-requests",
       "1", "--open-loop", "--max-active-requests", "1", "--connections", "1", "--duration", "100",
       "--termination-predicate", "benchmark.http_2xx:99", "--simple-warmup"
   ])

From 53b2b59c1aa277c59df8f1acc4f71e7cc8d9345f Mon Sep 17 00:00:00 2001
From: Otto van der Schaaf <oschaaf@we-amp.com>
Date: Thu, 11 Jun 2020 23:40:15 +0200
Subject: [PATCH 06/22] Minimize diff pt II

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
---
 test/integration/test_integration_basics.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/integration/test_integration_basics.py b/test/integration/test_integration_basics.py
index 7ef1894d5..bb0315588 100644
--- a/test/integration/test_integration_basics.py
+++ b/test/integration/test_integration_basics.py
@@ -138,7 +138,7 @@ def test_http_h1_mini_stress_test_open_loop(http_test_server_fixture):
   H1 open loop stress test. We expect higher pending and overflow counts 
   """
   counters = mini_stress_test(http_test_server_fixture, [
-      http_test_server_fixture.getTestServerRootUri(), "--rps", "999999", "--max-pending-requests",
+      http_test_server_fixture.getTestServerRootUri(), "--rps", "10000", "--max-pending-requests",
       "1", "--open-loop", "--max-active-requests", "1", "--connections", "1", "--duration", "100",
       "--termination-predicate", "benchmark.http_2xx:99", "--simple-warmup"
   ])

From 071b9411f8a1e46ecf18ce5b905eee94d94c8d4c Mon Sep 17 00:00:00 2001
From: Otto van der Schaaf <oschaaf@we-amp.com>
Date: Thu, 11 Jun 2020 23:46:50 +0200
Subject: [PATCH 07/22] Back out accidentally left in change

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
---
 test/integration/nighthawk_test_server.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/integration/nighthawk_test_server.py b/test/integration/nighthawk_test_server.py
index 194063371..28ecb359c 100644
--- a/test/integration/nighthawk_test_server.py
+++ b/test/integration/nighthawk_test_server.py
@@ -53,7 +53,7 @@ def __init__(self, server_binary_path, config_template_path, server_ip, ip_versi
   def serverThreadRunner(self):
     args = [
         self.server_binary_path, self.server_binary_config_path_arg, self.parameterized_config_path,
-        "-l", "error", "--concurrency", "1", "--base-id", self.instance_id, "--admin-address-path",
+        "-l", "error", "--base-id", self.instance_id, "--admin-address-path",
         self.admin_address_path
     ]
     logging.info("Test server popen() args: [%s]" % args)

From ede3a9d7a08bdceb66df64cca5aaf7c653052d9a Mon Sep 17 00:00:00 2001
From: Otto van der Schaaf <oschaaf@we-amp.com>
Date: Fri, 12 Jun 2020 00:01:30 +0200
Subject: [PATCH 08/22] Fix format issue

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
---
 test/integration/test_connection_management.py | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/test/integration/test_connection_management.py b/test/integration/test_connection_management.py
index edf727499..362519d45 100644
--- a/test/integration/test_connection_management.py
+++ b/test/integration/test_connection_management.py
@@ -109,7 +109,8 @@ def countLogLinesWithSubstring(logs, substring):
   _, logs = http_test_server_fixture.runNighthawkClient([
       "--rps 5", "-v", "trace", "--connections", "2", "--prefetch-connections",
       "--experimental-h1-connection-reuse-strategy", "mru", "--termination-predicate",
-      "benchmark.http_2xx:4", http_test_server_fixture.getTestServerRootUri()
+      "benchmark.http_2xx:4",
+      http_test_server_fixture.getTestServerRootUri()
   ])
 
   assertNotIn("[C1] message complete", logs)
@@ -121,7 +122,7 @@ def countLogLinesWithSubstring(logs, substring):
       "--rps", "5", "-v trace", "--connections",
       str(connections), "--prefetch-connections", "--experimental-h1-connection-reuse-strategy",
       "lru", "--termination-predicate",
-      "benchmark.http_2xx:%d" % (requests-1),
+      "benchmark.http_2xx:%d" % (requests - 1),
       http_test_server_fixture.getTestServerRootUri()
   ])
   for i in range(1, connections):

From 7280a32940490dfa001d742f063c0ddcd6ab81ca Mon Sep 17 00:00:00 2001
From: Otto van der Schaaf <oschaaf@we-amp.com>
Date: Fri, 12 Jun 2020 16:59:55 +0200
Subject: [PATCH 09/22] quick test

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
---
 test/integration/test_grpc_service.py | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/test/integration/test_grpc_service.py b/test/integration/test_grpc_service.py
index 418b424b2..dd820bae5 100644
--- a/test/integration/test_grpc_service.py
+++ b/test/integration/test_grpc_service.py
@@ -3,6 +3,8 @@
 
 from integration_test_fixtures import (http_test_server_fixture)
 from utility import *
+import subprocess
+import logging
 
 
 def test_grpc_service_happy_flow(http_test_server_fixture):
@@ -42,3 +44,18 @@ def test_grpc_service_stress(http_test_server_fixture):
   counters = http_test_server_fixture.getNighthawkCounterMapFromJson(parsed_json)
   assertGreaterEqual(counters["benchmark.http_2xx"], 5000)
   assertEqual(counters["requestsource.internal.upstream_rq_200"], 4)
+
+def run_service_with_arg(arg):
+  test_rundir = os.path.join(os.environ["TEST_SRCDIR"], os.environ["TEST_WORKSPACE"])
+  args = "%s %s" % (os.path.join(test_rundir, "nighthawk_service"), arg)
+  return subprocess.getstatusoutput(args)
+
+def test_grpc_service_help():
+  (exit_code, output) = run_service_with_arg("--help")
+  assert (exit_code == 0)
+  assert ("USAGE" in output)
+
+def test_grpc_service_bad_arguments():
+  (exit_code, output) = run_service_with_arg("--foo")
+  assert (exit_code == 1)
+  assert ("PARSE ERROR: Argument: --foo" in output)

From 3e191acff0759f0af0aa1703e703fb362b5a24e7 Mon Sep 17 00:00:00 2001
From: Otto van der Schaaf <oschaaf@we-amp.com>
Date: Fri, 12 Jun 2020 21:13:07 +0200
Subject: [PATCH 10/22] Tweaks to coverage

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
---
 test/integration/test_grpc_service.py |  3 +++
 test/run_nighthawk_bazel_coverage.sh  | 20 +++++++++++---------
 2 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/test/integration/test_grpc_service.py b/test/integration/test_grpc_service.py
index dd820bae5..5e8a0bf0a 100644
--- a/test/integration/test_grpc_service.py
+++ b/test/integration/test_grpc_service.py
@@ -45,16 +45,19 @@ def test_grpc_service_stress(http_test_server_fixture):
   assertGreaterEqual(counters["benchmark.http_2xx"], 5000)
   assertEqual(counters["requestsource.internal.upstream_rq_200"], 4)
 
+
 def run_service_with_arg(arg):
   test_rundir = os.path.join(os.environ["TEST_SRCDIR"], os.environ["TEST_WORKSPACE"])
   args = "%s %s" % (os.path.join(test_rundir, "nighthawk_service"), arg)
   return subprocess.getstatusoutput(args)
 
+
 def test_grpc_service_help():
   (exit_code, output) = run_service_with_arg("--help")
   assert (exit_code == 0)
   assert ("USAGE" in output)
 
+
 def test_grpc_service_bad_arguments():
   (exit_code, output) = run_service_with_arg("--foo")
   assert (exit_code == 1)
diff --git a/test/run_nighthawk_bazel_coverage.sh b/test/run_nighthawk_bazel_coverage.sh
index 227294bc9..fa7dd02d4 100755
--- a/test/run_nighthawk_bazel_coverage.sh
+++ b/test/run_nighthawk_bazel_coverage.sh
@@ -2,17 +2,23 @@
 
 # derived from test/run_envoy_bazel_coverage.sh over the Envoy repo.
 
-set -e
-set -x
+set -eo pipefail
+set +x
+set -u
 
-[[ -z "${SRCDIR}" ]] && SRCDIR="${PWD}"
-[[ -z "${VALIDATE_COVERAGE}" ]] && VALIDATE_COVERAGE=true
+SRCDIR="${SRCDIR:=${PWD}}"
+VALIDATE_COVERAGE="${VALIDATE_COVERAGE:=true}"
+ENVOY_COVERAGE_DIR="${ENVOY_COVERAGE_DIR:=}"
 
 echo "Starting run_nighthawk_bazel_coverage.sh..."
 echo "    PWD=$(pwd)"
 echo "    SRCDIR=${SRCDIR}"
 echo "    VALIDATE_COVERAGE=${VALIDATE_COVERAGE}"
 
+COVERAGE_DIR="${SRCDIR}"/generated/coverage
+rm -rf "${COVERAGE_DIR}"
+mkdir -p "${COVERAGE_DIR}"
+
 # This is the target that will be run to generate coverage data. It can be overridden by consumer
 # projects that want to run coverage on a different/combined target.
 # Command-line arguments take precedence over ${COVERAGE_TARGET}.
@@ -25,11 +31,7 @@ else
 fi
 
 BAZEL_BUILD_OPTIONS+=" --config=test-coverage --test_tag_filters=-nocoverage --test_env=ENVOY_IP_TEST_VERSIONS=v4only"
-bazel coverage ${BAZEL_BUILD_OPTIONS} --test_output=all ${COVERAGE_TARGETS}
-
-COVERAGE_DIR="${SRCDIR}"/generated/coverage
-mkdir -p "${COVERAGE_DIR}"
-
+bazel coverage ${BAZEL_BUILD_OPTIONS} --cache_test_results=no --test_output=all ${COVERAGE_TARGETS}
 COVERAGE_DATA="${COVERAGE_DIR}/coverage.dat"
 
 cp bazel-out/_coverage/_coverage_report.dat "${COVERAGE_DATA}"

From f6d7d99e4c1987a9cd903138d1a433b07baebf2e Mon Sep 17 00:00:00 2001
From: Otto van der Schaaf <oschaaf@we-amp.com>
Date: Sat, 13 Jun 2020 00:11:00 +0200
Subject: [PATCH 11/22] docker flow fixes

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
---
 ci/envoy_build_sha.sh                | 4 ++--
 test/run_nighthawk_bazel_coverage.sh | 4 +++-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/ci/envoy_build_sha.sh b/ci/envoy_build_sha.sh
index 709617afe..d2a984438 100644
--- a/ci/envoy_build_sha.sh
+++ b/ci/envoy_build_sha.sh
@@ -1,2 +1,2 @@
-ENVOY_BUILD_SHA=$(grep envoyproxy/envoy-build .circleci/config.yml | sed -e 's#.*envoyproxy/envoy-build:\(.*\)#\1#' | uniq)
-[[ $(wc -l <<< "${ENVOY_BUILD_SHA}" | awk '{$1=$1};1') == 1 ]] || (echo ".circleci/config.yml hashes are inconsistent!" && exit 1)
+ENVOY_BUILD_SHA=$(grep envoyproxy/envoy-build-ubuntu $(dirname $0)/../.bazelrc | sed -e 's#.*envoyproxy/envoy-build-ubuntu:\(.*\)#\1#' | uniq)
+[[ $(wc -l <<< "${ENVOY_BUILD_SHA}" | awk '{$1=$1};1') == 1 ]] || (echo ".bazelrc envoyproxy/envoy-build-ubuntu hashes are inconsistent!" && exit 1)
\ No newline at end of file
diff --git a/test/run_nighthawk_bazel_coverage.sh b/test/run_nighthawk_bazel_coverage.sh
index fa7dd02d4..55770cf0a 100755
--- a/test/run_nighthawk_bazel_coverage.sh
+++ b/test/run_nighthawk_bazel_coverage.sh
@@ -30,13 +30,15 @@ else
   COVERAGE_TARGETS=//test/...
 fi
 
+COVERAGE_TARGETS=//test:python_test
+
 BAZEL_BUILD_OPTIONS+=" --config=test-coverage --test_tag_filters=-nocoverage --test_env=ENVOY_IP_TEST_VERSIONS=v4only"
 bazel coverage ${BAZEL_BUILD_OPTIONS} --cache_test_results=no --test_output=all ${COVERAGE_TARGETS}
 COVERAGE_DATA="${COVERAGE_DIR}/coverage.dat"
 
 cp bazel-out/_coverage/_coverage_report.dat "${COVERAGE_DATA}"
 
-COVERAGE_VALUE=$(genhtml --prefix ${PWD} --output "${COVERAGE_DIR}" "${COVERAGE_DATA}" | tee /dev/stderr | grep lines... | cut -d ' ' -f 4)
+COVERAGE_VALUE=$(genhtml --prefix ${PWD} --output "${COVERAGE_DIR}" "${COVERAGE_DATA}" | grep lines... | cut -d ' ' -f 4)
 COVERAGE_VALUE=${COVERAGE_VALUE%?}
 
 [[ -z "${ENVOY_COVERAGE_DIR}" ]] || rsync -av "${COVERAGE_DIR}"/ "${ENVOY_COVERAGE_DIR}"

From 7c5856f5f15e186a3df519836e4c1e9d9310842f Mon Sep 17 00:00:00 2001
From: Otto van der Schaaf <oschaaf@we-amp.com>
Date: Sat, 13 Jun 2020 13:02:38 +0200
Subject: [PATCH 12/22] Coverage: separate build and coverage execution steps

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
---
 test/BUILD                            | 7 -------
 test/integration/test_grpc_service.py | 1 -
 test/run_nighthawk_bazel_coverage.sh  | 4 +---
 3 files changed, 1 insertion(+), 11 deletions(-)

diff --git a/test/BUILD b/test/BUILD
index ecd6183c4..32c466622 100644
--- a/test/BUILD
+++ b/test/BUILD
@@ -253,19 +253,12 @@ envoy_cc_test(
     ],
 )
 
-# We wrap the python integration tests here so we run the instrumented
-# code in coverage tests. To make that work, we need to declare the top-level
-# libraries as dependencies.
 envoy_cc_test(
     name = "python_test",
     srcs = ["python_test.cc"],
     data = ["//test/integration:integration_test"],
     repository = "@envoy",
     deps = [
-        "//source/client:nighthawk_client_lib",
-        "//source/client:nighthawk_service_lib",
-        "//source/common:nighthawk_common_lib",
-        "//source/server:http_test_server_filter_lib",
         "//test/test_common:environment_lib",
         "@envoy//test/test_common:network_utility_lib",
     ],
diff --git a/test/integration/test_grpc_service.py b/test/integration/test_grpc_service.py
index 5e8a0bf0a..9bf8530bb 100644
--- a/test/integration/test_grpc_service.py
+++ b/test/integration/test_grpc_service.py
@@ -4,7 +4,6 @@
 from integration_test_fixtures import (http_test_server_fixture)
 from utility import *
 import subprocess
-import logging
 
 
 def test_grpc_service_happy_flow(http_test_server_fixture):
diff --git a/test/run_nighthawk_bazel_coverage.sh b/test/run_nighthawk_bazel_coverage.sh
index 55770cf0a..ca23c4e75 100755
--- a/test/run_nighthawk_bazel_coverage.sh
+++ b/test/run_nighthawk_bazel_coverage.sh
@@ -30,9 +30,7 @@ else
   COVERAGE_TARGETS=//test/...
 fi
 
-COVERAGE_TARGETS=//test:python_test
-
-BAZEL_BUILD_OPTIONS+=" --config=test-coverage --test_tag_filters=-nocoverage --test_env=ENVOY_IP_TEST_VERSIONS=v4only"
+bazel build --config=coverage ${BAZEL_BUILD_OPTIONS} ${COVERAGE_TARGETS}
 bazel coverage ${BAZEL_BUILD_OPTIONS} --cache_test_results=no --test_output=all ${COVERAGE_TARGETS}
 COVERAGE_DATA="${COVERAGE_DIR}/coverage.dat"
 

From 8700f80eb84ae03f2938b36cf046e7ac274aae36 Mon Sep 17 00:00:00 2001
From: Otto van der Schaaf <oschaaf@we-amp.com>
Date: Sat, 13 Jun 2020 13:03:46 +0200
Subject: [PATCH 13/22] Coverage: NUM_CPUS 3->5

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
---
 ci/do_ci.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ci/do_ci.sh b/ci/do_ci.sh
index e6a43419e..2ebe2b864 100755
--- a/ci/do_ci.sh
+++ b/ci/do_ci.sh
@@ -126,7 +126,7 @@ if [ -n "$CIRCLECI" ]; then
         NUM_CPUS=5
     fi
     if [[ "$1" == "coverage" ]]; then
-        NUM_CPUS=3
+        NUM_CPUS=5
     fi
 fi
 

From 512b52ea5437cdf6fc8e71099ea3b13be72b6565 Mon Sep 17 00:00:00 2001
From: Otto van der Schaaf <oschaaf@we-amp.com>
Date: Sat, 13 Jun 2020 23:37:19 +0200
Subject: [PATCH 14/22] Back out increased parallelism

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
---
 ci/do_ci.sh                          | 2 +-
 test/run_nighthawk_bazel_coverage.sh | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/ci/do_ci.sh b/ci/do_ci.sh
index 2ebe2b864..e6a43419e 100755
--- a/ci/do_ci.sh
+++ b/ci/do_ci.sh
@@ -126,7 +126,7 @@ if [ -n "$CIRCLECI" ]; then
         NUM_CPUS=5
     fi
     if [[ "$1" == "coverage" ]]; then
-        NUM_CPUS=5
+        NUM_CPUS=3
     fi
 fi
 
diff --git a/test/run_nighthawk_bazel_coverage.sh b/test/run_nighthawk_bazel_coverage.sh
index ca23c4e75..bd9f75e49 100755
--- a/test/run_nighthawk_bazel_coverage.sh
+++ b/test/run_nighthawk_bazel_coverage.sh
@@ -30,7 +30,6 @@ else
   COVERAGE_TARGETS=//test/...
 fi
 
-bazel build --config=coverage ${BAZEL_BUILD_OPTIONS} ${COVERAGE_TARGETS}
 bazel coverage ${BAZEL_BUILD_OPTIONS} --cache_test_results=no --test_output=all ${COVERAGE_TARGETS}
 COVERAGE_DATA="${COVERAGE_DIR}/coverage.dat"
 

From 6b96503678da76183d01ca53cd1f8dbc85fdc6b0 Mon Sep 17 00:00:00 2001
From: Otto van der Schaaf <oschaaf@we-amp.com>
Date: Sun, 14 Jun 2020 10:03:09 +0200
Subject: [PATCH 15/22] coverage: restore ipv4 only test option

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
---
 test/run_nighthawk_bazel_coverage.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/test/run_nighthawk_bazel_coverage.sh b/test/run_nighthawk_bazel_coverage.sh
index bd9f75e49..bb2a33d33 100755
--- a/test/run_nighthawk_bazel_coverage.sh
+++ b/test/run_nighthawk_bazel_coverage.sh
@@ -30,6 +30,7 @@ else
   COVERAGE_TARGETS=//test/...
 fi
 
+BAZEL_BUILD_OPTIONS+=" --config=test-coverage --test_tag_filters=-nocoverage --test_env=ENVOY_IP_TEST_VERSIONS=v4only"
 bazel coverage ${BAZEL_BUILD_OPTIONS} --cache_test_results=no --test_output=all ${COVERAGE_TARGETS}
 COVERAGE_DATA="${COVERAGE_DIR}/coverage.dat"
 

From 3a23c5230fad1f54e33fad389f840a0c6f1a8194 Mon Sep 17 00:00:00 2001
From: Otto van der Schaaf <oschaaf@we-amp.com>
Date: Sun, 14 Jun 2020 11:48:14 +0200
Subject: [PATCH 16/22] Add some essential cli / coverage tests

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
---
 test/integration/BUILD                      |  8 ++++
 test/integration/test_grpc_service.py       | 21 +++++-----
 test/integration/test_integration_basics.py | 16 ++++++++
 test/integration/test_output_transform.py   | 43 +++++++++++++++++++++
 test/integration/utility.py                 |  7 ++++
 5 files changed, 86 insertions(+), 9 deletions(-)
 create mode 100644 test/integration/test_output_transform.py

diff --git a/test/integration/BUILD b/test/integration/BUILD
index 43cd271bb..720e83f00 100644
--- a/test/integration/BUILD
+++ b/test/integration/BUILD
@@ -23,6 +23,7 @@ py_library(
         "configurations/nighthawk_https_origin.yaml",
         "configurations/sni_origin.yaml",
         "//:nighthawk_client",
+        "//:nighthawk_output_transform",
         "//:nighthawk_service",
         "//:nighthawk_test_server",
         "@envoy//test/config/integration/certs",
@@ -83,6 +84,12 @@ py_library(
     deps = [":integration_test_base"],
 )
 
+py_library(
+    name = "test_output_transform_lib",
+    srcs = ["test_output_transform.py"],
+    deps = [":integration_test_base"],
+)
+
 py_binary(
     name = "integration_test",
     srcs = ["integration_test.py"],
@@ -99,6 +106,7 @@ py_binary(
         ":test_grpc_service_lib",
         ":test_integration_basics_lib",
         ":test_integration_zipkin_lib",
+        ":test_output_transform_lib",
         ":test_remote_execution_lib",
     ],
 )
diff --git a/test/integration/test_grpc_service.py b/test/integration/test_grpc_service.py
index 9bf8530bb..c1567ed67 100644
--- a/test/integration/test_grpc_service.py
+++ b/test/integration/test_grpc_service.py
@@ -1,9 +1,8 @@
 #!/usr/bin/env python3
 import pytest
 
-from integration_test_fixtures import (http_test_server_fixture)
-from utility import *
-import subprocess
+from test.integration.integration_test_fixtures import http_test_server_fixture
+from test.integration.utility import *
 
 
 def test_grpc_service_happy_flow(http_test_server_fixture):
@@ -45,19 +44,23 @@ def test_grpc_service_stress(http_test_server_fixture):
   assertEqual(counters["requestsource.internal.upstream_rq_200"], 4)
 
 
-def run_service_with_arg(arg):
-  test_rundir = os.path.join(os.environ["TEST_SRCDIR"], os.environ["TEST_WORKSPACE"])
-  args = "%s %s" % (os.path.join(test_rundir, "nighthawk_service"), arg)
-  return subprocess.getstatusoutput(args)
+def run_service_with_args(args):
+  return run_binary_with_args("nighthawk_service", args)
 
 
 def test_grpc_service_help():
-  (exit_code, output) = run_service_with_arg("--help")
+  (exit_code, output) = run_service_with_args("--help")
   assert (exit_code == 0)
   assert ("USAGE" in output)
 
 
 def test_grpc_service_bad_arguments():
-  (exit_code, output) = run_service_with_arg("--foo")
+  (exit_code, output) = run_service_with_args("--foo")
   assert (exit_code == 1)
   assert ("PARSE ERROR: Argument: --foo" in output)
+
+
+def test_grpc_service_nonexisting_listener_address():
+  (exit_code, output) = run_service_with_args("--listen 1.1.1.1:1")
+  assert (exit_code == 1)
+  assert ("Failure: Could not start the grpc service" in output)
diff --git a/test/integration/test_integration_basics.py b/test/integration/test_integration_basics.py
index bb0315588..037a4dcda 100644
--- a/test/integration/test_integration_basics.py
+++ b/test/integration/test_integration_basics.py
@@ -652,3 +652,19 @@ def test_http_request_release_timing(http_test_server_fixture, qps_parameterizat
         int(global_histograms["benchmark_http_client.queue_to_connect"]["count"]), total_requests)
 
     assertCounterEqual(counters, "benchmark.http_2xx", (total_requests))
+
+
+def run_client_with_args(args):
+  return run_binary_with_args("nighthawk_client", args)
+
+
+def test_client_help():
+  (exit_code, output) = run_client_with_args("--help")
+  assert (exit_code == 0)
+  assert ("USAGE" in output)
+
+
+def test_client_bad_arg():
+  (exit_code, output) = run_client_with_args("127.0.0.1 --foo")
+  assert (exit_code == 1)
+  assert ("PARSE ERROR: Argument: --foo" in output)
diff --git a/test/integration/test_output_transform.py b/test/integration/test_output_transform.py
new file mode 100644
index 000000000..8ada00f3e
--- /dev/null
+++ b/test/integration/test_output_transform.py
@@ -0,0 +1,43 @@
+#!/usr/bin/env python3
+import pytest
+
+from test.integration.utility import *
+import os
+import subprocess
+
+
+def run_output_transform_with_args(args):
+  return run_binary_with_args("nighthawk_output_transform", args)
+
+
+def test_output_transform_help():
+  (exit_code, output) = run_output_transform_with_args("--help")
+  assert (exit_code == 0)
+  assert ("USAGE" in output)
+
+
+def test_output_transform_bad_arguments():
+  (exit_code, output) = run_output_transform_with_args("--foo")
+  assert (exit_code == 1)
+  assert ("PARSE ERROR: Argument: --foo" in output)
+
+
+def test_output_transform_101():
+  """
+  Runs an arbitrary load test, which outputs to json.
+  This json output is then transformed to human readable output.
+  """
+
+  test_rundir = os.path.join(os.environ["TEST_SRCDIR"], os.environ["TEST_WORKSPACE"])
+  process = subprocess.run([
+      os.path.join(test_rundir, "nighthawk_client"), "--duration", "1", "--rps", "1", "127.0.0.1",
+      "--output-format", "json"
+  ],
+                           stdout=subprocess.PIPE)
+  output = process.stdout
+  process = subprocess.run(
+      [os.path.join(test_rundir, "nighthawk_output_transform"), "--output-format", "human"],
+      stdout=subprocess.PIPE,
+      input=output)
+  assert (process.returncode == 0)
+  assert ("Nighthawk - A layer 7 protocol benchmarking tool" in process.stdout.decode("utf-8"))
diff --git a/test/integration/utility.py b/test/integration/utility.py
index 28e9a2d37..766cc5c64 100644
--- a/test/integration/utility.py
+++ b/test/integration/utility.py
@@ -1,4 +1,5 @@
 import os
+import subprocess
 
 
 def assertEqual(a, b):
@@ -57,3 +58,9 @@ def assertCounterBetweenInclusive(counters, name, min_value, max_value):
 
 def isSanitizerRun():
   return True if os.environ.get("NH_INTEGRATION_TEST_SANITIZER_RUN", 0) == "1" else False
+
+
+def run_binary_with_args(binary, args):
+  test_rundir = os.path.join(os.environ["TEST_SRCDIR"], os.environ["TEST_WORKSPACE"])
+  args = "%s %s" % (os.path.join(test_rundir, binary), args)
+  return subprocess.getstatusoutput(args)

From f389a3f4e7a0d04327d5c041cd530ef34faffdb7 Mon Sep 17 00:00:00 2001
From: Otto van der Schaaf <oschaaf@we-amp.com>
Date: Sun, 14 Jun 2020 14:53:10 +0200
Subject: [PATCH 17/22] Add deps to py_test

Experiment: see if adding back the tests brings back counting
coverage for some of the lines in the .h files that went missing.

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
---
 test/BUILD | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/test/BUILD b/test/BUILD
index 32c466622..0c21ccb3a 100644
--- a/test/BUILD
+++ b/test/BUILD
@@ -259,6 +259,12 @@ envoy_cc_test(
     data = ["//test/integration:integration_test"],
     repository = "@envoy",
     deps = [
+        "//source/client:nighthawk_client_lib",
+        "//source/client:nighthawk_service_lib",
+        "//source/client:output_transform_main_lib",
+        "//source/common:nighthawk_common_lib",
+        "//source/common:request_source_impl_lib",
+        "//source/server:http_test_server_filter_lib",
         "//test/test_common:environment_lib",
         "@envoy//test/test_common:network_utility_lib",
     ],

From 2375d6d76d37fb73708d379a0b18d3ee2451c213 Mon Sep 17 00:00:00 2001
From: Otto van der Schaaf <oschaaf@we-amp.com>
Date: Sun, 14 Jun 2020 20:17:38 +0200
Subject: [PATCH 18/22] Increase test_timeout for asan runs

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
---
 .bazelrc | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/.bazelrc b/.bazelrc
index dbdd016de..101570225 100644
--- a/.bazelrc
+++ b/.bazelrc
@@ -1,6 +1,8 @@
 # The following .bazelrc content is forked from the main Envoy repository. This is necessary since	
 # this needs to be available before we can access the Envoy repository contents via Bazel.
 
+build:asan --test_timeout=900
+
 # Envoy specific Bazel build/test options.
 
 # Bazel doesn't need more than 200MB of memory for local build based on memory profiling:

From fe61b40d266d9f063e7c1a95b0795feb4aec30fa Mon Sep 17 00:00:00 2001
From: Otto van der Schaaf <oschaaf@we-amp.com>
Date: Sun, 14 Jun 2020 21:27:32 +0200
Subject: [PATCH 19/22] Tweak sanitizer test timeouts

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
---
 .bazelrc                  | 3 ++-
 test/rate_limiter_test.cc | 9 ++++++++-
 test/statistic_test.cc    | 5 +++++
 3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/.bazelrc b/.bazelrc
index 101570225..54769468c 100644
--- a/.bazelrc
+++ b/.bazelrc
@@ -1,7 +1,8 @@
 # The following .bazelrc content is forked from the main Envoy repository. This is necessary since	
 # this needs to be available before we can access the Envoy repository contents via Bazel.
 
-build:asan --test_timeout=900
+build:clang-asan --test_timeout=900
+build:clang-tsan --test_timeout=900
 
 # Envoy specific Bazel build/test options.
 
diff --git a/test/rate_limiter_test.cc b/test/rate_limiter_test.cc
index 8dee7ffb8..76d9b3e5d 100644
--- a/test/rate_limiter_test.cc
+++ b/test/rate_limiter_test.cc
@@ -166,8 +166,11 @@ TEST_F(RateLimiterTest, DistributionSamplingRateLimiterImplTest) {
   EXPECT_CALL(unsafe_mock_rate_limiter, timeSource)
       .Times(AtLeast(1))
       .WillRepeatedly(ReturnRef(time_system));
+  auto sampler = std::make_unique<UniformRandomDistributionSamplerImpl>(1);
+  EXPECT_EQ(sampler->min(), 0);
+  EXPECT_EQ(sampler->max(), 1);
   RateLimiterPtr rate_limiter = std::make_unique<DistributionSamplingRateLimiterImpl>(
-      std::make_unique<UniformRandomDistributionSamplerImpl>(1), std::move(mock_rate_limiter));
+      std::move(sampler), std::move(mock_rate_limiter));
 
   EXPECT_CALL(unsafe_mock_rate_limiter, tryAcquireOne).Times(tries).WillRepeatedly(Return(true));
   EXPECT_CALL(unsafe_mock_rate_limiter, releaseOne).Times(tries);
@@ -383,6 +386,10 @@ class GraduallyOpeningRateLimiterFilterTest : public Test {
     }
     // Verify we acquired everything.
     EXPECT_FALSE(rate_limiter->tryAcquireOne());
+    // Verify releaseOne works.
+    rate_limiter->releaseOne();
+    EXPECT_TRUE(rate_limiter->tryAcquireOne());
+    EXPECT_FALSE(rate_limiter->tryAcquireOne());
     return acquisition_timings;
   }
 };
diff --git a/test/statistic_test.cc b/test/statistic_test.cc
index c36faeb47..2f0a81a8a 100644
--- a/test/statistic_test.cc
+++ b/test/statistic_test.cc
@@ -321,6 +321,11 @@ TEST(StatisticTest, NullStatistic) {
   EXPECT_EQ(0, stat.count());
   stat.addValue(1);
   EXPECT_EQ(0, stat.count());
+  EXPECT_EQ(0, stat.pvariance());
+  EXPECT_EQ(0, stat.pstdev());
+  EXPECT_NE(nullptr, stat.combine(stat));
+  EXPECT_EQ(0, stat.significantDigits());
+  EXPECT_NE(nullptr, stat.createNewInstanceOfSameType());
 }
 
 } // namespace Nighthawk

From 2b6c0e3fdaf5e13f032825d8fd46d9257c28d279 Mon Sep 17 00:00:00 2001
From: Otto van der Schaaf <oschaaf@we-amp.com>
Date: Sun, 14 Jun 2020 23:20:13 +0200
Subject: [PATCH 20/22] Fix test line, bump coverage threshold

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
---
 test/BUILD                           | 1 -
 test/run_nighthawk_bazel_coverage.sh | 2 +-
 test/statistic_test.cc               | 2 +-
 3 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/test/BUILD b/test/BUILD
index 0c21ccb3a..dd5db3a5b 100644
--- a/test/BUILD
+++ b/test/BUILD
@@ -263,7 +263,6 @@ envoy_cc_test(
         "//source/client:nighthawk_service_lib",
         "//source/client:output_transform_main_lib",
         "//source/common:nighthawk_common_lib",
-        "//source/common:request_source_impl_lib",
         "//source/server:http_test_server_filter_lib",
         "//test/test_common:environment_lib",
         "@envoy//test/test_common:network_utility_lib",
diff --git a/test/run_nighthawk_bazel_coverage.sh b/test/run_nighthawk_bazel_coverage.sh
index bb2a33d33..8b48f4eec 100755
--- a/test/run_nighthawk_bazel_coverage.sh
+++ b/test/run_nighthawk_bazel_coverage.sh
@@ -43,7 +43,7 @@ COVERAGE_VALUE=${COVERAGE_VALUE%?}
 
 if [ "$VALIDATE_COVERAGE" == "true" ]
 then
-  COVERAGE_THRESHOLD=98.2
+  COVERAGE_THRESHOLD=98.6
   COVERAGE_FAILED=$(echo "${COVERAGE_VALUE}<${COVERAGE_THRESHOLD}" | bc)
   if test ${COVERAGE_FAILED} -eq 1; then
       echo Code coverage ${COVERAGE_VALUE} is lower than limit of ${COVERAGE_THRESHOLD}
diff --git a/test/statistic_test.cc b/test/statistic_test.cc
index 2f0a81a8a..c73e1d6b8 100644
--- a/test/statistic_test.cc
+++ b/test/statistic_test.cc
@@ -320,7 +320,7 @@ TEST(StatisticTest, NullStatistic) {
   NullStatistic stat;
   EXPECT_EQ(0, stat.count());
   stat.addValue(1);
-  EXPECT_EQ(0, stat.count());
+  EXPECT_EQ(0, stat.mean());
   EXPECT_EQ(0, stat.pvariance());
   EXPECT_EQ(0, stat.pstdev());
   EXPECT_NE(nullptr, stat.combine(stat));

From f0596d4278a0c20eb81a982ae2516cd1f4bb71f1 Mon Sep 17 00:00:00 2001
From: Otto van der Schaaf <oschaaf@we-amp.com>
Date: Thu, 18 Jun 2020 20:53:19 +0200
Subject: [PATCH 21/22] Review feedback

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
---
 test/BUILD                                  |  3 +++
 test/integration/test_grpc_service.py       | 23 +++++++++++----------
 test/integration/test_integration_basics.py | 14 ++++++-------
 test/integration/test_output_transform.py   | 20 +++++++++---------
 test/integration/utility.py                 | 10 +++++++++
 5 files changed, 42 insertions(+), 28 deletions(-)

diff --git a/test/BUILD b/test/BUILD
index 0d3c94e0c..868394027 100644
--- a/test/BUILD
+++ b/test/BUILD
@@ -255,6 +255,9 @@ envoy_cc_test(
     ],
 )
 
+# We wrap the python integration tests here so we run the instrumented
+# code in coverage tests. To make that work, we need to declare the top-level
+# libraries as dependencies.
 envoy_cc_test(
     name = "python_test",
     srcs = ["python_test.cc"],
diff --git a/test/integration/test_grpc_service.py b/test/integration/test_grpc_service.py
index c1567ed67..252760709 100644
--- a/test/integration/test_grpc_service.py
+++ b/test/integration/test_grpc_service.py
@@ -2,7 +2,8 @@
 import pytest
 
 from test.integration.integration_test_fixtures import http_test_server_fixture
-from test.integration.utility import *
+from test.integration.utility import (isSanitizerRun, assertEqual, assertIn, assertGreaterEqual,
+                                      run_binary_with_args)
 
 
 def test_grpc_service_happy_flow(http_test_server_fixture):
@@ -44,23 +45,23 @@ def test_grpc_service_stress(http_test_server_fixture):
   assertEqual(counters["requestsource.internal.upstream_rq_200"], 4)
 
 
-def run_service_with_args(args):
+def _run_service_with_args(args):
   return run_binary_with_args("nighthawk_service", args)
 
 
 def test_grpc_service_help():
-  (exit_code, output) = run_service_with_args("--help")
-  assert (exit_code == 0)
-  assert ("USAGE" in output)
+  (exit_code, output) = _run_service_with_args("--help")
+  assertEqual(exit_code, 0)
+  assertIn("USAGE", output)
 
 
 def test_grpc_service_bad_arguments():
-  (exit_code, output) = run_service_with_args("--foo")
-  assert (exit_code == 1)
-  assert ("PARSE ERROR: Argument: --foo" in output)
+  (exit_code, output) = _run_service_with_args("--foo")
+  assertEqual(exit_code, 1)
+  assertIn("PARSE ERROR: Argument: --foo", output)
 
 
 def test_grpc_service_nonexisting_listener_address():
-  (exit_code, output) = run_service_with_args("--listen 1.1.1.1:1")
-  assert (exit_code == 1)
-  assert ("Failure: Could not start the grpc service" in output)
+  (exit_code, output) = _run_service_with_args("--listen 1.1.1.1:1")
+  assertEqual(exit_code, 1)
+  assertIn("Failure: Could not start the grpc service", output)
diff --git a/test/integration/test_integration_basics.py b/test/integration/test_integration_basics.py
index 037a4dcda..df02ef193 100644
--- a/test/integration/test_integration_basics.py
+++ b/test/integration/test_integration_basics.py
@@ -654,17 +654,17 @@ def test_http_request_release_timing(http_test_server_fixture, qps_parameterizat
     assertCounterEqual(counters, "benchmark.http_2xx", (total_requests))
 
 
-def run_client_with_args(args):
+def _run_client_with_args(args):
   return run_binary_with_args("nighthawk_client", args)
 
 
 def test_client_help():
-  (exit_code, output) = run_client_with_args("--help")
-  assert (exit_code == 0)
-  assert ("USAGE" in output)
+  (exit_code, output) = _run_client_with_args("--help")
+  assertEqual(exit_code, 0)
+  assertIn("USAGE", output)
 
 
 def test_client_bad_arg():
-  (exit_code, output) = run_client_with_args("127.0.0.1 --foo")
-  assert (exit_code == 1)
-  assert ("PARSE ERROR: Argument: --foo" in output)
+  (exit_code, output) = _run_client_with_args("127.0.0.1 --foo")
+  assertEqual(exit_code, 1)
+  assertIn("PARSE ERROR: Argument: --foo", output)
diff --git a/test/integration/test_output_transform.py b/test/integration/test_output_transform.py
index 8ada00f3e..e479673fe 100644
--- a/test/integration/test_output_transform.py
+++ b/test/integration/test_output_transform.py
@@ -1,25 +1,25 @@
 #!/usr/bin/env python3
 import pytest
 
-from test.integration.utility import *
+from test.integration.utility import (run_binary_with_args, assertEqual, assertIn)
 import os
 import subprocess
 
 
-def run_output_transform_with_args(args):
+def _run_output_transform_with_args(args):
   return run_binary_with_args("nighthawk_output_transform", args)
 
 
 def test_output_transform_help():
-  (exit_code, output) = run_output_transform_with_args("--help")
-  assert (exit_code == 0)
-  assert ("USAGE" in output)
+  (exit_code, output) = _run_output_transform_with_args("--help")
+  assertEqual(exit_code, 0)
+  assertIn("USAGE", output)
 
 
 def test_output_transform_bad_arguments():
-  (exit_code, output) = run_output_transform_with_args("--foo")
-  assert (exit_code == 1)
-  assert ("PARSE ERROR: Argument: --foo" in output)
+  (exit_code, output) = _run_output_transform_with_args("--foo")
+  assertEqual(exit_code, 1)
+  assertIn("PARSE ERROR: Argument: --foo", output)
 
 
 def test_output_transform_101():
@@ -39,5 +39,5 @@ def test_output_transform_101():
       [os.path.join(test_rundir, "nighthawk_output_transform"), "--output-format", "human"],
       stdout=subprocess.PIPE,
       input=output)
-  assert (process.returncode == 0)
-  assert ("Nighthawk - A layer 7 protocol benchmarking tool" in process.stdout.decode("utf-8"))
+  assertEqual(process.returncode, 0)
+  assertIn("Nighthawk - A layer 7 protocol benchmarking tool", process.stdout.decode("utf-8"))
diff --git a/test/integration/utility.py b/test/integration/utility.py
index 766cc5c64..907ffab6f 100644
--- a/test/integration/utility.py
+++ b/test/integration/utility.py
@@ -61,6 +61,16 @@ def isSanitizerRun():
 
 
 def run_binary_with_args(binary, args):
+  """Executes a Nighthawk binary with the provided arguments.
+
+  Args:
+    binary: A string, the name of the to-be-called binary, e.g. "nighthawk_client".
+    args: A string, the command line arguments to the binary, e.g. "--foo --bar".
+
+  Returns:
+    A tuple in the form (exit_code, output), where exit_code is the code the Nighthawk
+    service terminated with and the output is its standard output.
+  """
   test_rundir = os.path.join(os.environ["TEST_SRCDIR"], os.environ["TEST_WORKSPACE"])
   args = "%s %s" % (os.path.join(test_rundir, binary), args)
   return subprocess.getstatusoutput(args)

From 66f658553658b31ad93fac5f1dfc4b7871045265 Mon Sep 17 00:00:00 2001
From: Otto van der Schaaf <oschaaf@we-amp.com>
Date: Fri, 19 Jun 2020 00:19:30 +0200
Subject: [PATCH 22/22] Review feedback: change imports

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
---
 test/integration/test_grpc_service.py     | 29 +++++++++++------------
 test/integration/test_output_transform.py | 17 ++++++-------
 2 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/test/integration/test_grpc_service.py b/test/integration/test_grpc_service.py
index 252760709..f0624caad 100644
--- a/test/integration/test_grpc_service.py
+++ b/test/integration/test_grpc_service.py
@@ -2,8 +2,7 @@
 import pytest
 
 from test.integration.integration_test_fixtures import http_test_server_fixture
-from test.integration.utility import (isSanitizerRun, assertEqual, assertIn, assertGreaterEqual,
-                                      run_binary_with_args)
+from test.integration import utility
 
 
 def test_grpc_service_happy_flow(http_test_server_fixture):
@@ -15,8 +14,8 @@ def test_grpc_service_happy_flow(http_test_server_fixture):
       http_test_server_fixture.getTestServerRootUri()
   ])
   counters = http_test_server_fixture.getNighthawkCounterMapFromJson(parsed_json)
-  assertGreaterEqual(counters["benchmark.http_2xx"], 5)
-  assertEqual(counters["requestsource.internal.upstream_rq_200"], 1)
+  utility.assertGreaterEqual(counters["benchmark.http_2xx"], 5)
+  utility.assertEqual(counters["requestsource.internal.upstream_rq_200"], 1)
 
 
 def test_grpc_service_down(http_test_server_fixture):
@@ -27,10 +26,10 @@ def test_grpc_service_down(http_test_server_fixture):
   ],
                                                                expect_failure=True)
   counters = http_test_server_fixture.getNighthawkCounterMapFromJson(parsed_json)
-  assertEqual(counters["requestsource.upstream_rq_pending_failure_eject"], 1)
+  utility.assertEqual(counters["requestsource.upstream_rq_pending_failure_eject"], 1)
 
 
-@pytest.mark.skipif(isSanitizerRun(), reason="Slow in sanitizer runs")
+@pytest.mark.skipif(utility.isSanitizerRun(), reason="Slow in sanitizer runs")
 def test_grpc_service_stress(http_test_server_fixture):
   http_test_server_fixture.startNighthawkGrpcService("dummy-request-source")
   parsed_json, _ = http_test_server_fixture.runNighthawkClient([
@@ -41,27 +40,27 @@ def test_grpc_service_stress(http_test_server_fixture):
       http_test_server_fixture.getTestServerRootUri()
   ])
   counters = http_test_server_fixture.getNighthawkCounterMapFromJson(parsed_json)
-  assertGreaterEqual(counters["benchmark.http_2xx"], 5000)
-  assertEqual(counters["requestsource.internal.upstream_rq_200"], 4)
+  utility.assertGreaterEqual(counters["benchmark.http_2xx"], 5000)
+  utility.assertEqual(counters["requestsource.internal.upstream_rq_200"], 4)
 
 
 def _run_service_with_args(args):
-  return run_binary_with_args("nighthawk_service", args)
+  return utility.run_binary_with_args("nighthawk_service", args)
 
 
 def test_grpc_service_help():
   (exit_code, output) = _run_service_with_args("--help")
-  assertEqual(exit_code, 0)
-  assertIn("USAGE", output)
+  utility.assertEqual(exit_code, 0)
+  utility.assertIn("USAGE", output)
 
 
 def test_grpc_service_bad_arguments():
   (exit_code, output) = _run_service_with_args("--foo")
-  assertEqual(exit_code, 1)
-  assertIn("PARSE ERROR: Argument: --foo", output)
+  utility.assertEqual(exit_code, 1)
+  utility.assertIn("PARSE ERROR: Argument: --foo", output)
 
 
 def test_grpc_service_nonexisting_listener_address():
   (exit_code, output) = _run_service_with_args("--listen 1.1.1.1:1")
-  assertEqual(exit_code, 1)
-  assertIn("Failure: Could not start the grpc service", output)
+  utility.assertEqual(exit_code, 1)
+  utility.assertIn("Failure: Could not start the grpc service", output)
diff --git a/test/integration/test_output_transform.py b/test/integration/test_output_transform.py
index e479673fe..3889fcdb0 100644
--- a/test/integration/test_output_transform.py
+++ b/test/integration/test_output_transform.py
@@ -1,25 +1,25 @@
 #!/usr/bin/env python3
 import pytest
 
-from test.integration.utility import (run_binary_with_args, assertEqual, assertIn)
+from test.integration import utility
 import os
 import subprocess
 
 
 def _run_output_transform_with_args(args):
-  return run_binary_with_args("nighthawk_output_transform", args)
+  return utility.run_binary_with_args("nighthawk_output_transform", args)
 
 
 def test_output_transform_help():
   (exit_code, output) = _run_output_transform_with_args("--help")
-  assertEqual(exit_code, 0)
-  assertIn("USAGE", output)
+  utility.assertEqual(exit_code, 0)
+  utility.assertIn("USAGE", output)
 
 
 def test_output_transform_bad_arguments():
   (exit_code, output) = _run_output_transform_with_args("--foo")
-  assertEqual(exit_code, 1)
-  assertIn("PARSE ERROR: Argument: --foo", output)
+  utility.assertEqual(exit_code, 1)
+  utility.assertIn("PARSE ERROR: Argument: --foo", output)
 
 
 def test_output_transform_101():
@@ -39,5 +39,6 @@ def test_output_transform_101():
       [os.path.join(test_rundir, "nighthawk_output_transform"), "--output-format", "human"],
       stdout=subprocess.PIPE,
       input=output)
-  assertEqual(process.returncode, 0)
-  assertIn("Nighthawk - A layer 7 protocol benchmarking tool", process.stdout.decode("utf-8"))
+  utility.assertEqual(process.returncode, 0)
+  utility.assertIn("Nighthawk - A layer 7 protocol benchmarking tool",
+                   process.stdout.decode("utf-8"))