Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore replay-connect callback event #297

Merged

Conversation

ryanjduffy
Copy link
Contributor

Issue

The plugin is adding then commands from our replay-connect callback to the beforeAll steps for each test. This wasn't currently visible to users because we aren't showing beforeAll and afterAll yet but it was introducing some confusion in troubleshooting SCS-1642 because there were events in places we didn't expect them to be.

Resolution

Suppress our handleReplayConnectResponse handler called by then from being added to steps and from creating annotations.

Copy link
Contributor

@jasonLaster jasonLaster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left some nits, but otherwise good

@@ -54,11 +54,27 @@ let gPluginServer: WebSocket | undefined;
// user-facing
const COMMAND_IGNORE_LIST = ["log-restore", "within-restore", "end-logGroup"];

function handleReplayConnectResponse(v: unknown) {
if (v && typeof v === "object" && "port" in v && typeof v.port === "number") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

purely a nit, but something like this is a bit cleaner for me

if (typeof v?.port !== "number") {
  cy.log("[replay.io] Received unexpected response when connecting to plugin");
  return;
}
gServerPort = v.port;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is cleaner but it doesn't pass typechecking since v is unknown so more narrowing is required.

function shouldIgnoreCommand(cmd: Cypress.EnqueuedCommand | Cypress.CommandQueue) {
if (isCommandQueue(cmd)) {
cmd = cmd.toJSON() as any as Cypress.EnqueuedCommand;
}

if (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it'd help to have a comment here

@ryanjduffy ryanjduffy merged commit 099f194 into main Dec 29, 2023
3 of 4 checks passed
@ryanjduffy ryanjduffy deleted the ryan/scs-1642-metabase-test-metadata-could-not-be-processed branch December 29, 2023 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants