Skip to content

Commit

Permalink
Update experimental from latest main (#10590)
Browse files Browse the repository at this point in the history
* Jump-to-source falls back to parent user event if marker frame cannot be found (#10561)

### [Loom overview](https://www.loom.com/share/6d697c9136274fb8892325d39283fdac)

Looking at [go/r/bb82f5c3-af5c-4e20-9f1c-5efa4ee6bd29](https://app.replay.io/recording/appreplayio--bb82f5c3-af5c-4e20-9f1c-5efa4ee6bd29), I observed that the `getCypressMarkerFrame` lookup fails to find a "marker frame" because there are only two sources in the call stack– the Replay plugin itself and Cypress. That method expects to find user code in between for some reason, so this breaks the jump-to-source behavior.

In this case, the source is:
```js
cy.contains('Product added to cart!').should('be.visible');
```

That gets rendered in the UI as this:

![Screenshot 2024-06-13 at 10 46 16 PM](https://github.com/replayio/devtools/assets/29597/63920c23-82c8-45b4-9bb9-29443984b3b9)

Clicking on the failed _assert_ (or trying to "jump to source") would ideally jump to the `should('be.visible')` but it fails because of the "marker frame" issue I mentioned above. The idea occurred to me that in this case, jumping to the parent command (`contains('Product added to cart!')`) is probably better than nothing. (In this case, they're even on the same line, but that won't always be true.)

* Add data-private attributes (#10562)

* Remove [data-private] CSS style (#10563)

* Reset selected test event when test changes (#10565)

* Improve visual style for React/Reduxt not-loaded error messages (#10566)

* Log point panel should never show 1/0 (#10568)

* Fix video toggle for horizontal DEV layout (#10569)

* Support multi-line print statements (#10570)

Support multi-line print statements to improve readability and text editing experience.

* Stop mousedown event propagation in the Focuser (#10575)

* Adjusting contrast (#10572)

* Always use hoverTime for the screenshot when the user is hovering (#10578)

* Fix detection of async parent pauses outside of the focus window (#10573)

* Revert "Improve message shown when async stack can't be loaded (#10557)"

This reverts commit 585e560.

* Fix detection of async parent pauses outside of the focus window

* Remove LogRocket from Devtools (#10577)

* Improve heuristics for finding the user frame that dispatched a Redux… (#10576)

* Improve heuristics for finding the user frame that dispatched a Redux action

The original logic was iterating backwards across the point stack frames looking for an applyMiddleware frame, and going backwards one more frame from there. This is problematic because I'm seeing variance in whether or not applyMiddleware actually shows up in stack frames. For some reason it shows up in Metabase recording stack traces, but not in our own E2E tests (and we're both using RTK).  That meant that the existing logic could keep right on walking backwards long past the frame it "should" have found.

What I've settled on is using the logic we had that tries to identify "is this function in a Redux middleware" based on source outlines (looking for the triply-nested function signature of a middleware definition), and stopping at the first frame that appears to be not a middleware and not applyMiddleware.  This seems to be producing pretty reasonable results.

* Update to a recent `breakpoints-01` recording

* Force re-running the J2C routine

* Add a util for verifying J2C behavior

* Add a test for Redux J2C behavior

* Ignore wrong lint error

* Update RDT-02 test to match current example recording

* Log point panel saves pending edits when removing condition (#10579)

* Add option to notify user when processing is complete (#10582)

* Guard against undefined value (#10581)

* Source viewer font-size fixes (#10580)

Print statement panel dynamic sizing bug fixes:
- Fixed minor font-size and line-height inconsistencies for print statement panel
- Handle edge case with empty last line (aka Shift+Enter)

![Screenshot 2024-06-21 at 10 58 36 AM](https://github.com/replayio/devtools/assets/29597/1a8e9514-1c56-4998-84e0-7b71a1d022b7)

Also updated the `useSourceListCssVariables` hook to reevaluate CSS variables (width of line numbers and hit counts) when the font size preference changes.

* Better view element icon (#10583)

* Enable asserts (#10584)

* Handle lack of `point.frame` (#10586)

* Remove unused `jumpToClickEventFunctionLocation` (#10585)

* Improve video cursor display, especially for Cypress tests (#10587)

* Extract utils to check for user click/keyboard test steps

* Simplify UserActionEventRow J2C checks

* Use mouse event times for hover display for click steps

* Narrow time that we show a mouse click indicator

* Only show mouse clicks within the current focus window

* Revert focus window filtering for mouse cursors

* Update the Cypress steps test to check for hover cursor position (#10588)

* Update cypress/bankaccounts.spec example

* Add test for Cypress cursor positioning

* Use dashed/camel-cased data names

---------

Co-authored-by: Brian Vaughn <[email protected]>
Co-authored-by: Holger Benl <[email protected]>
Co-authored-by: Jon Bell <[email protected]>
Co-authored-by: Domi <[email protected]>
Co-authored-by: Mateusz Burzyński <[email protected]>
  • Loading branch information
6 people authored Jun 28, 2024
1 parent db9b68c commit 73f72b9
Show file tree
Hide file tree
Showing 80 changed files with 1,323 additions and 1,067 deletions.
2 changes: 0 additions & 2 deletions docs/codebase-notes/replay-preferences-implementations.md
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,6 @@
export type ExperimentalUserSettings = {
apiKeys: ApiKey[];
defaultWorkspaceId: null | string;
disableLogRocket: boolean;
enableTeams: boolean;
enableLargeText: boolean;
};
Expand Down Expand Up @@ -164,7 +163,6 @@ export type CombinedExperimentalUserSettings = ExperimentalUserSettings &

- `ui/actions/session.ts`: `disableCache`, `listenForMetrics`
- `ui/components/DevTools.tsx`: `sidePanelSize`
- `ui/components/Redacted.tsx`: `showRedactions`
- `ui/components/SkeletonLoader.tsx`: `sidePanelSize`
- `ui/components/Viewer.tsx`: `secondaryPanelHeight`, `sidePanelSize`, `toolboxSize`
- `ui/hooks/settings.ts`: all, passed as key to `useStringPref` and `useBoolPref`
Expand Down
3 changes: 0 additions & 3 deletions package.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 8 additions & 4 deletions packages/e2e-tests/examples.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 15 additions & 0 deletions packages/e2e-tests/helpers/pause-information-panel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,21 @@ export async function stepOver(page: Page): Promise<void> {
await clickCommandBarButton(page, "Step Over");
}

export function waitForAllFramesToLoad(page: Page) {
return waitFor(async () => {
expect(await page.locator('[data-test-name="FramesLoading"]').count()).toBe(0);
});
}

export function getAsyncParentCount(page: Page) {
return page.locator('[data-test-name="AsyncParentLabel"]').count();
}

export async function isAsyncParentUnavailable(page: Page) {
const asyncParentUnavailable = page.locator('[data-test-name="AsyncParentUnavailable"]');
return (await asyncParentUnavailable.count()) > 0;
}

export async function verifyFramesCount(page: Page, expectedCount: number) {
const framesPanel = getFramesPanel(page);
return waitFor(async () => {
Expand Down
29 changes: 29 additions & 0 deletions packages/e2e-tests/helpers/source-panel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,35 @@ export async function getSelectedLineNumber(
return parseInt(textContent, 10);
}

export async function verifyJumpToCodeResults(
page: Page,
filename: string,
lineNumber: number,
expectedHits?: { current: number; total: number }
) {
await waitForSelectedSource(page, filename);
// Should highlight the line that ran
await waitFor(async () => {
const lineNumber = await getSelectedLineNumber(page, true);
expect(lineNumber).toBe(lineNumber);
});

if (expectedHits) {
// Should also have jumped in time. Since this can vary (slightly different progress %
// based on timing differences), we'll add a log statement and verify _which_ hit we're at.
await addLogpoint(page, {
url: filename,
lineNumber,
});

const { current, total } = expectedHits;

// Should have paused on the handler for the first valid keystroke
await verifyLogpointStep(page, `${current}/${total}`, { url: filename, lineNumber });
await removeLogPoint(page, { url: filename, lineNumber });
}
}

export function getSourceLocator(page: Page, sourceId: string): Locator {
return page.locator(getSourceSelector(sourceId));
}
Expand Down
1 change: 1 addition & 0 deletions packages/e2e-tests/scripts/buildkite_run_fe_tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,7 @@ export default async function run_fe_tests(
}
// process.env.RECORD_REPLAY_DIRECTORY =

process.env.REPLAY_ENABLE_ASSERTS = process.env.RECORD_REPLAY_ENABLE_ASSERTS = "1";
process.env.HASURA_ADMIN_SECRET ||= getSecret("prod/hasura-admin-secret", "us-east-2");
process.env.DISPATCH_ADDRESS ||= "wss://dispatch.replay.io";
process.env.AUTHENTICATED_TESTS_WORKSPACE_API_KEY = process.env.RECORD_REPLAY_API_KEY;
Expand Down
34 changes: 34 additions & 0 deletions packages/e2e-tests/tests/async-stack.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import { openDevToolsTab, startTest } from "../helpers";
import { warpToMessage } from "../helpers/console-panel";
import {
getAsyncParentCount,
isAsyncParentUnavailable,
waitForAllFramesToLoad,
} from "../helpers/pause-information-panel";
import { setFocusRange } from "../helpers/timeline";
import test, { expect } from "../testFixture";

test.use({ exampleKey: "doc_async_stack.html" });

test(`async-stack: should detect async stacks outside the focus window`, async ({
pageWithMeta: { page, recordingId, testScope },
exampleKey,
}) => {
await startTest(page, recordingId, testScope);
await openDevToolsTab(page);

await warpToMessage(page, "Starting", 7);
await waitForAllFramesToLoad(page);
expect(await getAsyncParentCount(page)).toBe(0);
expect(await isAsyncParentUnavailable(page)).toBe(false);

await warpToMessage(page, "ExampleFinished", 9);
await waitForAllFramesToLoad(page);
expect(await getAsyncParentCount(page)).toBe(1);
expect(await isAsyncParentUnavailable(page)).toBe(false);

await setFocusRange(page, { startTimeString: "00:01" });
await waitForAllFramesToLoad(page);
expect(await getAsyncParentCount(page)).toBe(1);
expect(await isAsyncParentUnavailable(page)).toBe(true);
});
44 changes: 43 additions & 1 deletion packages/e2e-tests/tests/cypress-05_hover-dom-previews.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
getTestSuitePanel,
openCypressTestPanel,
} from "../helpers/testsuites";
import { debugPrint, waitFor } from "../helpers/utils";
import { debugPrint, getByTestName, waitFor } from "../helpers/utils";
import test, { expect } from "../testFixture";

test.use({ exampleKey: "cypress-realworld/bankaccounts.spec.js" });
Expand Down Expand Up @@ -79,13 +79,55 @@ test("cypress-05: Test DOM node preview on user action step hover", async ({
await waitFor(async () =>
expect(await firstClickStep.getAttribute("data-selected")).toBe("true")
);

debugPrint(page, "Checking recorded cursor location for a click");
const recordedCursor = getByTestName(page, "recorded-cursor");

function getCursorAttributes(node: HTMLElement) {
return {
cursorDisplay: node.dataset.cursorDisplay,
clickDisplay: node.dataset.clickDisplay,
clientX: node.dataset.clientX,
clientY: node.dataset.clientY,
};
}
const clickCursorAttributes = await recordedCursor.evaluate(getCursorAttributes);

expect(clickCursorAttributes).toEqual({
cursorDisplay: "true",
clickDisplay: "true",
// Read directly from the mouse event in this test
clientX: "323",
clientY: "245",
});

// Make the highlighter go away
await firstStep.hover();
await highlighter.waitFor({ state: "hidden" });
// Hover over the selected `firstClickStep` and verify that the highlighter is shown again
await firstClickStep.hover();
await highlighter.waitFor({ state: "visible" });

debugPrint(page, "Checking recorded cursor location after a click has finished");

const openedBankAccountsStep = steps
.filter({
hasText: "Opened http://localhost:3000/bankaccounts",
})
.first();
await openedBankAccountsStep.hover();

const afterClickCursorAttributes = await recordedCursor.evaluate(getCursorAttributes);

expect(afterClickCursorAttributes).toEqual({
cursorDisplay: "true",
// No click display after the click has finished
clickDisplay: "false",
// Read directly from the mouse event in this test
clientX: "323",
clientY: "245",
});

debugPrint(page, "Checking highlighting for multiple nodes");

// Should also handle multiple found DOM nodes
Expand Down
38 changes: 5 additions & 33 deletions packages/e2e-tests/tests/jump-to-code-01_basic.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { openSourceExplorerPanel } from "../helpers/source-explorer-panel";
import {
addLogpoint,
getSelectedLineNumber,
verifyJumpToCodeResults,
verifyLogpointStep,
waitForSelectedSource,
} from "../helpers/source-panel";
Expand Down Expand Up @@ -71,7 +72,7 @@ test(`jump-to-code-01: Test basic jumping functionality`, async ({
const queryParams = new URLSearchParams();
// Force this test to always re-run the Event Listeners (and other) routines
// See pref names in packages/shared/user-data/GraphQL/config.ts
// queryParams.set("features", "backend_rerunRoutines");
queryParams.set("features", "backend_rerunRoutines");

await startTest(page, recordingId, testScope, undefined, queryParams);
await openDevToolsTab(page);
Expand Down Expand Up @@ -140,22 +141,11 @@ test(`jump-to-code-01: Test basic jumping functionality`, async ({

debugPrint(page, "Checking that the first keypress J2C jumps to the correct line");
await firstValidKeypressJumpButton.click();
await waitForSelectedSource(page, "Header.tsx");
// Should highlight the line that ran
await waitFor(async () => {
const lineNumber = await getSelectedLineNumber(page, true);
expect(lineNumber).toBe(12);
});

// Should have paused on the handler for the first valid keystroke.
// Should also have jumped in time. Since this can vary (slightly different progress %
// based on timing differences), we'll add a log statement and verify _which_ hit we're at.
await addLogpoint(page, {
url: "Header.tsx",
lineNumber: 12,
});

// Should have paused on the handler for the first valid keystroke
await verifyLogpointStep(page, "1/22", { url: "Header.tsx", lineNumber: 12 });
await verifyJumpToCodeResults(page, "Header.tsx", 12, { current: 1, total: 22 });

// the next clicks were on real buttons, so there is a handler
debugPrint(page, "Checking for an enabled click 'Jump' button");
Expand All @@ -164,24 +154,6 @@ test(`jump-to-code-01: Test basic jumping functionality`, async ({

debugPrint(page, "Checking that the first click J2C jumps to the correct line");
await firstValidClickJumpButton.click();
await waitForSelectedSource(page, "TodoListItem.tsx");
// Should highlight the line that ran
await waitFor(async () => {
const lineNumber = await getSelectedLineNumber(page, true);
expect(lineNumber).toBe(22);
});

// Should also have jumped in time
// Should also have jumped in time. Since this can vary (slightly different progress %
// based on timing differences), we'll add a log statement and verify _which_ hit we're at.
await addLogpoint(page, {
url: "TodoListItem.tsx",
lineNumber: 22,
});

// Should have paused on the handler for the first valid click
await verifyLogpointStep(page, "1/2", {
url: "TodoListItem.tsx",
lineNumber: 22,
});
await verifyJumpToCodeResults(page, "TodoListItem.tsx", 22, { current: 1, total: 2 });
});
97 changes: 97 additions & 0 deletions packages/e2e-tests/tests/jump-to-code-02_redux-j2c.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
import { Locator, Page, expect } from "@playwright/test";

import { openDevToolsTab, startTest } from "../helpers";
import { getEventJumpButton } from "../helpers/info-event-panel";
import { getReduxActions, openReduxDevtoolsPanel } from "../helpers/redux-devtools-panel";
import { closeSource, verifyJumpToCodeResults } from "../helpers/source-panel";
import { getByTestName, waitFor } from "../helpers/utils";
import test from "../testFixture";

// trunk-ignore(gitleaks/generic-api-key)
test.use({ exampleKey: "breakpoints-01" });

async function checkForJumpButton(actionListItem: Locator, shouldBeEnabled: boolean) {
const jumpButton = getEventJumpButton(actionListItem);
expect(await jumpButton.isVisible()).toBe(true);
await jumpButton.hover();

await waitFor(async () => {
const buttonText = await getByTestName(jumpButton, "JumpToCodeButtonLabel").innerText();
const expectedText = shouldBeEnabled ? "Jump to code" : "No results";
expect(buttonText).toBe(expectedText);
});

return jumpButton;
}

async function clickReduxActionJumpButton(page: Page, actionListItem: Locator) {
await actionListItem.scrollIntoViewIfNeeded();
await actionListItem.hover();
const jumpButton = await checkForJumpButton(actionListItem, true);
await jumpButton.click();
}

async function jumpToReduxDispatch(page: Page, actionType: string, index = 0) {
const reduxListItemsLocator = getReduxActions(page);
const reduxSearchInput = page.locator("#redux-searchbox");

await reduxSearchInput.fill(actionType);
const actionListItem = reduxListItemsLocator.filter({ hasText: actionType }).nth(index);
await clickReduxActionJumpButton(page, actionListItem);
}

test(`jump-to-code-02: Redux J2C functionality`, async ({
pageWithMeta: { page, recordingId, testScope },
exampleKey,
}) => {
await startTest(page, recordingId, testScope);
await openDevToolsTab(page);

await openReduxDevtoolsPanel(page);

const reduxListItemsLocator = getReduxActions(page);

await waitFor(async () => {
const numListItems = await reduxListItemsLocator.count();
expect(numListItems).toBeGreaterThan(0);
});

// Inside of a thunk
await jumpToReduxDispatch(page, "app/setRecordingId");
await verifyJumpToCodeResults(page, "session.ts", 170, { current: 1, total: 1 });
await closeSource(page, "session.ts");

// Inside of the same thunk, after several awaits
await jumpToReduxDispatch(page, "app/setRecordingTarget");
await verifyJumpToCodeResults(page, "session.ts", 363, { current: 1, total: 1 });
await closeSource(page, "session.ts");

// Inside of one of the bootstrapping functions that receives the store
// should be "debugger/src/client/index.ts"
await jumpToReduxDispatch(page, "sources/allSourcesReceived");
await verifyJumpToCodeResults(page, "index.ts", 13, { current: 1, total: 1 });
await closeSource(page, "index.ts");

// Inside of an RTK listener middleware effect
jumpToReduxDispatch(page, "tabs/tabsRestored");
await verifyJumpToCodeResults(page, "newSources.ts", 43, { current: 1, total: 1 });
await closeSource(page, "newSources.ts");

// Inside of a `useEffect`
jumpToReduxDispatch(page, "set_selected_primary_panel");
await verifyJumpToCodeResults(page, "SidePanel.tsx", 57, { current: 1, total: 1 });
await closeSource(page, "SidePanel.tsx");

// Inside of a `connect()`ed class component, with `this.props.setExpandedState()`.
// Note that this appears to be one or two execution ticks off, so the line hit won't
// line up perfectly, but it should still _display_ as "1/4"
jumpToReduxDispatch(page, "SET_EXPANDED_STATE");
await verifyJumpToCodeResults(page, "SourcesTree.tsx", 196, { current: 1, total: 4 });
await closeSource(page, "SourcesTree.tsx");

// Inside of an adapter that passes dispatch-wrapped actions to <QuickOpenModal>
// This is also one tick off, but should still _display_ as "1/3"
jumpToReduxDispatch(page, "quickOpen/setQuickOpenQuery");
await verifyJumpToCodeResults(page, "QuickOpenModal.tsx", 551, { current: 1, total: 3 });
await closeSource(page, "QuickOpenModal.tsx");
});
Loading

0 comments on commit 73f72b9

Please sign in to comment.