tracing: Extract a superclass for tracing unit tests

In an attempt to deflake some of the tracing tests, this superclass tries to set up and clean up PerfettoTracedProcess and the client library task runner consistently. This doesn't cover unit tests for ConsumerHost, which requires a more complicated threading setup. Also addresses another flake in TEDS's startup tracing test. Bug: 1166230 Change-Id: I4c82d4a97d3be293e8461871a3a97ac3e660c1d0 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2729046 Commit-Queue: Eric Seckler <eseckler@chromium.org> Reviewed-by: Stephen Nusko <nuskos@chromium.org> Cr-Commit-Position: refs/heads/master@{#859098}
chromium · Mar 2, 2021 · 41fafbe · 41fafbe
1 parent 36a20e6
commit 41fafbe
Show file tree

Hide file tree

Showing 8 changed files with 151 additions and 126 deletions.
diff --git a/services/tracing/BUILD.gn b/services/tracing/BUILD.gn
@@ -64,6 +64,8 @@ source_set("test_utils") {
 
   deps = [
     ":lib",
+    "//base",
+    "//base/test:test_support",
     "//testing/gtest",
     "//third_party/perfetto/protos/perfetto/common:lite",
     "//third_party/perfetto/protos/perfetto/trace:lite",

diff --git a/services/tracing/perfetto/perfetto_integration_unittest.cc b/services/tracing/perfetto/perfetto_integration_unittest.cc
@@ -11,7 +11,6 @@
 #include "base/run_loop.h"
 #include "base/strings/strcat.h"
 #include "base/test/bind.h"
-#include "base/test/task_environment.h"
 #include "base/threading/thread.h"
 #include "services/tracing/perfetto/perfetto_service.h"
 #include "services/tracing/perfetto/producer_host.h"
@@ -42,44 +41,26 @@ std::string GetPerfettoProducerName() {
   return base::StrCat({mojom::kPerfettoProducerNamePrefix, "123"});
 }
 
-RebindableTaskRunner* GetPerfettoTaskRunner() {
-  static base::NoDestructor<RebindableTaskRunner> task_runner;
-  return task_runner.get();
-}
-
-class PerfettoIntegrationTest : public testing::Test {
+class PerfettoIntegrationTest : public TracingUnitTest {
  public:
-  PerfettoIntegrationTest()
-      : task_environment_(base::test::TaskEnvironment::MainThreadType::IO) {}
-
   void SetUp() override {
-    // Run both the client library and the PerfettoTracedProcess on the same
-    // thread (current thread).
-    auto* perfetto_task_runner = GetPerfettoTaskRunner();
-    auto* perfetto_platform =
-        PerfettoTracedProcess::Get()->perfetto_platform_for_testing();
-    if (!perfetto_platform->did_start_task_runner())
-      perfetto_platform->StartTaskRunner(perfetto_task_runner);
-    perfetto_task_runner->set_task_runner(base::ThreadTaskRunnerHandle::Get());
-
-    PerfettoTracedProcess::ResetTaskRunnerForTesting(
-        base::ThreadTaskRunnerHandle::Get());
-    PerfettoTracedProcess::Get()->ClearDataSourcesForTesting();
+    TracingUnitTest::SetUp();
     data_source_ = TestDataSource::CreateAndRegisterDataSource(
         kPerfettoTestDataSourceName, 0);
     perfetto_service_ = std::make_unique<PerfettoService>();
     RunUntilIdle();
   }
 
-  void TearDown() override { perfetto_service_.reset(); }
+  void TearDown() override {
+    perfetto_service_.reset();
+    TracingUnitTest::TearDown();
+  }
 
   PerfettoService* perfetto_service() const { return perfetto_service_.get(); }
-  void RunUntilIdle() { task_environment_.RunUntilIdle(); }
 
  protected:
   std::unique_ptr<TestDataSource> data_source_;
   std::unique_ptr<PerfettoService> perfetto_service_;
-  base::test::TaskEnvironment task_environment_;
 };
 
 TEST_F(PerfettoIntegrationTest, ProducerDatasourceInitialized) {

diff --git a/services/tracing/perfetto/system_perfetto_unittest.cc b/services/tracing/perfetto/system_perfetto_unittest.cc
@@ -18,7 +18,6 @@
 #include "base/strings/string_number_conversions.h"
 #include "base/test/bind.h"
 #include "base/test/scoped_feature_list.h"
-#include "base/test/task_environment.h"
 #include "base/trace_event/trace_config.h"
 #include "build/build_config.h"
 #include "services/tracing/perfetto/perfetto_service.h"
@@ -47,7 +46,7 @@ namespace {
 
 const char kPerfettoTestDataSourceName[] =
     "org.chromium.chrome_integration_unittest";
-  
+
 std::string GetPerfettoProducerName() {
   return base::StrCat({mojom::kPerfettoProducerNamePrefix, "123"});
 }
@@ -92,12 +91,10 @@ class ClearAndRestoreSystemProducerScope {
   std::unique_ptr<SystemProducer> saved_producer_;
 };
 
-class SystemPerfettoTest : public testing::Test {
+class SystemPerfettoTest : public TracingUnitTest {
  public:
-  SystemPerfettoTest()
-      : task_environment_(base::test::TaskEnvironment::MainThreadType::IO) {
-    PerfettoTracedProcess::ResetTaskRunnerForTesting();
-    PerfettoTracedProcess::Get()->ClearDataSourcesForTesting();
+  void SetUp() override {
+    TracingUnitTest::SetUp();
 
     EXPECT_TRUE(tmp_dir_.CreateUniqueTempDir());
     // We need to set TMPDIR environment variable because when a new producer
@@ -131,6 +128,21 @@ class SystemPerfettoTest : public testing::Test {
     RunUntilIdle();
   }
 
+  void TearDown() override {
+    data_sources_.clear();
+    perfetto_service_.reset();
+
+    if (old_tmp_dir_) {
+      // Restore the old value back to its initial value.
+      setenv("TMPDIR", old_tmp_dir_, true);
+    } else {
+      // TMPDIR wasn't set originally so unset it.
+      unsetenv("TMPDIR");
+    }
+
+    TracingUnitTest::TearDown();
+  }
+
   std::unique_ptr<MockPosixSystemProducer> CreateMockPosixSystemProducer(
       MockSystemService* service,
       int num_data_sources_expected = 0,
@@ -173,24 +185,7 @@ class SystemPerfettoTest : public testing::Test {
                                                producer_socket_);
   }
 
-  ~SystemPerfettoTest() override {
-    RunUntilIdle();
-    // The producer client will be destroyed in the next iteration of the test,
-    // but the sequence it was used on disappears with the
-    // |task_environment_|. So we reset the sequence so it can be freely
-    // destroyed.
-    PerfettoTracedProcess::Get()->producer_client()->ResetSequenceForTesting();
-    if (old_tmp_dir_) {
-      // Restore the old value back to its initial value.
-      setenv("TMPDIR", old_tmp_dir_, true);
-    } else {
-      // TMPDIR wasn't set originally so unset it.
-      unsetenv("TMPDIR");
-    }
-  }
-
   PerfettoService* local_service() const { return perfetto_service_.get(); }
-  void RunUntilIdle() { task_environment_.RunUntilIdle(); }
 
   // Fork() + executes the perfetto cmdline client with the given args and
   // returns true if we exited with a success otherwise |stderr_| is populated
@@ -243,7 +238,6 @@ class SystemPerfettoTest : public testing::Test {
   std::string consumer_socket_;
   std::unique_ptr<PerfettoService> perfetto_service_;
   std::vector<std::unique_ptr<TestDataSource>> data_sources_;
-  base::test::TaskEnvironment task_environment_;
   std::string stderr_;
   const char* old_tmp_dir_ = nullptr;
 };

diff --git a/services/tracing/perfetto/test_utils.cc b/services/tracing/perfetto/test_utils.cc
@@ -7,6 +7,7 @@
 
 #include "base/bind.h"
 #include "base/run_loop.h"
+#include "services/tracing/public/cpp/perfetto/perfetto_platform.h"
 #include "services/tracing/public/cpp/perfetto/shared_memory.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/perfetto/include/perfetto/ext/tracing/core/commit_data_request.h"
@@ -19,6 +20,7 @@
 
 namespace tracing {
 namespace {
+
 perfetto::TraceConfig GetDefaultTraceConfig(
     const std::vector<std::string>& data_sources) {
   perfetto::TraceConfig trace_config;
@@ -30,7 +32,14 @@ perfetto::TraceConfig GetDefaultTraceConfig(
   }
   return trace_config;
 }
+
+RebindableTaskRunner* GetClientLibTaskRunner() {
+  static base::NoDestructor<RebindableTaskRunner> task_runner;
+  return task_runner.get();
+}
+
 }  // namespace
+
 // static
 std::unique_ptr<TestDataSource> TestDataSource::CreateAndRegisterDataSource(
     const std::string& data_source_name,
@@ -441,4 +450,49 @@ bool RebindableTaskRunner::RunsTasksInCurrentSequence() const {
   return task_runner_->RunsTasksInCurrentSequence();
 }
 
+TracingUnitTest::TracingUnitTest()
+    : task_environment_(std::make_unique<base::test::TaskEnvironment>(
+          base::test::TaskEnvironment::MainThreadType::IO)) {}
+
+TracingUnitTest::~TracingUnitTest() {
+  CHECK(setup_called_ && teardown_called_);
+}
+
+void TracingUnitTest::SetUp() {
+  setup_called_ = true;
+
+  // Since Perfetto's platform backend can only be initialized once in a
+  // process, we give it a task runner that can outlive the per-test task
+  // environment.
+  auto* client_lib_task_runner = GetClientLibTaskRunner();
+  auto* perfetto_platform =
+      PerfettoTracedProcess::Get()->perfetto_platform_for_testing();
+  if (!perfetto_platform->did_start_task_runner())
+    perfetto_platform->StartTaskRunner(client_lib_task_runner);
+  client_lib_task_runner->set_task_runner(base::ThreadTaskRunnerHandle::Get());
+
+  // Also tell PerfettoTracedProcess to use the current task environment.
+  PerfettoTracedProcess::ResetTaskRunnerForTesting(
+      base::ThreadTaskRunnerHandle::Get());
+  PerfettoTracedProcess::Get()->ClearDataSourcesForTesting();
+
+  // Wait for any posted construction tasks to execute.
+  RunUntilIdle();
+}
+
+void TracingUnitTest::TearDown() {
+  teardown_called_ = true;
+
+  // Wait for any posted destruction tasks to execute.
+  RunUntilIdle();
+
+  // From here on, no more tasks should be posted.
+  task_environment_.reset();
+
+  // Clear task runner and data sources.
+  PerfettoTracedProcess::Get()->GetTaskRunner()->ResetTaskRunnerForTesting(
+      nullptr);
+  PerfettoTracedProcess::Get()->ClearDataSourcesForTesting();
+}
+
 }  // namespace tracing
diff --git a/services/tracing/perfetto/test_utils.h b/services/tracing/perfetto/test_utils.h
@@ -9,11 +9,13 @@
 #include <string>
 #include <vector>
 
+#include "base/test/task_environment.h"
 #include "mojo/public/cpp/bindings/receiver.h"
 #include "services/tracing/perfetto/perfetto_service.h"
 #include "services/tracing/perfetto/producer_host.h"
 #include "services/tracing/public/cpp/perfetto/perfetto_traced_process.h"
 #include "services/tracing/public/cpp/perfetto/producer_client.h"
+#include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/perfetto/include/perfetto/ext/tracing/core/consumer.h"
 #include "third_party/perfetto/include/perfetto/tracing/core/trace_config.h"
 #include "third_party/perfetto/protos/perfetto/common/observable_events.pb.h"
@@ -251,6 +253,25 @@ class RebindableTaskRunner : public base::SequencedTaskRunner {
   scoped_refptr<base::SequencedTaskRunner> task_runner_;
 };
 
+// Base class for various tracing unit tests, ensuring cleanup of
+// PerfettoTracedProcess. Tracing tasks are run on the test thread.
+class TracingUnitTest : public testing::Test {
+ public:
+  TracingUnitTest();
+  ~TracingUnitTest() override;
+
+  void SetUp() override;
+  void TearDown() override;
+
+ protected:
+  void RunUntilIdle() { task_environment_->RunUntilIdle(); }
+
+ private:
+  std::unique_ptr<base::test::TaskEnvironment> task_environment_;
+  bool setup_called_ = false;
+  bool teardown_called_ = false;
+};
+
 }  // namespace tracing
 
 #endif  // SERVICES_TRACING_PERFETTO_TEST_UTILS_H_