Better source locations for EXPECT_CALL failures (facebook#54129)

Summary:
Pull Request resolved: facebook#54129

Changelog: [Internal]

The team maintaining gmock is [uninterested, as a matter of principle,](google/googletest#2646 (comment)) in improving the ergonomics of writing helper functions around `EXPECT_CALL`. However, such helpers have proven very useful for the `jsinspector-modern` C++ test suite. The out-of-the-box ergonomics *are* pretty bad, though, so this diff tries to improve the situation.

The biggest offender in our test suite is the `expectMessageFromPage` helper function - if a test fails because an expectation isn't met, the error message points to the `EXPECT_CALL` line inside `expectMessageFromPage`, which is utterly useless compared to the line of test code that *called* `expectMessageFromPage`.

Here, we reach into gmock's internals slightly to create a variant of the `EXPECT_CALL` macro that allows passing in a [`std::source_location`](https://en.cppreference.com/w/cpp/utility/source_location.html) (thanks, C++20!). We then teach `expectMessageFromPage` and other such helpers to capture a source location at the call site (using a parameter with a default value to `source_location::current()`) and use the modified macro to pass it into gmock.

Reviewed By: huntie

Differential Revision: D84368993

fbshipit-source-id: 62f64b6c5f626a54bc3da7143d84ee2d75d16ac6

Author: motiz88

packages/react-native/ReactCommon/jsinspector-modern/tests/ConsoleApiTest.cpp

@@ -90,11 +90,13 @@ class ConsoleApiTest : public JsiIntegrationPortableTestBase<
    * Expect a console API call to be reported with parameters matching \param
    * paramsMatcher.
    */
-  void expectConsoleApiCall(Matcher<folly::dynamic> paramsMatcher) {
+  void expectConsoleApiCall(
+      Matcher<folly::dynamic> paramsMatcher,
+      std::source_location location = std::source_location::current()) {
     if (runtimeEnabled_) {
-      expectConsoleApiCallImpl(std::move(paramsMatcher));
+      expectConsoleApiCallImpl(std::move(paramsMatcher), location);
     } else {
-      expectedConsoleApiCalls_.emplace_back(paramsMatcher);
+      expectedConsoleApiCalls_.emplace_back(paramsMatcher, location);
     }
   }
 
@@ -103,9 +105,11 @@ class ConsoleApiTest : public JsiIntegrationPortableTestBase<
    * paramsMatcher, only if the Runtime domain is currently enabled ( = the call
    * is reported in real time).
    */
-  void expectConsoleApiCallImmediate(Matcher<folly::dynamic> paramsMatcher) {
+  void expectConsoleApiCallImmediate(
+      Matcher<folly::dynamic> paramsMatcher,
+      std::source_location location = std::source_location::current()) {
     if (runtimeEnabled_) {
-      expectConsoleApiCallImpl(std::move(paramsMatcher));
+      expectConsoleApiCallImpl(std::move(paramsMatcher), location);
     }
   }
 
@@ -115,9 +119,10 @@ class ConsoleApiTest : public JsiIntegrationPortableTestBase<
    * call will be buffered and reported later upon enabling the domain).
    */
   void expectConsoleApiCallBuffered(
-      const Matcher<folly::dynamic>& paramsMatcher) {
+      const Matcher<folly::dynamic>& paramsMatcher,
+      std::source_location location = std::source_location::current()) {
     if (!runtimeEnabled_) {
-      expectedConsoleApiCalls_.emplace_back(paramsMatcher);
+      expectedConsoleApiCalls_.emplace_back(paramsMatcher, location);
     }
   }
 
@@ -145,19 +150,23 @@ class ConsoleApiTest : public JsiIntegrationPortableTestBase<
     return it->second;
   }
 
-  void expectConsoleApiCallImpl(Matcher<folly::dynamic> paramsMatcher) {
-    this->expectMessageFromPage(JsonParsed(AllOf(
-        AtJsonPtr("/method", "Runtime.consoleAPICalled"),
-        AtJsonPtr("/params", std::move(paramsMatcher)))));
+  void expectConsoleApiCallImpl(
+      Matcher<folly::dynamic> paramsMatcher,
+      std::source_location location) {
+    this->expectMessageFromPage(
+        JsonParsed(AllOf(
+            AtJsonPtr("/method", "Runtime.consoleAPICalled"),
+            AtJsonPtr("/params", std::move(paramsMatcher)))),
+        location);
   }
 
   void enableRuntimeDomain() {
     InSequence s;
     auto executionContextInfo = this->expectMessageFromPage(JsonParsed(
         AllOf(AtJsonPtr("/method", "Runtime.executionContextCreated"))));
     if (!runtimeEnabled_) {
-      for (auto& call : expectedConsoleApiCalls_) {
-        expectConsoleApiCallImpl(call);
+      for (auto& [call, location] : expectedConsoleApiCalls_) {
+        expectConsoleApiCallImpl(call, location);
       }
       expectedConsoleApiCalls_.clear();
     }
@@ -200,7 +209,8 @@ class ConsoleApiTest : public JsiIntegrationPortableTestBase<
     }
   }
 
-  std::vector<Matcher<folly::dynamic>> expectedConsoleApiCalls_;
+  std::vector<std::pair<Matcher<folly::dynamic>, std::source_location>>
+      expectedConsoleApiCalls_;
   bool runtimeEnabled_{false};
   std::unordered_map<std::string, std::string> scriptUrlsById_;
 };

packages/react-native/ReactCommon/jsinspector-modern/tests/GmockHelpers.h

@@ -0,0 +1,28 @@
+/*
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+#include <gmock/gmock.h>
+
+#pragma once
+
+/**
+ * A variant of GMOCK_ON_CALL_IMPL that allows specifying the source location as
+ * a std::source_location parameter.
+ */
+#define GMOCK_ON_CALL_WITH_SOURCE_LOCATION_IMPL_(         \
+    location, mock_expr, Setter, call)                    \
+  ((mock_expr).gmock_##call)(                             \
+      ::testing::internal::GetWithoutMatchers(), nullptr) \
+      .Setter((location).file_name(), (location).line(), #mock_expr, #call)
+
+/**
+ * A variant of EXPECT_CALL that allows specifying the source location as a
+ * std::source_location parameter;
+ */
+#define EXPECT_CALL_WITH_SOURCE_LOCATION(location, obj, call) \
+  GMOCK_ON_CALL_WITH_SOURCE_LOCATION_IMPL_(                   \
+      location, obj, InternalExpectedAt, call)

packages/react-native/ReactCommon/jsinspector-modern/tests/JsiIntegrationTest.h

@@ -16,8 +16,10 @@
 #include <jsinspector-modern/InspectorInterfaces.h>
 
 #include <memory>
+#include <source_location>
 
 #include "FollyDynamicMatchers.h"
+#include "GmockHelpers.h"
 #include "InspectorMocks.h"
 #include "UniquePtrFactory.h"
 #include "utils/InspectorFlagOverridesGuard.h"
@@ -88,13 +90,15 @@ class JsiIntegrationPortableTestBase : public ::testing::Test,
    */
   virtual void setupRuntimeBeforeRegistration(jsi::Runtime& /*runtime*/) {}
 
-  void connect() {
+  void connect(
+      std::source_location location = std::source_location::current()) {
     ASSERT_FALSE(toPage_) << "Can only connect once in a JSI integration test.";
     toPage_ = page_->connect(remoteConnections_.make_unique());
 
     using namespace ::testing;
     // Default to ignoring console messages originating inside the backend.
-    EXPECT_CALL(
+    EXPECT_CALL_WITH_SOURCE_LOCATION(
+        location,
         fromPage(),
         onMessage(JsonParsed(AllOf(
             AtJsonPtr("/method", "Runtime.consoleAPICalled"),
@@ -103,7 +107,8 @@ class JsiIntegrationPortableTestBase : public ::testing::Test,
 
     // We'll always get an onDisconnect call when we tear
     // down the test. Expect it in order to satisfy the strict mock.
-    EXPECT_CALL(*remoteConnections_[0], onDisconnect());
+    EXPECT_CALL_WITH_SOURCE_LOCATION(
+        location, *remoteConnections_[0], onDisconnect());
   }
 
   void reload() {
@@ -146,10 +151,13 @@ class JsiIntegrationPortableTestBase : public ::testing::Test,
    */
   template <typename Matcher>
   std::shared_ptr<const std::optional<folly::dynamic>> expectMessageFromPage(
-      Matcher&& matcher) {
+      Matcher&& matcher,
+      std::source_location location = std::source_location::current()) {
+    using namespace ::testing;
+    ScopedTrace scope(location.file_name(), location.line(), "");
     std::shared_ptr result =
         std::make_shared<std::optional<folly::dynamic>>(std::nullopt);
-    EXPECT_CALL(fromPage(), onMessage(matcher))
+    EXPECT_CALL_WITH_SOURCE_LOCATION(location, fromPage(), onMessage(matcher))
         .WillOnce(
             ([result](auto message) { *result = folly::parseJson(message); }))
         .RetiresOnSaturation();
@@ -188,15 +196,17 @@ class JsiIntegrationPortableTestBase : public ::testing::Test,
     JsiIntegrationPortableTestBase<EngineAdapter, Executor>& test_;
   };
 
-  SecondaryConnection connectSecondary() {
+  SecondaryConnection connectSecondary(
+      std::source_location location = std::source_location::current()) {
     auto toPage = page_->connect(remoteConnections_.make_unique());
 
     SecondaryConnection secondary{
         std::move(toPage), *this, remoteConnections_.objectsVended() - 1};
 
     using namespace ::testing;
     // Default to ignoring console messages originating inside the backend.
-    EXPECT_CALL(
+    EXPECT_CALL_WITH_SOURCE_LOCATION(
+        location,
         secondary.fromPage(),
         onMessage(JsonParsed(AllOf(
             AtJsonPtr("/method", "Runtime.consoleAPICalled"),
@@ -205,7 +215,8 @@ class JsiIntegrationPortableTestBase : public ::testing::Test,
 
     // We'll always get an onDisconnect call when we tear
     // down the test. Expect it in order to satisfy the strict mock.
-    EXPECT_CALL(secondary.fromPage(), onDisconnect());
+    EXPECT_CALL_WITH_SOURCE_LOCATION(
+        location, secondary.fromPage(), onDisconnect());
 
     return secondary;
   }