Skip to content

Commit

Permalink
Enable JSI integration tests for Hermes CDPAgent (#43391)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #43391

## Context

We are migrating to the new Hermes `CDPAgent` and `CDPDebugAPI` APIs in the modern CDP server (previously `HermesCDPHandler`).

## This diff

Bootstraps `HermesRuntimeAgentDelegateNew` within `JsiIntegrationTest.cpp`.

Test cases which currently do not pass with `HermesRuntimeAgentDelegateNew` are selectively matched against a new alias to exclude them: `JsiIntegrationHermesLegacyTest`.

Changelog: [Internal]

Reviewed By: motiz88

Differential Revision: D53810357

fbshipit-source-id: 2d7d7446038530d19d93add71361b4bf581cff18
  • Loading branch information
huntie authored and facebook-github-bot committed Mar 11, 2024
1 parent c6ca7d6 commit a7fde7c
Show file tree
Hide file tree
Showing 9 changed files with 109 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "UniquePtrFactory.h"
#include "engines/JsiIntegrationTestGenericEngineAdapter.h"
#include "engines/JsiIntegrationTestHermesEngineAdapter.h"
#include "engines/JsiIntegrationTestHermesWithCDPAgentEngineAdapter.h"

using namespace ::testing;
using folly::sformat;
Expand Down Expand Up @@ -52,7 +53,9 @@ class JsiIntegrationPortableTest : public Test, private HostTargetDelegate {
folly::QueuedImmediateExecutor immediateExecutor_;

protected:
JsiIntegrationPortableTest() : engineAdapter_{immediateExecutor_} {
JsiIntegrationPortableTest()
: inspectorFlagsGuard_{EngineAdapter::getInspectorFlagOverrides()},
engineAdapter_{immediateExecutor_} {
instance_ = &page_->registerInstance(instanceTargetDelegate_);
runtimeTarget_ = &instance_->registerRuntime(
engineAdapter_->getRuntimeTargetDelegate(),
Expand Down Expand Up @@ -136,6 +139,7 @@ class JsiIntegrationPortableTest : public Test, private HostTargetDelegate {
InstanceTarget* instance_{};
RuntimeTarget* runtimeTarget_{};

InspectorFlagOverridesGuard inspectorFlagsGuard_;
MockInstanceTargetDelegate instanceTargetDelegate_;
std::optional<EngineAdapter> engineAdapter_;

Expand Down Expand Up @@ -168,17 +172,27 @@ class JsiIntegrationPortableTest : public Test, private HostTargetDelegate {
*/
using AllEngines = Types<
JsiIntegrationTestHermesEngineAdapter,
JsiIntegrationTestHermesWithCDPAgentEngineAdapter,
JsiIntegrationTestGenericEngineAdapter>;

using AllHermesVariants = Types<JsiIntegrationTestHermesEngineAdapter>;
using AllHermesVariants = Types<
JsiIntegrationTestHermesEngineAdapter,
JsiIntegrationTestHermesWithCDPAgentEngineAdapter>;

using LegacyHermesVariants = Types<JsiIntegrationTestHermesEngineAdapter>;

TYPED_TEST_SUITE(JsiIntegrationPortableTest, AllEngines);

template <typename EngineAdapter>
using JsiIntegrationHermesTest = JsiIntegrationPortableTest<EngineAdapter>;
TYPED_TEST_SUITE(JsiIntegrationHermesTest, AllHermesVariants);

////////////////////////////////////////////////////////////////////////////////
template <typename EngineAdapter>
using JsiIntegrationHermesLegacyTest =
JsiIntegrationPortableTest<EngineAdapter>;
TYPED_TEST_SUITE(JsiIntegrationHermesLegacyTest, LegacyHermesVariants);

#pragma region JsiIntegrationPortableTest

TYPED_TEST(JsiIntegrationPortableTest, ConnectWithoutCrashing) {
this->connect();
Expand Down Expand Up @@ -458,7 +472,8 @@ TYPED_TEST(JsiIntegrationPortableTest, ExceptionDuringAddBindingIsIgnored) {
EXPECT_TRUE(this->eval("globalThis.foo === 42").getBool());
}

////////////////////////////////////////////////////////////////////////////////
#pragma endregion
#pragma region JsiIntegrationHermesTest

TYPED_TEST(JsiIntegrationHermesTest, EvaluateExpression) {
this->connect();
Expand All @@ -479,7 +494,11 @@ TYPED_TEST(JsiIntegrationHermesTest, EvaluateExpression) {
})");
}

TYPED_TEST(JsiIntegrationHermesTest, EvaluateExpressionInExecutionContext) {
// TODO(T181299386): Restore stale execution context validation under
// HermesRuntimeAgentDelegateNew
TYPED_TEST(
JsiIntegrationHermesLegacyTest,
EvaluateExpressionInExecutionContext) {
this->connect();

InSequence s;
Expand Down Expand Up @@ -538,7 +557,9 @@ TYPED_TEST(JsiIntegrationHermesTest, EvaluateExpressionInExecutionContext) {
std::to_string(executionContextId)));
}

TYPED_TEST(JsiIntegrationHermesTest, ResolveBreakpointAfterReload) {
// TODO(T178858701): Restore breakpoint reload persistence under
// HermesRuntimeAgentDelegateNew
TYPED_TEST(JsiIntegrationHermesLegacyTest, ResolveBreakpointAfterReload) {
this->connect();

InSequence s;
Expand Down Expand Up @@ -580,4 +601,6 @@ TYPED_TEST(JsiIntegrationHermesTest, ResolveBreakpointAfterReload) {
scriptInfo->value()["params"]["scriptId"]);
}

#pragma endregion

} // namespace facebook::react::jsinspector_modern
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ JsiIntegrationTestGenericEngineAdapter::JsiIntegrationTestGenericEngineAdapter(
runtimeTargetDelegate_{
"Generic engine (" + runtime_->description() + ")"} {}

/* static */ InspectorFlagOverrides
JsiIntegrationTestGenericEngineAdapter::getInspectorFlagOverrides() noexcept {
return {.enableModernCDPRegistry = true};
}

RuntimeTargetDelegate&
JsiIntegrationTestGenericEngineAdapter::getRuntimeTargetDelegate() {
return runtimeTargetDelegate_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

#pragma once

#include "../utils/InspectorFlagOverridesGuard.h"

#include <jsinspector-modern/FallbackRuntimeTargetDelegate.h>
#include <jsinspector-modern/RuntimeTarget.h>

Expand All @@ -26,6 +28,8 @@ class JsiIntegrationTestGenericEngineAdapter {
public:
explicit JsiIntegrationTestGenericEngineAdapter(folly::Executor& jsExecutor);

static InspectorFlagOverrides getInspectorFlagOverrides() noexcept;

RuntimeTargetDelegate& getRuntimeTargetDelegate();

jsi::Runtime& getRuntime() const noexcept;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,10 @@
* LICENSE file in the root directory of this source tree.
*/

#include <folly/executors/QueuedImmediateExecutor.h>

#include "JsiIntegrationTestHermesEngineAdapter.h"
#include "../utils/InspectorFlagOverridesGuard.h"

#include <folly/executors/QueuedImmediateExecutor.h>

using facebook::hermes::makeHermesRuntime;

Expand All @@ -19,6 +20,11 @@ JsiIntegrationTestHermesEngineAdapter::JsiIntegrationTestHermesEngineAdapter(
jsExecutor_{jsExecutor},
runtimeTargetDelegate_{runtime_} {}

/* static */ InspectorFlagOverrides
JsiIntegrationTestHermesEngineAdapter::getInspectorFlagOverrides() noexcept {
return {.enableModernCDPRegistry = true};
}

RuntimeTargetDelegate&
JsiIntegrationTestHermesEngineAdapter::getRuntimeTargetDelegate() {
return runtimeTargetDelegate_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

#pragma once

#include "../utils/InspectorFlagOverridesGuard.h"

#include <jsinspector-modern/RuntimeTarget.h>

#include <folly/executors/QueuedImmediateExecutor.h>
Expand All @@ -26,6 +28,8 @@ class JsiIntegrationTestHermesEngineAdapter {
public:
explicit JsiIntegrationTestHermesEngineAdapter(folly::Executor& jsExecutor);

static InspectorFlagOverrides getInspectorFlagOverrides() noexcept;

RuntimeTargetDelegate& getRuntimeTargetDelegate();

jsi::Runtime& getRuntime() const noexcept;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
* 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 "JsiIntegrationTestHermesWithCDPAgentEngineAdapter.h"

namespace facebook::react::jsinspector_modern {

JsiIntegrationTestHermesWithCDPAgentEngineAdapter::
JsiIntegrationTestHermesWithCDPAgentEngineAdapter(
folly::Executor& jsExecutor)
: JsiIntegrationTestHermesEngineAdapter(jsExecutor) {}

/* static */ InspectorFlagOverrides
JsiIntegrationTestHermesWithCDPAgentEngineAdapter::
getInspectorFlagOverrides() noexcept {
return {
.enableHermesCDPAgent = true,
.enableModernCDPRegistry = true,
};
}

} // namespace facebook::react::jsinspector_modern
Original file line number Diff line number Diff line change
@@ -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.
*/

#pragma once

#include "../utils/InspectorFlagOverridesGuard.h"
#include "JsiIntegrationTestHermesEngineAdapter.h"

namespace facebook::react::jsinspector_modern {

/**
* An engine adapter for JsiIntegrationTest that uses Hermes (and Hermes's
* new CDPAgent API).
*/
class JsiIntegrationTestHermesWithCDPAgentEngineAdapter
: public JsiIntegrationTestHermesEngineAdapter {
public:
explicit JsiIntegrationTestHermesWithCDPAgentEngineAdapter(
folly::Executor& jsExecutor);

static InspectorFlagOverrides getInspectorFlagOverrides() noexcept;
};

} // namespace facebook::react::jsinspector_modern
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ class ReactNativeFeatureFlagsOverrides
return overrides_.enableCxxInspectorPackagerConnection;
}

bool inspectorEnableHermesCDPAgent() override {
return overrides_.enableHermesCDPAgent;
}

bool inspectorEnableModernCDPRegistry() override {
return overrides_.enableModernCDPRegistry;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ struct InspectorFlagOverrides {
// NOTE: Keep these entries in sync with ReactNativeFeatureFlagsOverrides in
// the implementation file.
bool enableCxxInspectorPackagerConnection = false;
bool enableHermesCDPAgent = false;
bool enableModernCDPRegistry = false;
};

Expand Down

0 comments on commit a7fde7c

Please sign in to comment.