Skip to content

Commit

Permalink
Re-enable stale execution context test under new Hermes CDP Agent
Browse files Browse the repository at this point in the history
Summary:
Re-enable a test now that D54649943 added correct handling of stale execution context IDs. 

NOTE: There's a minor difference in returned error code. The test now allows either, though note that the old implementation is more consistent with Chrome itself which returns -32000 (server error). Semantically -32600 (invalid request) seems more appropriate.

Changelog: [Internal]

Differential Revision: D54805777
  • Loading branch information
robhogan authored and facebook-github-bot committed Mar 12, 2024
1 parent e2c5a36 commit 75f5905
Showing 1 changed file with 9 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -494,11 +494,7 @@ TYPED_TEST(JsiIntegrationHermesTest, EvaluateExpression) {
})");
}

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

InSequence s;
Expand Down Expand Up @@ -546,8 +542,14 @@ TYPED_TEST(
this->reload();

// Now the old execution context is stale.
this->expectMessageFromPage(
JsonParsed(AllOf(AtJsonPtr("/id", 3), AtJsonPtr("/error/code", -32000))));
this->expectMessageFromPage(JsonParsed(AllOf(
AtJsonPtr("/id", 3),
AtJsonPtr(
"/error/code",
// HermesRuntimeAgentDelegateNew responds more correctly with -32600
// (invalid request), old responds -32000 (server error). Accept
// either.
AnyOf(-32600, -32000)))));
this->toPage_->sendMessage(sformat(
R"({{
"id": 3,
Expand Down

0 comments on commit 75f5905

Please sign in to comment.