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.

Sign up for GitHub

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

mrc-5967 update e2e tests #229

Merged
merged 4 commits into from
Nov 13, 2024
Merged

mrc-5967 update e2e tests #229

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
15c8fc1
update e2e tests
M-Kusumgar Nov 6, 2024
9b5c429
fix getter
M-Kusumgar Nov 6, 2024
3f9f497
fix bug
M-Kusumgar Nov 6, 2024
57246fc
add comments to code e2e test about glyph
M-Kusumgar Nov 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions app/static/src/store/graphs/getters.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,11 @@ export const getters: GraphsGetters & GetterTree<GraphsState, AppState> = {
},
[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].length);
}
return maxVariableLength * LEGEND_WIDTH_PER_CHAR + LEGEND_LINE_PADDING;
Comment on lines +36 to +41
Copy link
Contributor

Choose a reason for hiding this comment

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

This the change where the sort was reordering the result of the getter - because the getter result is cached and reused when some drop-down uses it? Which drop down was that?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yep exactly its being reused but sort also mutated the array in place, it was setting the wrong order in the select in LinkData.vue

}
};
64 changes: 34 additions & 30 deletions app/static/tests/e2e/code.etest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@
sigma <- user(2)
`;

const editorGlyphs: any = {
error: "fa-solid fa-circle-xmark",
warning: "fa-solid fa-triangle-exclamation"
const editorGlyphs: Record<EditorStates, string> = {
error: "fa-circle-xmark",
warning: "fa-triangle-exclamation"
};

enum EditorStates {
Expand All @@ -47,36 +47,40 @@
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`);
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like this method could use some comments because I'm honestly quite confused about what's being checked for here! In the loop we're checking that there's only current-line/background styling for numOfLinesth line and its predecessor? And then the last part is checking that the glyph is on the specified line (by top)..?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yep will add some comments but yh the for loop checks for the background colour of the line so the highlighting and this part is checking if the glyph is vertically aligned with the correct line number

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

};

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";

Check failure on line 72 in app/static/tests/e2e/code.etest.ts

View workflow job for this annotation

GitHub Actions / test 

'isGlyph' is assigned a value but never used
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");
Comment on lines +82 to +88
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ignore the commented out code and the block below, i remove them in the next PR, i accidentally pushed the cleaning out commented code commit to the next pr

await expect(await page.getByText(message)).toBeVisible()
};

test.describe("Code Tab tests", () => {
Expand Down Expand Up @@ -128,9 +132,9 @@
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"
);
});
Expand Down Expand Up @@ -355,12 +359,12 @@

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 }) => {
Expand All @@ -378,7 +382,7 @@
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 }) => {
Expand All @@ -396,7 +400,7 @@
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 }) => {
Expand Down
4 changes: 2 additions & 2 deletions app/static/tests/e2e/multiSensitivity.etest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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";

Expand Down Expand Up @@ -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,
Expand Down
29 changes: 16 additions & 13 deletions app/static/tests/e2e/options.etest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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");
Expand All @@ -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();
Expand Down Expand Up @@ -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 }) => {
Expand All @@ -434,20 +434,23 @@ 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 }) => {
await createParameterSet(page);
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);
Comment on lines +449 to +453
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be removed now?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yep removed in the linting PR (next one)

});

const fillInAdvancedInputs = async (type: string, advancedSetting: any, index: number) => {
Expand Down
2 changes: 1 addition & 1 deletion app/static/tests/e2e/sensitivity.etest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 4 additions & 0 deletions app/static/tests/e2e/sessions.etest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ test.describe("Sessions tests", () => {
return chromium.launchPersistentContext(userDataDir);
};

test.use({
permissions: ["clipboard-read", "clipboard-write"]
})

Comment on lines +60 to +63
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yep it does! we have copy to clipboard buttons when we copy the code or link for a session and we check those like line 134 in this file

test("can use Sessions page", async () => {
const browser = await usePersistentContext();
const page = await browser.newPage();
Expand Down
2 changes: 1 addition & 1 deletion app/static/tests/e2e/stochastic.etest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { expect, Page, test } from "@playwright/test";

Check failure on line 1 in app/static/tests/e2e/stochastic.etest.ts

View workflow job for this annotation

GitHub Actions / test 

'Page' is defined but never used
import PlaywrightConfig from "../../playwright.config";
import { expectSummaryValues } from "./utils";

Expand Down Expand Up @@ -85,7 +85,7 @@
// 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");
Expand Down
Loading