Separate TestRandomGenerator from utility_lib (envoyproxy#25050)

Commit Message: Separate TestRandomGenerator from utility_lib Additional Description: This is a precursor cleanup to envoyproxy#24994. When adding stat() to Filesystem operations, the fake file system will need to keep track of file timestamps. As such, it needs access to a TimeSource to generate those timestamps, i.e. simulated_time_system.h. However, utility.h depends on the Memfile filesystem, and simulated_time_system depended on utility.h, so this creates a circular dependency. simulated_time_system only actually wants random numbers and thread_factory from utility.h, so separating TestRandomGenerator into its own library allows for a reduction in the dependency chain size and a solution to that otherwise-upcoming circular dependency. Risk Level: None, this is test-only and no-op. Testing: Test-only and no-op. Docs Changes: n/a Release Notes: n/a Platform Specific Features: n/a Signed-off-by: Raven Black <[email protected]>
pchaseh · Jan 27, 2023 · 6fffa40 · 6fffa40
1 parent a9e6031
commit 6fffa40
Show file tree

Hide file tree

Showing 11 changed files with 72 additions and 38 deletions.
diff --git a/test/common/event/scaled_range_timer_manager_impl_test.cc b/test/common/event/scaled_range_timer_manager_impl_test.cc
@@ -9,6 +9,7 @@
 #include "test/mocks/common.h"
 #include "test/mocks/event/wrapped_dispatcher.h"
 #include "test/test_common/simulated_time_system.h"
+#include "test/test_common/utility.h"
 
 #include "gtest/gtest.h"
 

diff --git a/test/extensions/access_loggers/open_telemetry/substitution_formatter_speed_test.cc b/test/extensions/access_loggers/open_telemetry/substitution_formatter_speed_test.cc
@@ -3,6 +3,7 @@
 #include "source/extensions/access_loggers/open_telemetry/substitution_formatter.h"
 
 #include "test/common/stream_info/test_util.h"
+#include "test/test_common/utility.h"
 
 #include "benchmark/benchmark.h"
 #include "opentelemetry/proto/common/v1/common.pb.h"

diff --git a/test/extensions/filters/common/expr/context_test.cc b/test/extensions/filters/common/expr/context_test.cc
@@ -9,6 +9,7 @@
 #include "test/mocks/ssl/mocks.h"
 #include "test/mocks/stream_info/mocks.h"
 #include "test/mocks/upstream/host.h"
+#include "test/test_common/utility.h"
 
 #include "absl/time/time.h"
 #include "gmock/gmock.h"

diff --git a/test/test_common/BUILD b/test/test_common/BUILD
@@ -112,6 +112,15 @@ envoy_cc_test_library(
     deps = ["//source/common/singleton:const_singleton"],
 )
 
+envoy_cc_test_library(
+    name = "test_random_generator_lib",
+    srcs = ["test_random_generator.cc"],
+    hdrs = ["test_random_generator.h"],
+    deps = [
+        "//source/common/common:utility_lib",
+    ],
+)
+
 envoy_cc_test_library(
     name = "utility_lib",
     srcs = ["utility.cc"],
@@ -124,6 +133,7 @@ envoy_cc_test_library(
         ":logging_lib",
         ":printers_lib",
         ":resources_lib",
+        ":test_random_generator_lib",
         ":test_time_lib",
         ":thread_factory_for_test_lib",
         "//envoy/buffer:buffer_interface",
@@ -187,6 +197,7 @@ envoy_cc_test_library(
     srcs = ["file_system_for_test.cc"],
     hdrs = ["file_system_for_test.h"],
     deps = [
+        ":simulated_time_system_lib",
         "//source/common/common:utility_lib",
         "//source/common/filesystem:filesystem_lib",
     ],
@@ -273,8 +284,9 @@ envoy_cc_test_library(
     srcs = ["simulated_time_system.cc"],
     hdrs = ["simulated_time_system.h"],
     deps = [
+        ":test_random_generator_lib",
         ":test_time_system_interface",
-        ":utility_lib",
+        ":thread_factory_for_test_lib",
         "//source/common/event:event_impl_base_lib",
         "//source/common/event:real_time_system_lib",
         "//source/common/event:timer_lib",

diff --git a/test/test_common/simulated_time_system.h b/test/test_common/simulated_time_system.h
@@ -4,10 +4,10 @@
 
 #include "source/common/common/lock_guard.h"
 #include "source/common/common/thread.h"
-#include "source/common/common/utility.h"
 
+#include "test/test_common/test_random_generator.h"
 #include "test/test_common/test_time_system.h"
-#include "test/test_common/utility.h"
+#include "test/test_common/thread_factory_for_test.h"
 
 #include "absl/container/flat_hash_map.h"
 

diff --git a/test/test_common/simulated_time_system_test.cc b/test/test_common/simulated_time_system_test.cc
@@ -6,7 +6,6 @@
 #include "test/mocks/common.h"
 #include "test/mocks/event/mocks.h"
 #include "test/test_common/simulated_time_system.h"
-#include "test/test_common/utility.h"
 
 #include "event2/event.h"
 #include "gtest/gtest.h"

diff --git a/test/test_common/test_random_generator.cc b/test/test_common/test_random_generator.cc
@@ -0,0 +1,25 @@
+#include "test/test_common/test_random_generator.h"
+
+#include "gtest/gtest.h"
+
+using testing::GTEST_FLAG(random_seed);
+
+namespace Envoy {
+
+// The purpose of using the static seed here is to use --test_arg=--gtest_random_seed=[seed]
+// to specify the seed of the problem to replay.
+int32_t getSeed() {
+  static const int32_t seed = std::chrono::duration_cast<std::chrono::nanoseconds>(
+                                  std::chrono::system_clock::now().time_since_epoch())
+                                  .count();
+  return seed;
+}
+
+TestRandomGenerator::TestRandomGenerator()
+    : seed_(GTEST_FLAG(random_seed) == 0 ? getSeed() : GTEST_FLAG(random_seed)), generator_(seed_) {
+  ENVOY_LOG_MISC(info, "TestRandomGenerator running with seed {}", seed_);
+}
+
+uint64_t TestRandomGenerator::random() { return generator_(); }
+
+} // namespace Envoy
diff --git a/test/test_common/test_random_generator.h b/test/test_common/test_random_generator.h
@@ -0,0 +1,22 @@
+#pragma once
+
+#include <random>
+
+#include "source/common/common/utility.h"
+
+namespace Envoy {
+
+// Random number generator which logs its seed to stderr. To repeat a test run with a non-zero seed
+// one can run the test with --test_arg=--gtest_random_seed=[seed]
+class TestRandomGenerator {
+public:
+  TestRandomGenerator();
+
+  uint64_t random();
+
+private:
+  const int32_t seed_;
+  std::ranlux48 generator_;
+};
+
+} // namespace Envoy
diff --git a/test/test_common/utility.cc b/test/test_common/utility.cc
@@ -49,22 +49,6 @@ using testing::GTEST_FLAG(random_seed);
 
 namespace Envoy {
 
-// The purpose of using the static seed here is to use --test_arg=--gtest_random_seed=[seed]
-// to specify the seed of the problem to replay.
-int32_t getSeed() {
-  static const int32_t seed = std::chrono::duration_cast<std::chrono::nanoseconds>(
-                                  std::chrono::system_clock::now().time_since_epoch())
-                                  .count();
-  return seed;
-}
-
-TestRandomGenerator::TestRandomGenerator()
-    : seed_(GTEST_FLAG(random_seed) == 0 ? getSeed() : GTEST_FLAG(random_seed)), generator_(seed_) {
-  ENVOY_LOG_MISC(info, "TestRandomGenerator running with seed {}", seed_);
-}
-
-uint64_t TestRandomGenerator::random() { return generator_(); }
-
 bool TestUtility::headerMapEqualIgnoreOrder(const Http::HeaderMap& lhs,
                                             const Http::HeaderMap& rhs) {
   absl::flat_hash_set<std::string> lhs_keys;
@@ -319,6 +303,10 @@ std::vector<std::string> TestUtility::listFiles(const std::string& path, bool re
   return file_names;
 }
 
+std::string TestUtility::uniqueFilename() {
+  return absl::StrCat(getpid(), "_", std::chrono::system_clock::now().time_since_epoch().count());
+}
+
 std::string TestUtility::addLeftAndRightPadding(absl::string_view to_pad, int desired_length) {
   int line_fill_len = desired_length - to_pad.length();
   int first_half_len = line_fill_len / 2;

diff --git a/test/test_common/utility.h b/test/test_common/utility.h
@@ -29,6 +29,7 @@
 #include "test/test_common/file_system_for_test.h"
 #include "test/test_common/logging.h"
 #include "test/test_common/printers.h"
+#include "test/test_common/test_random_generator.h"
 #include "test/test_common/test_time_system.h"
 #include "test/test_common/thread_factory_for_test.h"
 
@@ -129,20 +130,6 @@ class TestEnvoyBug {
   static void callEnvoyBug() { ENVOY_BUG(false, ""); }
 };
 
-// Random number generator which logs its seed to stderr. To repeat a test run with a non-zero seed
-// one can run the test with --test_arg=--gtest_random_seed=[seed]
-class TestRandomGenerator {
-public:
-  TestRandomGenerator();
-
-  uint64_t random();
-
-private:
-  const int32_t seed_;
-  std::ranlux48 generator_;
-  RealTimeSource real_time_source_;
-};
-
 // See https://github.com/envoyproxy/envoy/issues/21245.
 enum class Http1ParserImpl {
   HttpParser, // http-parser from node.js
@@ -357,9 +344,7 @@ class TestUtility {
    * @return a filename based on the process id and current time.
    */
 
-  static std::string uniqueFilename() {
-    return absl::StrCat(getpid(), "_", std::chrono::system_clock::now().time_since_epoch().count());
-  }
+  static std::string uniqueFilename();
 
   /**
    * Compare two protos of the same type for equality.

diff --git a/tools/code_format/config.yaml b/tools/code_format/config.yaml
@@ -147,10 +147,10 @@ paths:
     - test/integration/integration.h
     - test/test_common/simulated_time_system.cc
     - test/test_common/simulated_time_system.h
+    - test/test_common/test_random_generator.cc
     - test/test_common/test_time.cc
     - test/test_common/test_time.h
     - test/test_common/utility.cc
-    - test/test_common/utility.h
     - test/tools/wee8_compile/wee8_compile.cc
 
   # Tests in these paths may make use of the Registry::RegisterFactory constructor or the