Remove breakpoints (#10604)

Follow on to #10601

- Delete breakpoint UI and protocol-related code
- Remove unnecessary abstractions (now that there aren't two types of "points" in the app)
- Rename Nag/Tour related code that had potentially confusing references to breakpoints
- Update any e2e tests that got broken by this change

Author: bvaughn

packages/e2e-tests/authenticated/logpoints-01.test.ts

@@ -64,7 +64,7 @@ test(`authenticated/logpoints-01: Shared logpoints functionality`, async ({
     const page = await context.newPage();
     await load(page, recordingId, testUsers![1].apiKey, testScope);
 
-    const locator = await findPoints(page, "logpoint", { lineNumber });
+    const locator = await findPoints(page, { lineNumber });
 
     // Verify point is not editable
     await expect(await isPointEditable(locator)).toBe(false);

packages/e2e-tests/helpers/pause-information-panel.ts

@@ -18,10 +18,6 @@ async function toggleAccordionPane(page: Page, pane: Locator, targetState: "open
   }
 }
 
-export async function closeBreakpointsAccordionPane(page: Page): Promise<void> {
-  await toggleAccordionPane(page, getBreakpointsAccordionPane(page), "closed");
-}
-
 export async function closeCallStackAccordionPane(page: Page): Promise<void> {
   await toggleAccordionPane(page, getCallStackAccordionPane(page), "closed");
 }
@@ -49,7 +45,7 @@ export async function expandAllScopesBlocks(page: Page): Promise<void> {
 
 export async function closeSidePanel(page: Page) {
   // Ensure that it's closed by forcing the "Pause" pane to open instead...
-  const pane = getBreakpointsAccordionPane(page);
+  const pane = getPrintStatementsAccordionPane(page);
   const pauseButton = page.locator('[data-test-name="ToolbarButton-PauseInformation"]');
   await pauseButton.click();
   const isVisible = await pane.isVisible();
@@ -58,10 +54,6 @@ export async function closeSidePanel(page: Page) {
   }
 }
 
-export function getBreakpointsAccordionPane(page: Page): Locator {
-  return page.locator('[data-test-id="AccordionPane-Breakpoints"]');
-}
-
 export function getCallStackAccordionPane(page: Page): Locator {
   return page.locator('[data-test-id="AccordionPane-CallStack"]');
 }
@@ -110,17 +102,13 @@ export function getScopesPanel(page: Page): Locator {
   return page.locator('[data-test-name="ScopesList"]');
 }
 
-export async function openBreakpointsAccordionPane(page: Page): Promise<void> {
-  await toggleAccordionPane(page, getBreakpointsAccordionPane(page), "open");
-}
-
 export async function openCallStackPane(page: Page): Promise<void> {
   await toggleAccordionPane(page, getCallStackAccordionPane(page), "open");
 }
 
 export async function openPauseInformationPanel(page: Page): Promise<void> {
   // Only click if it's not already open; clicking again will collapse the side bar.
-  const pane = getBreakpointsAccordionPane(page);
+  const pane = getPrintStatementsAccordionPane(page);
 
   let isVisible = await pane.isVisible();
   if (!isVisible) {
@@ -344,7 +332,6 @@ export async function waitForScopeValue(
 
 export function findPoints(
   page: Page,
-  type: "logpoint" | "breakpoint",
   options: {
     sourceId?: SourceId;
     lineNumber?: number;
@@ -354,8 +341,7 @@ export function findPoints(
   const { columnIndex, lineNumber, sourceId } = options;
 
   const selectorCriteria = [
-    '[data-test-name="Breakpoint"]',
-    `[data-test-type="${type}"]`,
+    '[data-test-name="LogPoint"]',
     columnIndex != null ? `[data-test-column-index="${columnIndex}"]` : "",
     lineNumber != null ? `[data-test-line-number="${lineNumber}"]` : "",
     sourceId != null ? `[data-test-source-id="${sourceId}"]` : "",
@@ -365,17 +351,17 @@ export function findPoints(
 }
 
 export async function isPointEditable(pointLocator: Locator) {
-  const editButton = pointLocator.locator('[data-test-name="RemoveBreakpointButton"]');
+  const editButton = pointLocator.locator('[data-test-name="RemoveLogPointButton"]');
   return (await editButton.count()) > 0;
 }
 
 export async function removePoint(pointLocator: Locator) {
-  await pointLocator.locator('[data-test-name="RemoveBreakpointButton"]').first().click();
+  await pointLocator.locator('[data-test-name="RemoveLogPointButton"]').first().click();
 }
 
 export async function togglePoint(page: Page, pointLocator: Locator, enabled: boolean) {
   const targetState = enabled ? POINT_BEHAVIOR_ENABLED : POINT_BEHAVIOR_DISABLED_TEMPORARILY;
-  const toggle = pointLocator.locator('[data-test-name="BreakpointToggle"]');
+  const toggle = pointLocator.locator('[data-test-name="LogPointToggle"]');
   const currentState = await toggle.getAttribute("date-test-state");
   if (targetState !== currentState) {
     await debugPrint(page, `Toggling point to ${targetState}`, "togglePoint");

packages/e2e-tests/helpers/source-panel.ts

@@ -3,74 +3,12 @@ import chalk from "chalk";
 
 import { Badge } from "shared/client/types";
 
-import { clearText, focus, hideTypeAheadSuggestions, typeLogPoint as typeLexical } from "./lexical";
+import { focus, hideTypeAheadSuggestions, typeLogPoint as typeLexical } from "./lexical";
 import { findPoints, openPauseInformationPanel, removePoint } from "./pause-information-panel";
 import { openSource } from "./source-explorer-panel";
-import { clearTextArea, debugPrint, delay, getByTestName, getCommandKey, waitFor } from "./utils";
+import { clearTextArea, debugPrint, getByTestName, getCommandKey, waitFor } from "./utils";
 import { openDevToolsTab } from ".";
 
-export async function addBreakpoint(
-  page: Page,
-  options: {
-    columnIndex?: number;
-    lineNumber: number;
-    url: string;
-  }
-): Promise<void> {
-  const { lineNumber, url } = options;
-
-  await openDevToolsTab(page);
-
-  if (url) {
-    await openSource(page, url);
-  }
-
-  await debugPrint(
-    page,
-    `Adding breakpoint at ${chalk.bold(`${url}:${lineNumber}`)}`,
-    "addBreakpoint"
-  );
-
-  await scrollUntilLineIsVisible(page, lineNumber);
-  await waitForSourceLineHitCounts(page, lineNumber);
-
-  const lineLocator = await getSourceLine(page, lineNumber);
-
-  // Account for the fact that SourceListRow doesn't render SourceListRowMouseEvents while scrolling
-  await waitFor(async () => {
-    const isScrolling = await lineLocator.getAttribute("data-test-is-scrolling");
-    expect(isScrolling).toBe(null);
-  });
-
-  const lineNumberLocator = lineLocator.locator('[data-test-name="SourceLine-LineNumber"]');
-  await lineNumberLocator.waitFor();
-
-  const breakpointToggleLocator = lineLocator.locator('[data-test-name="BreakpointToggle"]');
-
-  await waitFor(async () => {
-    // Mouse out then back over
-    await page.mouse.move(0, 0);
-    await lineNumberLocator.hover({ force: true });
-
-    // Wait for breakpoint toggle from mouse over
-    await breakpointToggleLocator.waitFor();
-
-    const state = await breakpointToggleLocator.getAttribute("data-test-state");
-    if (state === "off") {
-      await lineLocator.locator('[data-test-name="SourceLine-LineNumber"]').click({ force: true });
-    }
-  });
-
-  await waitForBreakpoint(page, options);
-
-  // We want to add a slight delay after adding a breakpoint so that the
-  // breakpoint logic will have time to send protocol commands to the server,
-  // since that is not guaranteed to happen synchronously on click. This is
-  // important for cases where we add a breakpoint and then immediately
-  // attempt to step to the breakpoint location.
-  await delay(2000);
-}
-
 export async function editBadge(
   page: Page,
   options: {
@@ -514,69 +452,18 @@ export async function openLogPointPanelContextMenu(
   }
 }
 
-export async function removeAllBreakpoints(page: Page): Promise<void> {
-  await debugPrint(page, `Removing all breakpoints for the current source`, "removeBreakpoint");
-
-  await openPauseInformationPanel(page);
-  const points = await findPoints(page, "breakpoint");
-  const count = await points.count();
-  for (let index = count - 1; index >= 0; index--) {
-    const point = points.nth(index);
-    await removePoint(point);
-  }
-}
-
 export async function removeAllLogpoints(page: Page): Promise<void> {
-  await debugPrint(page, `Removing all breakpoints for the current source`, "removeBreakpoint");
+  await debugPrint(page, `Removing all logpoints for the current source`, "removeAllLogpoints");
 
   await openPauseInformationPanel(page);
-  const points = await findPoints(page, "logpoint");
+  const points = await findPoints(page);
   const count = await points.count();
   for (let index = count - 1; index >= 0; index--) {
     const point = points.nth(index);
     await removePoint(point);
   }
 }
 
-export async function removeBreakpoint(
-  page: Page,
-  options: {
-    lineNumber: number;
-    url: string;
-  }
-): Promise<void> {
-  const { lineNumber, url } = options;
-
-  await debugPrint(
-    page,
-    `Removing breakpoint at ${chalk.bold(`${url}:${lineNumber}`)}`,
-    "removeBreakpoint"
-  );
-
-  await openDevToolsTab(page);
-
-  if (url) {
-    await openSource(page, url);
-  }
-
-  const lineLocator = await getSourceLine(page, lineNumber);
-  const numberLocator = lineLocator.locator(`[data-test-name="SourceLine-LineNumber"]`);
-  await numberLocator.hover({ force: true });
-  const state = await lineLocator
-    .locator('[data-test-name="BreakpointToggle"]')
-    .getAttribute("data-test-state");
-  if (state !== "off") {
-    await numberLocator.click({ force: true });
-  }
-
-  // We want to add a slight delay after removing a breakpoint so that the
-  // breakpoint logic will have time to send protocol commands to the server,
-  // since that is not guaranteed to happen synchronously on click. This is
-  // important for cases where we remove a breakpoint and then immediately
-  // attempt to step across the breakpoint location.
-  await delay(1000);
-}
-
 export async function removeConditional(
   page: Page,
   options: {
@@ -698,39 +585,6 @@ export async function toggleMappedSources(page: Page, targetState: "on" | "off")
   }
 }
 
-export async function waitForBreakpoint(
-  page: Page,
-  options: {
-    columnIndex?: number;
-    lineNumber: number;
-    url: string;
-  }
-): Promise<void> {
-  const { columnIndex, lineNumber, url } = options;
-
-  await debugPrint(
-    page,
-    `Waiting for breakpoint at ${chalk.bold(`${url}:${lineNumber}`)}`,
-    "waitForBreakpoint"
-  );
-
-  await openPauseInformationPanel(page);
-
-  const breakpointGroup = await page.waitForSelector(
-    `[data-test-name="BreakpointsList"]:has-text("${url}")`
-  );
-
-  if (columnIndex != null) {
-    await breakpointGroup.waitForSelector(
-      `[data-test-name="PointLocation"]:has-text("${lineNumber}:${columnIndex}")`
-    );
-  } else {
-    await breakpointGroup.waitForSelector(
-      `[data-test-name="PointLocation"]:has-text("${lineNumber}")`
-    );
-  }
-}
-
 export async function waitForLogpoint(
   page: Page,
   options: {
@@ -775,7 +629,7 @@ export async function verifyLogpointStep(
 
   await debugPrint(
     page,
-    `Verifying breakpoint status "${chalk.bold(expectedStatus)}" for line ${chalk.bold(
+    `Verifying logpoint status "${chalk.bold(expectedStatus)}" for line ${chalk.bold(
       options.lineNumber
     )}`,
     "verifyLogpointStep"

packages/e2e-tests/tests/logpoints-01.test.ts

@@ -8,7 +8,7 @@ import {
   verifyLogpointBadge,
 } from "../helpers/console-panel";
 import { reverseStepOverToLine } from "../helpers/pause-information-panel";
-import { addBreakpoint, addLogpoint } from "../helpers/source-panel";
+import { addLogpoint } from "../helpers/source-panel";
 import { waitFor } from "../helpers/utils";
 import test, { expect } from "../testFixture";
 
@@ -21,7 +21,6 @@ test(`logpoints-01: log-points appear in the correct order and allow time warpin
   await startTest(page, recordingId, testScope);
   await openDevToolsTab(page);
 
-  await addBreakpoint(page, { lineNumber: 20, url: exampleKey });
   await addLogpoint(page, {
     content: '"Logpoint Number " + number',
     lineNumber: 20,

packages/e2e-tests/tests/logpoints-02.test.ts

@@ -10,7 +10,6 @@ import {
   seekToPreviousLogPointHit,
   verifyLogpointStep,
 } from "../helpers/source-panel";
-import { delay } from "../helpers/utils";
 import test, { expect } from "../testFixture";
 
 test.use({ exampleKey: "doc_rr_basic.html" });
@@ -47,7 +46,7 @@ test(`logpoints-02: conditional log-points`, async ({
   await expect(logPointMessages.first()).toHaveText(/Beginning/);
   await expect(logPointMessages.last()).toHaveText(/Ending/);
 
-  const logpoint = findPoints(page, "logpoint", { lineNumber: 20 });
+  const logpoint = findPoints(page, { lineNumber: 20 });
   await togglePoint(page, logpoint, false);
   await expect(logPointMessages).toHaveCount(2);
   await togglePoint(page, logpoint, true);

packages/e2e-tests/tests/logpoints-06.test.ts

@@ -33,7 +33,7 @@ test(`logpoints-06: should be temporarily disabled`, async ({
   // Find the newly added point in the side panel
   await openPauseInformationPanel(page);
   await openPrintStatementsAccordionPane(page);
-  const logpoints = findPoints(page, "logpoint", { lineNumber });
+  const logpoints = findPoints(page, { lineNumber });
   await expect(await logpoints.count()).toBe(1);
   const logpoint = logpoints.first();
 

packages/e2e-tests/tests/node_control_flow.test.ts

@@ -1,12 +1,7 @@
 import { startTest } from "../helpers";
 import { openSource } from "../helpers/source-explorer-panel";
-import { addBreakpoint, fastForwardToLine, rewindToLine } from "../helpers/source-panel";
-import test, { Page } from "../testFixture";
-
-async function resumeToBreakpoint(page: Page, line: number) {
-  await addBreakpoint(page, { url: "control_flow.js", lineNumber: line });
-  await fastForwardToLine(page, { lineNumber: line });
-}
+import { fastForwardToLine, rewindToLine } from "../helpers/source-panel";
+import test from "../testFixture";
 
 test.use({ exampleKey: "node/control_flow.js" });
 
@@ -22,20 +17,18 @@ test("node_control_flow: catch, finally, generators, and async/await", async ({
 
   await rewindToLine(page, { lineNumber: 84 });
 
-  await resumeToBreakpoint(page, 10);
-  await resumeToBreakpoint(page, 12);
-  await resumeToBreakpoint(page, 18);
-  await resumeToBreakpoint(page, 20);
-  await resumeToBreakpoint(page, 32);
-  await resumeToBreakpoint(page, 27);
-
+  await fastForwardToLine(page, { lineNumber: 10 });
+  await fastForwardToLine(page, { lineNumber: 12 });
+  await fastForwardToLine(page, { lineNumber: 18 });
+  await fastForwardToLine(page, { lineNumber: 20 });
   await fastForwardToLine(page, { lineNumber: 32 });
   await fastForwardToLine(page, { lineNumber: 27 });
-
-  await resumeToBreakpoint(page, 42);
-  await resumeToBreakpoint(page, 44);
-  await resumeToBreakpoint(page, 50);
-  await resumeToBreakpoint(page, 54);
-  await resumeToBreakpoint(page, 65);
-  await resumeToBreakpoint(page, 72);
+  await fastForwardToLine(page, { lineNumber: 32 });
+  await fastForwardToLine(page, { lineNumber: 27 });
+  await fastForwardToLine(page, { lineNumber: 42 });
+  await fastForwardToLine(page, { lineNumber: 44 });
+  await fastForwardToLine(page, { lineNumber: 50 });
+  await fastForwardToLine(page, { lineNumber: 54 });
+  await fastForwardToLine(page, { lineNumber: 65 });
+  await fastForwardToLine(page, { lineNumber: 72 });
 });