From 565c24def0ffe4548eb9bc0d9dbbc69aad940b3d Mon Sep 17 00:00:00 2001 From: Moti Zilberman Date: Fri, 10 Oct 2025 13:17:28 -0700 Subject: [PATCH 1/2] Add tests for Tracing domain reporting of network requests Summary: Changelog: [Internal] Adds an integration test for `NetworkReporter`'s CDP Tracing domain output (via `PerformanceTracer` behind the scenes), i.e. the traces that power the Network track in the Performance panel in React Native DevTools. The test covers the `Tracing` and `Network` domains being enabled simultaneously, as well as `Tracing` on its own. Differential Revision: D84337901 --- .../tests/NetworkReporterTest.cpp | 255 +++++++++++++++++- .../JsiIntegrationTestHermesEngineAdapter.cpp | 8 +- 2 files changed, 250 insertions(+), 13 deletions(-) diff --git a/packages/react-native/ReactCommon/jsinspector-modern/tests/NetworkReporterTest.cpp b/packages/react-native/ReactCommon/jsinspector-modern/tests/NetworkReporterTest.cpp index 896802818606b8..dea8c2e30de47a 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/tests/NetworkReporterTest.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/tests/NetworkReporterTest.cpp @@ -19,7 +19,7 @@ namespace facebook::react::jsinspector_modern { namespace { -struct Params { +struct NetworkReporterTestParams { bool enableNetworkEventReporting; }; @@ -27,18 +27,21 @@ struct Params { /** * A test fixture for the way the internal NetworkReporter API interacts with - * the CDP Network domain. + * the CDP Network and Tracing domains. */ -class NetworkReporterTest : public JsiIntegrationPortableTestBase< - JsiIntegrationTestHermesEngineAdapter, - folly::QueuedImmediateExecutor>, - public WithParamInterface { +template + requires std::convertible_to +class NetworkReporterTestBase : public JsiIntegrationPortableTestBase< + JsiIntegrationTestHermesEngineAdapter, + folly::QueuedImmediateExecutor>, + public WithParamInterface { protected: - NetworkReporterTest() + NetworkReporterTestBase() : JsiIntegrationPortableTestBase({ .networkInspectionEnabled = true, .enableNetworkEventReporting = - GetParam().enableNetworkEventReporting, + WithParamInterface::GetParam() + .enableNetworkEventReporting, }) {} void SetUp() override { @@ -65,6 +68,58 @@ class NetworkReporterTest : public JsiIntegrationPortableTestBase< urlMatcher); } + void startTracing() { + this->expectMessageFromPage(JsonEq(R"({ + "id": 1, + "result": {} + })")); + + this->toPage_->sendMessage(R"({ + "id": 1, + "method": "Tracing.start" + })"); + } + + /** + * Helper method to end tracing and collect all trace events from potentially + * multiple chunked Tracing.dataCollected messages. + * \returns A vector containing all collected trace events + */ + std::vector endTracingAndCollectEvents() { + InSequence s; + + this->expectMessageFromPage(JsonEq(R"({ + "id": 1, + "result": {} + })")); + + std::vector allTraceEvents; + + EXPECT_CALL( + fromPage(), + onMessage(JsonParsed(AtJsonPtr("/method", "Tracing.dataCollected")))) + .Times(AtLeast(1)) + .WillRepeatedly(Invoke([&allTraceEvents](const std::string& message) { + auto parsedMessage = folly::parseJson(message); + auto& events = parsedMessage.at("params").at("value"); + allTraceEvents.insert( + allTraceEvents.end(), + std::make_move_iterator(events.begin()), + std::make_move_iterator(events.end())); + })); + + this->expectMessageFromPage(JsonParsed(AllOf( + AtJsonPtr("/method", "Tracing.tracingComplete"), + AtJsonPtr("/params/dataLossOccurred", false)))); + + this->toPage_->sendMessage(R"({ + "id": 1, + "method": "Tracing.end" + })"); + + return allTraceEvents; + } + private: std::optional getScriptUrlById(const std::string& scriptId) { auto it = scriptUrlsById_.find(scriptId); @@ -77,6 +132,8 @@ class NetworkReporterTest : public JsiIntegrationPortableTestBase< std::unordered_map scriptUrlsById_; }; +using NetworkReporterTest = NetworkReporterTestBase; + TEST_P(NetworkReporterTest, testNetworkEnableDisable) { InSequence s; @@ -581,12 +638,186 @@ TEST_P(NetworkReporterTest, testCreateRequestIdWithoutNetworkDomain) { EXPECT_NE(id1, id2); } -static const auto paramValues = testing::Values( - Params{.enableNetworkEventReporting = true}, - Params{ +struct NetworkReporterTracingTestParams { + bool enableNetworkEventReporting; + bool enableNetworkDomain; + + operator NetworkReporterTestParams() const { + return NetworkReporterTestParams{ + .enableNetworkEventReporting = enableNetworkEventReporting, + }; + } +}; + +using NetworkReporterTracingTest = + NetworkReporterTestBase; + +TEST_P( + NetworkReporterTracingTest, + testReportsToTracingDomainPlusNetworkDomain) { + InSequence s; + + this->startTracing(); + + if (GetParam().enableNetworkDomain) { + this->expectMessageFromPage(JsonEq(R"({ + "id": 1, + "result": {} + })")); + this->toPage_->sendMessage(R"({ + "id": 1, + "method": "Network.enable" + })"); + + this->expectMessageFromPage(JsonParsed(AllOf( + AtJsonPtr("/method", "Network.requestWillBeSent"), + AtJsonPtr("/params/requestId", "trace-events-request"), + AtJsonPtr("/params/loaderId", ""), + AtJsonPtr("/params/documentURL", "mobile"), + AtJsonPtr("/params/request/url", "https://trace.example.com/events"), + AtJsonPtr("/params/request/method", "GET"), + AtJsonPtr("/params/request/headers/Accept", "application/json"), + AtJsonPtr("/params/timestamp", Gt(0)), + AtJsonPtr("/params/wallTime", Gt(0)), + AtJsonPtr("/params/initiator/type", "script"), + AtJsonPtr("/params/redirectHasExtraInfo", false)))); + + this->expectMessageFromPage(JsonParsed(AllOf( + AtJsonPtr("/method", "Network.requestWillBeSentExtraInfo"), + AtJsonPtr("/params/requestId", "trace-events-request"), + AtJsonPtr("/params/associatedCookies", "[]"_json), + AtJsonPtr("/params/headers", "{}"_json), + AtJsonPtr("/params/connectTiming/requestTime", Gt(0))))); + + this->expectMessageFromPage(JsonParsed(AllOf( + AtJsonPtr("/method", "Network.responseReceived"), + AtJsonPtr("/params/requestId", "trace-events-request"), + AtJsonPtr("/params/loaderId", ""), + AtJsonPtr("/params/timestamp", Gt(0)), + AtJsonPtr("/params/type", "XHR"), + AtJsonPtr("/params/response/url", "https://trace.example.com/events"), + AtJsonPtr("/params/response/status", 200), + AtJsonPtr("/params/response/statusText", "OK"), + AtJsonPtr("/params/response/headers/Content-Type", "application/json"), + AtJsonPtr("/params/response/mimeType", "application/json"), + AtJsonPtr("/params/response/encodedDataLength", 1024), + AtJsonPtr("/params/hasExtraInfo", false)))); + + this->expectMessageFromPage(JsonParsed(AllOf( + AtJsonPtr("/method", "Network.loadingFinished"), + AtJsonPtr("/params/requestId", "trace-events-request"), + AtJsonPtr("/params/timestamp", Gt(0)), + AtJsonPtr("/params/encodedDataLength", 1024)))); + } + + NetworkReporter::getInstance().reportRequestStart( + "trace-events-request", + { + .url = "https://trace.example.com/events", + .httpMethod = "GET", + .headers = Headers{{"Accept", "application/json"}}, + }, + 0, + std::nullopt); + + NetworkReporter::getInstance().reportConnectionTiming( + "trace-events-request", std::nullopt); + + NetworkReporter::getInstance().reportResponseStart( + "trace-events-request", + { + .url = "https://trace.example.com/events", + .statusCode = 200, + .headers = Headers{{"Content-Type", "application/json"}}, + }, + 1024); + + NetworkReporter::getInstance().reportResponseEnd( + "trace-events-request", 1024); + + auto allTraceEvents = endTracingAndCollectEvents(); + + EXPECT_THAT( + allTraceEvents, + Contains(AllOf( + AtJsonPtr("/name", "ResourceSendRequest"), + AtJsonPtr("/cat", "devtools.timeline"), + AtJsonPtr("/ph", "I"), + AtJsonPtr("/s", "t"), + AtJsonPtr("/tid", oscompat::getCurrentThreadId()), + AtJsonPtr("/pid", oscompat::getCurrentProcessId()), + AtJsonPtr("/args/data/initiator", "{}"_json), + AtJsonPtr("/args/data/requestId", "trace-events-request"), + AtJsonPtr("/args/data/url", "https://trace.example.com/events"), + AtJsonPtr("/args/data/requestMethod", "GET"), + AtJsonPtr("/args/data/priority", "VeryHigh"), + AtJsonPtr("/args/data/renderBlocking", "non_blocking"), + AtJsonPtr("/args/data/resourceType", "Other")))); + + EXPECT_THAT( + allTraceEvents, + Contains(AllOf( + AtJsonPtr("/name", "ResourceReceiveResponse"), + AtJsonPtr("/cat", "devtools.timeline"), + AtJsonPtr("/ph", "I"), + AtJsonPtr("/s", "t"), + AtJsonPtr("/tid", oscompat::getCurrentThreadId()), + AtJsonPtr("/pid", oscompat::getCurrentProcessId()), + AtJsonPtr("/ts", Gt(0)), + AtJsonPtr("/args/data/requestId", "trace-events-request"), + AtJsonPtr("/args/data/statusCode", 200), + AtJsonPtr("/args/data/mimeType", "application/json"), + AtJsonPtr("/args/data/protocol", "h2"), + AtJsonPtr("/args/data/encodedDataLength", 1024), + AtJsonPtr( + "/args/data/headers", + R"([{ "name": "Content-Type", "value": "application/json" }])"_json), + AtJsonPtr( + "/args/data/timing", + AllOf( + AtJsonPtr("/requestTime", Ge(0)), + AtJsonPtr("/sendStart", Ge(0)), + AtJsonPtr("/sendEnd", Ge(0)), + AtJsonPtr("/receiveHeadersStart", Ge(0)), + AtJsonPtr("/receiveHeadersEnd", Ge(0))))))); + + EXPECT_THAT( + allTraceEvents, + Contains(AllOf( + AtJsonPtr("/name", "ResourceFinish"), + AtJsonPtr("/cat", "devtools.timeline"), + AtJsonPtr("/ph", "I"), + AtJsonPtr("/s", "t"), + AtJsonPtr("/tid", oscompat::getCurrentThreadId()), + AtJsonPtr("/pid", oscompat::getCurrentProcessId()), + AtJsonPtr("/args/data/requestId", "trace-events-request"), + AtJsonPtr("/args/data/encodedDataLength", 1024), + AtJsonPtr("/args/data/decodedBodyLength", 0), + AtJsonPtr("/args/data/didFail", false)))); +} + +static const auto networkReporterTestParamValues = testing::Values( + NetworkReporterTestParams{.enableNetworkEventReporting = true}, + NetworkReporterTestParams{ .enableNetworkEventReporting = false, }); -INSTANTIATE_TEST_SUITE_P(NetworkReporterTest, NetworkReporterTest, paramValues); +static const auto networkReporterTracingTestParamValues = testing::Values( + NetworkReporterTracingTestParams{ + .enableNetworkEventReporting = true, + .enableNetworkDomain = true}, + NetworkReporterTracingTestParams{ + .enableNetworkEventReporting = true, + .enableNetworkDomain = false}); + +INSTANTIATE_TEST_SUITE_P( + NetworkReporterTest, + NetworkReporterTest, + networkReporterTestParamValues); + +INSTANTIATE_TEST_SUITE_P( + NetworkReporterTracingTest, + NetworkReporterTracingTest, + networkReporterTracingTestParamValues); } // namespace facebook::react::jsinspector_modern diff --git a/packages/react-native/ReactCommon/jsinspector-modern/tests/engines/JsiIntegrationTestHermesEngineAdapter.cpp b/packages/react-native/ReactCommon/jsinspector-modern/tests/engines/JsiIntegrationTestHermesEngineAdapter.cpp index ad2b3e663bfc5d..19a8af8c5c6100 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/tests/engines/JsiIntegrationTestHermesEngineAdapter.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/tests/engines/JsiIntegrationTestHermesEngineAdapter.cpp @@ -17,7 +17,13 @@ JsiIntegrationTestHermesEngineAdapter::JsiIntegrationTestHermesEngineAdapter( ::hermes::vm::CompilationMode::ForceLazyCompilation) .build())}, jsExecutor_{jsExecutor}, - runtimeTargetDelegate_{runtime_} {} + runtimeTargetDelegate_{runtime_} { + // NOTE: In React Native, registerForProfiling is called by + // HermesInstance::unstable_initializeOnJsThread, called from + // ReactInstance::initializeRuntime. Ideally, we should figure out how to + // manages this from inside the CDP backend, + runtime_->registerForProfiling(); +} RuntimeTargetDelegate& JsiIntegrationTestHermesEngineAdapter::getRuntimeTargetDelegate() { From fdd34b7f1589c6abe719c912ee44368f8e7be19b Mon Sep 17 00:00:00 2001 From: Moti Zilberman Date: Fri, 10 Oct 2025 13:17:28 -0700 Subject: [PATCH 2/2] Better source locations for EXPECT_CALL failures Summary: Changelog: [Internal] The team maintaining gmock is [uninterested, as a matter of principle,](https://github.com/google/googletest/issues/2646#issuecomment-630919122) 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. Differential Revision: D84368993 --- .../tests/ConsoleApiTest.cpp | 38 ++++++++++++------- .../jsinspector-modern/tests/GmockHelpers.h | 28 ++++++++++++++ .../tests/JsiIntegrationTest.h | 27 +++++++++---- 3 files changed, 71 insertions(+), 22 deletions(-) create mode 100644 packages/react-native/ReactCommon/jsinspector-modern/tests/GmockHelpers.h diff --git a/packages/react-native/ReactCommon/jsinspector-modern/tests/ConsoleApiTest.cpp b/packages/react-native/ReactCommon/jsinspector-modern/tests/ConsoleApiTest.cpp index 6a20aac06d31c6..f62ec8063f6001 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/tests/ConsoleApiTest.cpp +++ b/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 paramsMatcher) { + void expectConsoleApiCall( + Matcher 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 paramsMatcher) { + void expectConsoleApiCallImmediate( + Matcher 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& paramsMatcher) { + const Matcher& paramsMatcher, + std::source_location location = std::source_location::current()) { if (!runtimeEnabled_) { - expectedConsoleApiCalls_.emplace_back(paramsMatcher); + expectedConsoleApiCalls_.emplace_back(paramsMatcher, location); } } @@ -145,10 +150,14 @@ class ConsoleApiTest : public JsiIntegrationPortableTestBase< return it->second; } - void expectConsoleApiCallImpl(Matcher paramsMatcher) { - this->expectMessageFromPage(JsonParsed(AllOf( - AtJsonPtr("/method", "Runtime.consoleAPICalled"), - AtJsonPtr("/params", std::move(paramsMatcher))))); + void expectConsoleApiCallImpl( + Matcher paramsMatcher, + std::source_location location) { + this->expectMessageFromPage( + JsonParsed(AllOf( + AtJsonPtr("/method", "Runtime.consoleAPICalled"), + AtJsonPtr("/params", std::move(paramsMatcher)))), + location); } void enableRuntimeDomain() { @@ -156,8 +165,8 @@ class ConsoleApiTest : public JsiIntegrationPortableTestBase< 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> expectedConsoleApiCalls_; + std::vector, std::source_location>> + expectedConsoleApiCalls_; bool runtimeEnabled_{false}; std::unordered_map scriptUrlsById_; }; diff --git a/packages/react-native/ReactCommon/jsinspector-modern/tests/GmockHelpers.h b/packages/react-native/ReactCommon/jsinspector-modern/tests/GmockHelpers.h new file mode 100644 index 00000000000000..164d9e44413a0b --- /dev/null +++ b/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 + +#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) diff --git a/packages/react-native/ReactCommon/jsinspector-modern/tests/JsiIntegrationTest.h b/packages/react-native/ReactCommon/jsinspector-modern/tests/JsiIntegrationTest.h index 050bead57ac3d2..d6a34b24f56953 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/tests/JsiIntegrationTest.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/tests/JsiIntegrationTest.h @@ -16,8 +16,10 @@ #include #include +#include #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 std::shared_ptr> 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::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,7 +196,8 @@ class JsiIntegrationPortableTestBase : public ::testing::Test, JsiIntegrationPortableTestBase& test_; }; - SecondaryConnection connectSecondary() { + SecondaryConnection connectSecondary( + std::source_location location = std::source_location::current()) { auto toPage = page_->connect(remoteConnections_.make_unique()); SecondaryConnection secondary{ @@ -196,7 +205,8 @@ class JsiIntegrationPortableTestBase : public ::testing::Test, 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; }