Skip to content

Commit

Permalink
fix percy flakiness (metabase#23837)
Browse files Browse the repository at this point in the history
* fix percy flakiness

* run e2e

* fix percy flakiness

* update docs, remove timeout config that did not help
  • Loading branch information
alxnddr authored Jul 22, 2022
1 parent 361e20b commit b0e26a3
Show file tree
Hide file tree
Showing 33 changed files with 88 additions and 71 deletions.
3 changes: 2 additions & 1 deletion .github/workflows/percy-issue-comment.yml
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ jobs:
env:
NPM_TOKEN: ${{ secrets.NPM_TOKEN }}
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

MB_EDITION: ee
ENTERPRISE_TOKEN: ${{ secrets.ENTERPRISE_TOKEN }}
steps:
- uses: actions/checkout@v3
with:
Expand Down
2 changes: 2 additions & 0 deletions .github/workflows/percy-visual-label.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ jobs:
env:
NPM_TOKEN: ${{ secrets.NPM_TOKEN }}
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
MB_EDITION: ee
ENTERPRISE_TOKEN: ${{ secrets.ENTERPRISE_TOKEN }}
steps:
- uses: actions/checkout@v3
- name: Prepare front-end environment
Expand Down
1 change: 1 addition & 0 deletions .percy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@ snapshot:
widths:
- 1280
min-height: 800

discovery:
disable-cache: true
5 changes: 3 additions & 2 deletions docs/developers-guide/visual-tests.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ In addition to that, we need to ensure that underlying Cypress tests are valid,

We use Cypress to write Percy tests so we can fully use all existing helpers and custom commands.

Visual regression tests live inside the `frontend/test/metabase-visual` directory. Writing a Percy test consists of creating a desired page state and executing `cy.percySnapshot()` command.
Visual regression tests live inside the `frontend/test/metabase-visual` directory. Writing a Percy test consists of creating a desired page state and executing `cy.createPercySnapshot()` command.

### Goal

Expand All @@ -49,4 +49,5 @@ Consider the page state at `percyHealthCheck` step as the one that will be captu

- You don't need to export `PERCY_TOKEN` for running tests. If a token is exported Percy will send snapshots from your local machine to their servers so that you will be able to see your local run in their interface.
- When the application code uses `Date.now()`, you can [freeze](https://docs.percy.io/docs/freezing-dynamic-data#freezing-datetime-in-cypress) date/time in Cypress.
- [Stub](https://github.com/metabase/metabase/pull/17380/files#diff-4e8ebaf75969143a5eee6bfb8adcd4b72d4330d18d77319e3434d11cf6c75e40R15) `Math.random` when to deal with randomization.
- [Stub](https://github.com/metabase/metabase/pull/17380/files#diff-4e8ebaf75969143a5eee6bfb8adcd4b72d4330d18d77319e3434d11cf6c75e40R15) `Math.random` when you deal with randomization.
- When testing a page that renders any dates coming from the server, in order to avoid unwanted visual changes, add `data-server-date` attribute to all DOM nodes that render dates. The custom `createPercySnapshot` command replaces content with a constant placeholder before capturing a snapshot.
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ const ArchiveModal = ({
onClose={onClose}
>
{isCreator(item, user) && hasUnsubscribed && (
<ModalMessage>
<ModalMessage data-server-date>
{getCreatorMessage(type, user)}
{t`As the creator you can also choose to delete this if it’s no longer relevant to others as well.`}
</ModalMessage>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ const NotificationCard = ({
{getChannelMessage(channel)}
</NotificationMessage>
))}
<NotificationMessage>
<NotificationMessage data-server-date>
{getCreatorMessage(item, user)}
</NotificationMessage>
</NotificationDescription>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ export function BaseTableItem({
<ItemCell data-testid={`${testId}-last-edited-by`}>
<Ellipsified>{lastEditedBy}</Ellipsified>
</ItemCell>
<ItemCell data-testid={`${testId}-last-edited-at`}>
<ItemCell data-testid={`${testId}-last-edited-at`} data-server-date>
{lastEditInfo && (
<Tooltip tooltip={<DateTime value={lastEditInfo.timestamp} />}>
<TableItemSecondaryField>{lastEditedAt}</TableItemSecondaryField>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ const EventCard = ({
{event.description && (
<CardDescription>{event.description}</CardDescription>
)}
<CardCreatorInfo>{creatorMessage}</CardCreatorInfo>
<CardCreatorInfo data-server-date>{creatorMessage}</CardCreatorInfo>
</CardBody>
{menuItems.length > 0 && (
<CardAside>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ const EventCard = ({
{event.description && (
<CardDescription>{event.description}</CardDescription>
)}
<CardCreatorInfo>{creatorMessage}</CardCreatorInfo>
<CardCreatorInfo data-server-date>{creatorMessage}</CardCreatorInfo>
</CardBody>
{menuItems.length > 0 && (
<CardAside onClick={handleAsideClick}>
Expand Down
2 changes: 2 additions & 0 deletions frontend/test/__support__/e2e/commands.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,5 @@ import "./commands/visibility/findByTextEnsureVisible";
import "./commands/visibility/isRenderedWithinViewport";

import "./commands/overwrites/log";

import "./commands/percy/createPercySnapshot";
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
Cypress.Commands.add("createPercySnapshot", (name, options) => {
cy.window().then(win => {
// Replace dates that came from server
win.document
.querySelectorAll("[data-server-date]")
.forEach(el => (el.innerHTML = "server-side date"));
});

cy.percySnapshot(name, options);
});
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,6 @@ describe("visual tests > account > notifications", () => {
it("renders notifications", () => {
cy.visit("/account/notifications");
cy.findByText("Question");
cy.percySnapshot();
cy.createPercySnapshot();
});
});
12 changes: 6 additions & 6 deletions frontend/test/metabase-visual/admin/colors.cy.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,14 @@ describeEE("visual tests > admin > colors", () => {
});

visitQuestionAdhoc(questionDetails);
cy.percySnapshot("chart");
cy.createPercySnapshot("chart");

cy.icon("notebook").click();
cy.percySnapshot("filters");
cy.createPercySnapshot("filters");
cy.icon("notebook").click();

cy.findByText("Summarize").click();
cy.percySnapshot("summarize");
cy.createPercySnapshot("summarize");
});

it("should use custom chart colors", () => {
Expand All @@ -65,13 +65,13 @@ describeEE("visual tests > admin > colors", () => {
});

visitQuestionAdhoc(questionDetails);
cy.percySnapshot("chart");
cy.createPercySnapshot("chart");

cy.icon("notebook").click();
cy.percySnapshot("filters");
cy.createPercySnapshot("filters");
cy.icon("notebook").click();

cy.findByText("Summarize").click();
cy.percySnapshot("summarize");
cy.createPercySnapshot("summarize");
});
});
4 changes: 2 additions & 2 deletions frontend/test/metabase-visual/admin/fonts.cy.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ describeEE("visual tests > admin > fonts", () => {
cy.findByText("Roboto Mono").click();
waitForFont("Roboto Mono");

cy.percySnapshot();
cy.createPercySnapshot();
});

it("should set a custom font", () => {
Expand All @@ -32,7 +32,7 @@ describeEE("visual tests > admin > fonts", () => {
typeAndBlurUsingLabel("Regular", CUSTOM_FONT_URL);
waitForFont("Custom");

cy.percySnapshot();
cy.createPercySnapshot();
});
});

Expand Down
12 changes: 6 additions & 6 deletions frontend/test/metabase-visual/admin/permissions.cy.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,41 +10,41 @@ describe("visual tests > admin > permissions", () => {
it("database focused view", () => {
cy.visit("/admin/permissions/data/database/1");
cy.findByPlaceholderText("Search for a group");
cy.percySnapshot();
cy.createPercySnapshot();
});

it("database focused view > table", () => {
cy.visit("/admin/permissions/data/database/1/schema/PUBLIC/table/1");
cy.findByPlaceholderText("Search for a group");
cy.findByPlaceholderText("Search for a table");
cy.percySnapshot();
cy.createPercySnapshot();
});

it("group focused view", () => {
cy.visit("/admin/permissions/data/group/1");
cy.findByPlaceholderText("Search for a database");
cy.percySnapshot();
cy.createPercySnapshot();
});

it("group focused view > database", () => {
cy.visit("/admin/permissions/data/group/1/database/1");
cy.findByPlaceholderText("Search for a table");
cy.percySnapshot();
cy.createPercySnapshot();
});
});

describe("collection permissions", () => {
it("editor", () => {
cy.visit("/admin/permissions/collections/11");
cy.findByPlaceholderText("Search for a group");
cy.percySnapshot();
cy.createPercySnapshot();
});

// This revealed the infinite loop which resulted in metabase#21026
it.skip("modal", () => {
cy.visit("/collection/root/permissions");
cy.findByText("Group name");
cy.percySnapshot();
cy.createPercySnapshot();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ describe("Bookmarks in a collection page", () => {
getSectionTitle(/Bookmarks/);
});

cy.percySnapshot();
cy.createPercySnapshot();
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,15 @@ describe("timelines", () => {
cy.visit("/collection/root/timelines");

cy.findByText("Our analytics events").should("be.visible");
cy.percySnapshot();
cy.createPercySnapshot();
});

it("should display timeline events in collections", () => {
cy.createTimelineWithEvents({ events: EVENTS }).then(({ timeline }) => {
cy.visit(`/collection/root/timelines/${timeline.id}`);

cy.findByText("Timeline").should("be.visible");
cy.percySnapshot();
cy.createPercySnapshot();
});
});

Expand Down Expand Up @@ -65,7 +65,7 @@ describe("timelines", () => {

cy.findByLabelText("star icon").realHover();
cy.findByText("RC1");
cy.percySnapshot();
cy.createPercySnapshot();
});

it("should display timeline events with a native question", () => {
Expand Down Expand Up @@ -96,6 +96,6 @@ describe("timelines", () => {

cy.findByLabelText("star icon").realHover();
cy.findByText("RC1");
cy.percySnapshot();
cy.createPercySnapshot();
});
});
4 changes: 2 additions & 2 deletions frontend/test/metabase-visual/dashboard/fullscreen.cy.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,13 @@ describe("visual tests > dashboard > fullscreen", () => {

cy.icon("moon");

cy.percySnapshot("day");
cy.createPercySnapshot("day");

cy.icon("moon").click();

cy.icon("sun");

cy.percySnapshot("night");
cy.createPercySnapshot("night");

cy.icon("contract").click();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ describe(`visual tests > dashboard > parameters widget`, () => {
cy.get("main")
.scrollTo(0, 264)
.then(() => {
cy.percySnapshot();
cy.createPercySnapshot();
});

cy.findByTestId("dashboard-parameters-widget-container").should(
Expand All @@ -69,7 +69,7 @@ describe(`visual tests > dashboard > parameters widget`, () => {
cy.findByTestId("dashboard-parameters-and-cards")
.scrollTo(0, 464)
.then(() => {
cy.percySnapshot();
cy.createPercySnapshot();
});

cy.findByTestId("edit-dashboard-parameters-widget-container").should(
Expand All @@ -87,7 +87,7 @@ describe(`visual tests > dashboard > parameters widget`, () => {
cy.get("main")
.scrollTo(0, 264)
.then(() => {
cy.percySnapshot(null, { widths: [MOBILE_WIDTH] });
cy.createPercySnapshot(null, { widths: [MOBILE_WIDTH] });
});

cy.findByTestId("dashboard-parameters-widget-container").should(
Expand All @@ -105,7 +105,7 @@ describe(`visual tests > dashboard > parameters widget`, () => {
cy.findByTestId("dashboard-parameters-and-cards")
.scrollTo(0, 464)
.then(() => {
cy.percySnapshot(null, { widths: [MOBILE_WIDTH] });
cy.createPercySnapshot(null, { widths: [MOBILE_WIDTH] });
});

cy.findByTestId("edit-dashboard-parameters-widget-container").should(
Expand Down Expand Up @@ -149,7 +149,7 @@ describe(`visual tests > dashboard > parameters widget`, () => {
cy.get("main")
.scrollTo(0, 264)
.then(() => {
cy.percySnapshot();
cy.createPercySnapshot();
});

cy.findByTestId("dashboard-parameters-widget-container").should(
Expand All @@ -167,7 +167,7 @@ describe(`visual tests > dashboard > parameters widget`, () => {
cy.findByTestId("dashboard-parameters-and-cards")
.scrollTo(0, 464)
.then(() => {
cy.percySnapshot();
cy.createPercySnapshot();
});

cy.findByTestId("edit-dashboard-parameters-widget-container").should(
Expand All @@ -185,7 +185,7 @@ describe(`visual tests > dashboard > parameters widget`, () => {
cy.get("main")
.scrollTo(0, 264)
.then(() => {
cy.percySnapshot(null, { widths: [MOBILE_WIDTH] });
cy.createPercySnapshot(null, { widths: [MOBILE_WIDTH] });
});

cy.findByTestId("dashboard-parameters-widget-container").should(
Expand All @@ -203,7 +203,7 @@ describe(`visual tests > dashboard > parameters widget`, () => {
cy.findByTestId("dashboard-parameters-and-cards")
.scrollTo(0, 464)
.then(() => {
cy.percySnapshot(null, { widths: [MOBILE_WIDTH] });
cy.createPercySnapshot(null, { widths: [MOBILE_WIDTH] });
});

cy.findByTestId("edit-dashboard-parameters-widget-container").should(
Expand Down
8 changes: 4 additions & 4 deletions frontend/test/metabase-visual/models/editor.cy.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ describe("visual tests > models > editor", () => {
cy.wait("@cardQuery");
cy.findByText(/Doing science/).should("not.exist");
cy.wait(100); // waits for the colums widths calculation
cy.percySnapshot(
cy.createPercySnapshot(
"visual tests > models > editor > GUI query > renders query editor correctly",
);
});
Expand All @@ -37,7 +37,7 @@ describe("visual tests > models > editor", () => {
cy.wait("@cardQuery");
cy.findByText(/Doing science/).should("not.exist");
cy.wait(100); // waits for the colums widths calculation
cy.percySnapshot(
cy.createPercySnapshot(
"visual tests > models > editor > GUI query > renders metadata editor correctly",
);
});
Expand All @@ -56,7 +56,7 @@ describe("visual tests > models > editor", () => {
cy.visit(`/model/${MODEL_ID}/query`);
cy.wait("@cardQuery");
cy.findByText(/Doing science/).should("not.exist");
cy.percySnapshot(
cy.createPercySnapshot(
"visual tests > models > editor > native query > renders query editor correctly",
);
});
Expand All @@ -74,7 +74,7 @@ describe("visual tests > models > editor", () => {
cy.wait("@cardQuery");
cy.findByText(/Doing science/).should("not.exist");
cy.wait(100); // waits for the colums widths calculation
cy.percySnapshot(
cy.createPercySnapshot(
"visual tests > models > editor > native query > renders metadata editor correctly",
);
});
Expand Down
Loading

0 comments on commit b0e26a3

Please sign in to comment.