From 15c8fc11e88a689f16c4118200da977d68bd154c Mon Sep 17 00:00:00 2001 From: Mantra Date: Wed, 6 Nov 2024 12:56:06 +0000 Subject: [PATCH 1/4] update e2e tests --- app/static/tests/e2e/code.etest.ts | 64 ++++++++++--------- .../tests/e2e/multiSensitivity.etest.ts | 4 +- app/static/tests/e2e/options.etest.ts | 29 +++++---- app/static/tests/e2e/sensitivity.etest.ts | 2 +- app/static/tests/e2e/sessions.etest.ts | 4 ++ app/static/tests/e2e/stochastic.etest.ts | 2 +- 6 files changed, 58 insertions(+), 47 deletions(-) diff --git a/app/static/tests/e2e/code.etest.ts b/app/static/tests/e2e/code.etest.ts index 3012dfde..efc65ad9 100644 --- a/app/static/tests/e2e/code.etest.ts +++ b/app/static/tests/e2e/code.etest.ts @@ -33,9 +33,9 @@ beta <- user(4) sigma <- user(2) `; -const editorGlyphs: any = { - error: "fa-solid fa-circle-xmark", - warning: "fa-solid fa-triangle-exclamation" +const editorGlyphs: Record = { + error: "fa-circle-xmark", + warning: "fa-triangle-exclamation" }; enum EditorStates { @@ -47,36 +47,40 @@ test.beforeEach(async ({ page }) => { await page.goto("/apps/day1"); }); -const expectMonacoDecoration = async (state: EditorStates, line: number, numOfLines: number, page: any) => { +const expectMonacoDecoration = async (state: EditorStates, line: number, numOfLines: number, page: Page) => { for (let i = 0; i < numOfLines; i += 1) { const lineElement = await page.locator(`.view-overlays div:nth-child(${i + 1}) >> div`); - const glyphElement = await page.locator(`.margin-view-overlays div:nth-child(${i + 1}) >> div`); if (i === line - 1) { - expect(lineElement).toHaveClass(`cdr editor-${state}-background`); - expect(glyphElement.nth(0)).toHaveClass(`cgmr codicon ${editorGlyphs[state]} ${state}-glyph-style ms-1`); - expect(glyphElement.nth(1)).toHaveClass("line-numbers lh-odd"); + await expect(lineElement).toHaveClass(`cdr editor-${state}-background`); } else if (i === numOfLines - 1) { - expect(lineElement).toHaveClass("current-line"); - expect(glyphElement.nth(0)).toHaveClass("current-line current-line-margin-both"); - expect(glyphElement.nth(1)).toHaveClass(/\bactive-line-number\b/); - expect(glyphElement.nth(1)).toHaveClass(/\bline-numbers\b/); - expect(glyphElement.nth(1)).toHaveClass(/\blh-odd\b/); + await expect(lineElement).toHaveClass("current-line"); } else { - expect(lineElement).toHaveCount(0); - expect(glyphElement).toHaveClass("line-numbers lh-odd"); + await expect(lineElement).toHaveCount(0); } } + + const lineHeight = await page.locator(".margin-view-overlays").evaluate(el => { + return window.getComputedStyle(el).getPropertyValue("line-height"); + }); + const glyphTop = await page.locator(`.${editorGlyphs[state]}`).evaluate(el => { + return window.getComputedStyle(el).getPropertyValue("top"); + }); + expect(glyphTop).toBe(`${(line - 1) * parseInt(lineHeight)}px`); }; -const expectMonacoHover = async (type: "glyph" | "content", line: number, message: string, page: any) => { +const expectMonacoHover = async (type: "glyph" | "content", line: number, message: string, page: Page) => { const isGlyph = type === "glyph"; - const hoverElem = `.${isGlyph ? "margin-" : ""}view-overlays div:nth-child(${line}) >> div`; - const tooltipElem = `${isGlyph ? ".overlayWidgets" : ".overflowingContentWidgets"} .hover-contents div p`; + const hoverElem = type === "content" ? + `.view-overlays div:nth-child(${line}) >> div` : + ".glyph-margin-widgets .fa-solid"; + // const hoverElem = `.${isGlyph ? "margin-" : ""}view-overlays div:nth-child(${line}) >> div`; + // const tooltipElem = `${isGlyph ? ".overlayWidgets" : ".overflowingContentWidgets"} .hover-contents div p`; await page.hover(hoverElem, { force: true }); - const tooltip = await page.locator(tooltipElem); - await tooltip.waitFor({ timeout: 1000 }); - await expect(tooltip).toHaveText(message); - await expect(tooltip).toHaveCSS("visibility", "visible"); + // const tooltip = await page.locator(tooltipElem); + // await tooltip.waitFor({ timeout: 2000 }); + // await expect(tooltip).toHaveText(message); + // await expect(tooltip).toHaveCSS("visibility", "visible"); + await expect(await page.getByText(message)).toBeVisible() }; test.describe("Code Tab tests", () => { @@ -128,9 +132,9 @@ test.describe("Code Tab tests", () => { test("code loading on input renders as expected", async ({ page }) => { await page.press(".monaco-editor textarea", "Control+A"); await page.press(".monaco-editor textarea", "Delete"); - page.fill(".monaco-editor textarea", "blah"); - expect(page.locator("#code-status")).toHaveClass("mt-2 code-validating-text"); - expect(page.locator("#code-status").locator("i")).toHaveClass( + await page.fill(".monaco-editor textarea", "blah"); + await expect(page.locator("#code-status")).toHaveClass("mt-2 code-validating-text"); + await expect(page.locator("#code-status").locator("i")).toHaveClass( "vue-feather vue-feather--check inline-icon me-1 code-validating-icon" ); }); @@ -355,12 +359,12 @@ test.describe("Code Tab tests", () => { test("can display help dialog", async ({ page }) => { await page.click("div.code-tab i.generic-help-icon"); - expect((await page.innerText(".draggable-dialog .dragtarget")).trim()).toBe("Write odin code"); - expect(await page.innerText(".draggable-dialog .draggable-content")).toContain("Write code in this editor"); + await expect((await page.innerText(".draggable-dialog .dragtarget")).trim()).toBe("Write odin code"); + await expect(await page.innerText(".draggable-dialog .draggable-content")).toContain("Write code in this editor"); // close dialog await page.click("i.vue-feather--x"); - expect(await page.locator(".draggable-dialog")).not.toBeVisible(); + await expect(await page.locator(".draggable-dialog")).not.toBeVisible(); }); test("can see error after changing tabs and coming back", async ({ page }) => { @@ -378,7 +382,7 @@ test.describe("Code Tab tests", () => { await page.click(".nav-tabs a:has-text('Options')"); await page.click(".nav-tabs a:has-text('Code')"); const lineElement = await page.locator(`.view-overlays div:nth-child(${2}) >> div`); - expect(lineElement).toHaveClass("cdr editor-error-background"); + await expect(lineElement).toHaveClass("cdr editor-error-background"); }); test("can see warning after changing tabs and coming back", async ({ page }) => { @@ -396,7 +400,7 @@ test.describe("Code Tab tests", () => { await page.click(".nav-tabs a:has-text('Options')"); await page.click(".nav-tabs a:has-text('Code')"); const lineElement = await page.locator(`.view-overlays div:nth-child(${3}) >> div`); - expect(lineElement).toHaveClass("cdr editor-warning-background"); + await expect(lineElement).toHaveClass("cdr editor-warning-background"); }); test("can change graph setting for log scale y axis from code tab", async ({ page }) => { diff --git a/app/static/tests/e2e/multiSensitivity.etest.ts b/app/static/tests/e2e/multiSensitivity.etest.ts index 029d1cb1..68acf79e 100644 --- a/app/static/tests/e2e/multiSensitivity.etest.ts +++ b/app/static/tests/e2e/multiSensitivity.etest.ts @@ -1,5 +1,5 @@ import { expect, Page, test } from "@playwright/test"; -import { SensitivityScaleType, SensitivityVariationType } from "../../src/app/store/sensitivity/state"; +import { SensitivityScaleType, SensitivityVariationType } from "../../src/store/sensitivity/state"; import PlaywrightConfig from "../../playwright.config"; import { expectCanRunMultiSensitivity, writeCode } from "./utils"; @@ -84,7 +84,7 @@ test.describe("Multi-sensitivity tests", () => { test("can edit Multi-sensitivity options", async ({ page }) => { // check default param settings - await expect(await page.locator(".sensitivity-options-settings").count()).toBe(1); + await expect(page.locator(".sensitivity-options-settings")).toHaveCount(1); await expectOptionsTabParamSettings( page, 1, diff --git a/app/static/tests/e2e/options.etest.ts b/app/static/tests/e2e/options.etest.ts index 990ffb02..79e51229 100644 --- a/app/static/tests/e2e/options.etest.ts +++ b/app/static/tests/e2e/options.etest.ts @@ -206,7 +206,7 @@ test.describe("Options Tab tests", () => { test("can change graph setting for log scale y axis", async ({ page }) => { await page.locator(".log-scale-y-axis input").click(); // should update y axis tick - const tickSelector = ":nth-match(.plotly .ytick text, 2)"; + const tickSelector = ":nth-match(.ytick text, 2)"; await expect(await page.innerHTML(tickSelector)).toBe("10n"); // change back to linear await page.locator(".log-scale-y-axis input").click(); @@ -216,7 +216,7 @@ test.describe("Options Tab tests", () => { test("can change graph setting for lock axes", async ({ page }) => { await page.locator(".lock-y-axis input").click(); - const tickSelector = ":nth-match(.plotly .ytick text, 6)"; + const tickSelector = ":nth-match(.ytick text, 6)"; await expect(await page.innerHTML(tickSelector)).toBe("1M"); await page.locator(":nth-match(.parameter-input, 3)").fill("1000000000"); @@ -234,7 +234,7 @@ test.describe("Options Tab tests", () => { test("overrides axes lock if log scale toggle changes", async ({ page }) => { await page.locator(".lock-y-axis input").click(); - const tickSelector = ":nth-match(.plotly .ytick text, 2)"; + const tickSelector = ":nth-match(.ytick text, 2)"; await expect(await page.innerHTML(tickSelector)).toBe("0.2M"); await page.locator(".log-scale-y-axis input").click(); @@ -412,13 +412,13 @@ test.describe("Options Tab tests", () => { await sigmaParam.fill("3"); const sigmaSavedParamTile = page.getByText("sigma: 2"); - expect(sigmaSavedParamTile).toBeVisible(); - expect(page.getByText("beta: 4")).not.toBeVisible(); + await expect(sigmaSavedParamTile).toBeVisible(); + await expect(page.getByText("beta: 4")).not.toBeVisible(); await page.getByLabel("Show unchanged parameters").check(); - expect(sigmaSavedParamTile).toBeVisible(); - expect(page.getByText("beta: 4")).toBeVisible(); + await expect(sigmaSavedParamTile).toBeVisible(); + await expect(page.getByText("beta: 4")).toBeVisible(); }); test("error tooltip and doesn't change name if same name exists", async ({ page }) => { @@ -434,8 +434,7 @@ test.describe("Options Tab tests", () => { await editDisplayName(2, page); await inputDisplayName(2, page, "random name 1"); await saveDisplayName(1, page); - await expect((await page.innerText(":nth-match(.tooltip-inner, 2)")).trim()).toBe("Name already exists"); - await expect(await page.isVisible(":nth-match(.param-name-input, 2)")).toBe(true); + await expect(await page.getByText("Name already exists")).toBeVisible(); }); test("error tooltip and doesn't change name if Set [number] format used", async ({ page }) => { @@ -443,11 +442,15 @@ test.describe("Options Tab tests", () => { await editDisplayName(1, page); await inputDisplayName(1, page, "Set 10"); await saveDisplayName(1, page); - await expect((await page.innerText(":nth-match(.tooltip-inner, 2)")).trim()).toBe( + await expect(await page.getByText( "Set 10 (or any Set [number] combination) is reserved for default set names. " + - "Please choose another set name or name this set back to its original name of 'Set 1'" - ); - await expect(await page.isVisible(".param-name-input")).toBe(true); + "Please choose another set name or name this set back to its original name of 'Set 1'" + )).toBeVisible(); + // await expect((await page.innerText(":nth-match(.tooltip-inner, 2)")).trim()).toBe( + // "Set 10 (or any Set [number] combination) is reserved for default set names. " + + // "Please choose another set name or name this set back to its original name of 'Set 1'" + // ); + // await expect(await page.isVisible(".param-name-input")).toBe(true); }); const fillInAdvancedInputs = async (type: string, advancedSetting: any, index: number) => { diff --git a/app/static/tests/e2e/sensitivity.etest.ts b/app/static/tests/e2e/sensitivity.etest.ts index a52d7c9e..0f315d81 100644 --- a/app/static/tests/e2e/sensitivity.etest.ts +++ b/app/static/tests/e2e/sensitivity.etest.ts @@ -106,7 +106,7 @@ test.describe("Sensitivity tests", () => { // run and see all traces await page.click("#run-sens-btn"); const linesSelector = `${plotSelector} .scatterlayer .trace .lines path`; - expect((await page.locator(`:nth-match(${linesSelector}, 30)`).getAttribute("d"))!.startsWith("M0")).toBe(true); + await expect((await page.locator(`:nth-match(${linesSelector}, 30)`).getAttribute("d"))!.startsWith("M0")).toBe(true); // expected legend and axes await expectLegend(page); diff --git a/app/static/tests/e2e/sessions.etest.ts b/app/static/tests/e2e/sessions.etest.ts index dbf2e463..458b2e45 100644 --- a/app/static/tests/e2e/sessions.etest.ts +++ b/app/static/tests/e2e/sessions.etest.ts @@ -57,6 +57,10 @@ test.describe("Sessions tests", () => { return chromium.launchPersistentContext(userDataDir); }; + test.use({ + permissions: ["clipboard-read", "clipboard-write"] + }) + test("can use Sessions page", async () => { const browser = await usePersistentContext(); const page = await browser.newPage(); diff --git a/app/static/tests/e2e/stochastic.etest.ts b/app/static/tests/e2e/stochastic.etest.ts index 8a4b05f3..0ab40eee 100644 --- a/app/static/tests/e2e/stochastic.etest.ts +++ b/app/static/tests/e2e/stochastic.etest.ts @@ -85,7 +85,7 @@ test.describe("stochastic app", () => { // Open Sensitivity tab await page.click(":nth-match(.wodin-right .nav-tabs a, 3)"); - page.click("#run-sens-btn"); + await page.click("#run-sens-btn"); // Should briefly see 'Running..' message await expect(await page.innerText("#sensitivity-running")).toContain("Running sensitivity"); From 9b5c429baff354bea4aff7150f67cb17755ae3f2 Mon Sep 17 00:00:00 2001 From: Mantra Date: Wed, 6 Nov 2024 13:32:54 +0000 Subject: [PATCH 2/4] fix getter --- app/static/src/store/graphs/getters.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/app/static/src/store/graphs/getters.ts b/app/static/src/store/graphs/getters.ts index 6cd947f5..58241699 100644 --- a/app/static/src/store/graphs/getters.ts +++ b/app/static/src/store/graphs/getters.ts @@ -33,9 +33,11 @@ export const getters: GraphsGetters & GetterTree = { }, [GraphsGetter.legendWidth]: (_, graphsGetters): number => { // Heuristic for setting graph legend width based on string length of longest variable name - const longestVar = graphsGetters[GraphsGetter.allSelectedVariables].sort((a: string, b: string) => - a.length < b.length ? 1 : -1 - )[0]; - return longestVar.length * LEGEND_WIDTH_PER_CHAR + LEGEND_LINE_PADDING; + let maxVariableLength = 0; + const variables = graphsGetters[GraphsGetter.allSelectedVariables]; + for (let i = 0; i < variables.length; i++) { + maxVariableLength = Math.max(maxVariableLength, variables[i]); + } + return maxVariableLength * LEGEND_WIDTH_PER_CHAR + LEGEND_LINE_PADDING; } }; From 3f9f49767b8422c9c2aca618f1fffa23576d7ce6 Mon Sep 17 00:00:00 2001 From: Mantra Date: Wed, 6 Nov 2024 14:10:31 +0000 Subject: [PATCH 3/4] fix bug --- app/static/src/store/graphs/getters.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/static/src/store/graphs/getters.ts b/app/static/src/store/graphs/getters.ts index 58241699..0fa47f6a 100644 --- a/app/static/src/store/graphs/getters.ts +++ b/app/static/src/store/graphs/getters.ts @@ -36,7 +36,7 @@ export const getters: GraphsGetters & GetterTree = { let maxVariableLength = 0; const variables = graphsGetters[GraphsGetter.allSelectedVariables]; for (let i = 0; i < variables.length; i++) { - maxVariableLength = Math.max(maxVariableLength, variables[i]); + maxVariableLength = Math.max(maxVariableLength, variables[i].length); } return maxVariableLength * LEGEND_WIDTH_PER_CHAR + LEGEND_LINE_PADDING; } From 57246fc1dce94ebc02e6a505bbcff42cc0c90d68 Mon Sep 17 00:00:00 2001 From: Mantra Date: Fri, 8 Nov 2024 12:11:12 +0000 Subject: [PATCH 4/4] add comments to code e2e test about glyph --- app/static/tests/e2e/code.etest.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/static/tests/e2e/code.etest.ts b/app/static/tests/e2e/code.etest.ts index efc65ad9..6d702450 100644 --- a/app/static/tests/e2e/code.etest.ts +++ b/app/static/tests/e2e/code.etest.ts @@ -48,6 +48,8 @@ test.beforeEach(async ({ page }) => { }); const expectMonacoDecoration = async (state: EditorStates, line: number, numOfLines: number, page: Page) => { + // check that the correct line has the correct background color here by looping through + // each line for (let i = 0; i < numOfLines; i += 1) { const lineElement = await page.locator(`.view-overlays div:nth-child(${i + 1}) >> div`); if (i === line - 1) { @@ -59,6 +61,10 @@ const expectMonacoDecoration = async (state: EditorStates, line: number, numOfLi } } + // check that the glyph (our error/warning icon) is vertically aligned with the + // correct line number, monaco editor just sets a "top" css property to vertically + // align it next to the line number so we calculate what that translation should be + // by multiplying the line-height by the number of lines down we have to go down const lineHeight = await page.locator(".margin-view-overlays").evaluate(el => { return window.getComputedStyle(el).getPropertyValue("line-height"); });