From e6c518a93170f686726a30e627749f8de55649cc Mon Sep 17 00:00:00 2001
From: Mark Bastian <markbastian@gmail.com>
Date: Thu, 29 Feb 2024 16:39:29 -0700
Subject: [PATCH] Fixing E2E Cross-version Tests (#39285)

* Fixing E2E Cross-version Tests

E2E cross version tests were broken for multiple reasons, primarily due to changes made in UI layout, cypress testing, and data changes. Overall, the basic workflows still work, but subtle click order differences and other small changes made the entire job consistently fail.

To fix this, this PR:
- Provides a function, `parseVersionString`, which computes version information provided a test version string. This information includes the raw string version, the edition (ee or oss), and the major, minor, and patch version of the version string.
- In the same file, `cross-version-helpers.js`, adds constants identifying versions in which certain breaking UI or Cypress changes occurred.
- Finally, a set of exported functions are provided which take a version object and execute conditional logic based on the provided version so that behavior is consistent across Metabase versions.

The versions are computed in `cross-version-source-helpers.js` and `cross-version-target-helpers.js` and are used in js files as shown:

```js
import { visualize } from "e2e/support/helpers";
import {
  fillAreaUnderLineChart,
  newQuestion,
  saveQuestion,
} from "e2e/test/scenarios/cross-version/helpers/cross-version-helpers.js";
```

This then allows logic like the below, where the high level call is made and the right logic is dispatched based on the version information.

```js
it("should create questions", () => {
  cy.signInAsAdmin();

  cy.visit("/question/new");

  newQuestion(version);

  // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
  cy.findByText("Orders").click();
})
```

Other smaller changes were also made that created a more universal sequence of UI actions.

Fixes #39229

* clear -> click

* Fixing area icon selection

* Restoring scheduling to align with master

* fe lint
---
 .github/workflows/e2e-cross-version.yml       |   2 +
 .../helpers/cross-version-helpers.js          | 175 ++++++++++++++++++
 .../cross-version/source/00-setup.cy.spec.js  |  11 +-
 .../source/02-datamodel.cy.spec.js            |  31 +++-
 .../source/03-questions.cy.spec.js            |  50 ++++-
 .../helpers/cross-version-source-helpers.js   |  46 +----
 .../helpers/cross-version-target-helpers.js   |   4 +-
 .../cross-version/target/smoke.cy.spec.js     |  17 +-
 8 files changed, 264 insertions(+), 72 deletions(-)
 create mode 100644 e2e/test/scenarios/cross-version/helpers/cross-version-helpers.js

diff --git a/.github/workflows/e2e-cross-version.yml b/.github/workflows/e2e-cross-version.yml
index 440ce2340c2..38a3fc0b9a3 100644
--- a/.github/workflows/e2e-cross-version.yml
+++ b/.github/workflows/e2e-cross-version.yml
@@ -11,6 +11,8 @@ jobs:
     strategy:
       matrix:
         version: [
+          # Major OSS upgrade
+          { source: v0.42.2, target: v0.48.7 },
           # OSS upgrade
           { source: v0.41.3.1, target: v0.42.2 },
           # EE upgrade
diff --git a/e2e/test/scenarios/cross-version/helpers/cross-version-helpers.js b/e2e/test/scenarios/cross-version/helpers/cross-version-helpers.js
new file mode 100644
index 00000000000..8cc385a0ee3
--- /dev/null
+++ b/e2e/test/scenarios/cross-version/helpers/cross-version-helpers.js
@@ -0,0 +1,175 @@
+export function parseVersionString(versionString) {
+  if (typeof versionString === "undefined") {
+    return []; // Return empty array if versionString is undefined
+  }
+
+  const segments = versionString.match(/\d+/g);
+  if (!segments) {
+    return { version: versionString }; // Return versionString if no segments found
+  }
+
+  const segmentInts = segments.map(segment => parseInt(segment, 10));
+  const editionIndex = segmentInts[0];
+
+  let edition = null;
+  if (editionIndex === 1) {
+    edition = "ee";
+  } else if (editionIndex === 0) {
+    edition = "oss";
+  }
+
+  return {
+    version: versionString,
+    edition: edition,
+    majorVersion: segmentInts[1],
+    minorVersion: segmentInts[2],
+    patchVersion: segmentInts[3],
+  };
+}
+
+// Versions in which a breaking GUI change was introduced. For example, version
+// 32 added a "What will you use Metabase for?" stage in the initial setup.
+
+// Change: Adds "What will you use Metabase for?" stage in the initial setup.
+// Git sha: d88d32e5e021ad4f47b5d740b78df73945e8ff82
+// Date: 2024-02-08
+// Author: npretto
+const metabasePurposeVersion = 49;
+
+// Change: Question save modal logic changes required different cypress logic
+//         to click "Save" button as former cy.clck("Save") became ambiguous.
+// Git sha: f90e4db22e0b668600846e00ed4d1e28d8f92d95
+// Date: 2024-02-23
+// Author: markbastian
+const updatedCypressSaveLogicVersion = 49;
+
+// Change: Sample data times changed.
+// TODO: The specific version in which this changed hasn't been tracked down
+// yet, but this is an upper bound. If more cross-version checks are added for
+// earlier versions and this breaks, this number may need to be adjusted down.
+export const sampleDataTimesChangedVersion = 47;
+
+// Change: In older versions of metabase, when a question was loaded a modal was
+// presented stating "It's okay to play around with saved questions". The user
+// was required to press "Okay" to dismiss this modal. In later versions this
+// modal was not presented in the same location.
+// TODO: The specific version in which this changed hasn't been tracked down
+// yet, but this is an upper bound. If more cross-version checks are added for
+// earlier versions and this breaks, this number may need to be adjusted down.
+export const questionsAreOkToPlayWithModalVersion = 45;
+
+// Change: Older versions of Metabase presented a 3-panel display, the center
+// panel being "Custom question". In later generations, this was accessible via
+// clicking "New" -> "Question" at the top right corner in the UI.
+// TODO: The specific version in which this changed hasn't been tracked down
+// yet, but this is an upper bound. If more cross-version checks are added for
+// earlier versions and this breaks, this number may need to be adjusted down.
+export const newQuestionMenuVersion = 45;
+
+// Change: In older versions of metabase, area and line charts were selectable
+// Visualization types, but the actual operation to fill the area was to select
+// the "area" icon in the Display settings, which is really weird.
+// TODO: The specific version in which this changed hasn't been tracked down
+// yet, but this is an upper bound. If more cross-version checks are added for
+// earlier versions and this breaks, this number may need to be adjusted down.
+export const filledAreaIconRemovedVersion = 45;
+
+export function setupLanguage() {
+  // Make sure English is the default selected language
+  cy.findByText("English")
+    .should("have.css", "background-color")
+    .and("eq", "rgb(80, 158, 227)");
+
+  cy.button("Next").click();
+  cy.findByText("Your language is set to English");
+}
+
+export function setupInstance({ version, majorVersion }) {
+  const companyLabel =
+    version === "v0.41.3.1"
+      ? "Your company or team name"
+      : "Company or team name";
+
+  const finalSetupButton = version === "v0.41.3.1" ? "Next" : "Finish";
+
+  cy.findByLabelText("First name").type("Superuser");
+  cy.findByLabelText("Last name").type("Tableton");
+  cy.findByLabelText("Email").type("admin@metabase.test");
+  cy.findByLabelText(companyLabel).type("Metabase");
+  cy.findByLabelText("Create a password").type("12341234");
+  cy.findByLabelText("Confirm your password").type("12341234");
+  cy.button("Next").click();
+  cy.findByText("Hi, Superuser. Nice to meet you!");
+
+  // A "What will you use Metabase for?" prompt exists in later versions.
+  // If it exists, click through.
+  if (majorVersion >= metabasePurposeVersion) {
+    cy.button("Next").click();
+  }
+
+  cy.findByText("I'll add my data later").click();
+  cy.findByText("I'll add my own data later");
+
+  // Collection defaults to on and describes data collection
+  cy.findByText("All collection is completely anonymous.");
+  // turn collection off, which hides data collection description
+  cy.findByLabelText(
+    "Allow Metabase to anonymously collect usage events",
+  ).click();
+  cy.findByText("All collection is completely anonymous.").should("not.exist");
+  cy.findByText(finalSetupButton).click();
+  cy.findByText("Take me to Metabase").click();
+
+  cy.location("pathname").should("eq", "/");
+  cy.contains("Reviews");
+}
+
+export function newQuestion({ majorVersion }) {
+  if (majorVersion < newQuestionMenuVersion) {
+    cy.findByText("Custom question").click();
+  } else {
+    cy.button("New").click();
+    cy.findByText("Question").click();
+  }
+}
+
+export function saveQuestion({ majorVersion }) {
+  if (majorVersion < updatedCypressSaveLogicVersion) {
+    cy.button("Save").click();
+  } else {
+    cy.findByTestId("save-question-modal").within(modal => {
+      cy.findByText("Save").click();
+    });
+  }
+}
+
+export function assertTimelineData({ majorVersion }) {
+  if (majorVersion < sampleDataTimesChangedVersion) {
+    cy.get(".x.axis .tick")
+      .should("contain", "Q1 - 2017")
+      .and("contain", "Q1 - 2018")
+      .and("contain", "Q1 - 2019")
+      .and("contain", "Q1 - 2020");
+  } else {
+    cy.get(".x.axis .tick")
+      .should("contain", "Q1 2023")
+      .and("contain", "Q1 2024")
+      .and("contain", "Q1 2025")
+      .and("contain", "Q1 2026");
+  }
+}
+
+export function dismissOkToPlayWithQuestionsModal({ majorVersion }) {
+  if (majorVersion < questionsAreOkToPlayWithModalVersion) {
+    cy.findByText("It's okay to play around with saved questions");
+    cy.button("Okay").click();
+  }
+}
+
+export function fillAreaUnderLineChart({ majorVersion }) {
+  if (majorVersion < filledAreaIconRemovedVersion) {
+    cy.findByTestId("sidebar-left").within(element => {
+      cy.icon("area").click();
+    });
+  }
+}
diff --git a/e2e/test/scenarios/cross-version/source/00-setup.cy.spec.js b/e2e/test/scenarios/cross-version/source/00-setup.cy.spec.js
index 50838f0d803..6c1cb826f68 100644
--- a/e2e/test/scenarios/cross-version/source/00-setup.cy.spec.js
+++ b/e2e/test/scenarios/cross-version/source/00-setup.cy.spec.js
@@ -1,8 +1,9 @@
 import {
-  version,
   setupLanguage,
   setupInstance,
-} from "./helpers/cross-version-source-helpers";
+} from "e2e/test/scenarios/cross-version/helpers/cross-version-helpers.js";
+
+import { version } from "./helpers/cross-version-source-helpers";
 
 describe(`setup on ${version}`, () => {
   it("should set up metabase", () => {
@@ -14,10 +15,12 @@ describe(`setup on ${version}`, () => {
     // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
     cy.findByText("Let's get started").click();
 
-    setupLanguage();
+    setupLanguage(version);
     setupInstance(version);
 
     cy.visit("/admin");
-    cy.icon("store");
+    // Find an element on the admin page so we know we've landed.
+    // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
+    cy.findByText("Setup");
   });
 });
diff --git a/e2e/test/scenarios/cross-version/source/02-datamodel.cy.spec.js b/e2e/test/scenarios/cross-version/source/02-datamodel.cy.spec.js
index 87e06e0593a..d25d5c94f61 100644
--- a/e2e/test/scenarios/cross-version/source/02-datamodel.cy.spec.js
+++ b/e2e/test/scenarios/cross-version/source/02-datamodel.cy.spec.js
@@ -21,7 +21,13 @@ it("should configure data model settings", () => {
   );
 
   cy.get(".AdminList").findByText("Orders").click();
-  cy.findByDisplayValue("Product ID").parent().find(".Icon-gear").click();
+
+  cy.findByDisplayValue("Product ID")
+    .parent()
+    .parent()
+    .find(".Icon-gear")
+    .click();
+
   // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
   cy.findByText("Use original value").click();
   // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
@@ -36,7 +42,8 @@ it("should configure data model settings", () => {
     "remapRatingValues",
   );
 
-  cy.findByDisplayValue("Rating").parent().find(".Icon-gear").click();
+  cy.findByDisplayValue("Rating").parent().parent().find(".Icon-gear").click();
+
   // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
   cy.findByText("Use original value").click();
   // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
@@ -67,13 +74,21 @@ it("should configure data model settings", () => {
   cy.get(".AdminList").findByText("Products").click();
 
   cy.intercept("PUT", `/api/field/${PRODUCTS.EAN}`).as("hideEan");
-  cy.findByDisplayValue("Ean").parent().contains("Everywhere").click();
+
+  cy.findByDisplayValue("Ean").parent().parent().contains("Everywhere").click();
+
   // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
   cy.findByText("Do not include").click();
   cy.wait("@hideEan");
 
   cy.intercept("PUT", `/api/field/${PRODUCTS.PRICE}`).as("updatePriceField");
-  cy.findByDisplayValue("Price").parent().contains("No semantic type").click();
+
+  cy.findByDisplayValue("Price")
+    .parent()
+    .parent()
+    .contains("No semantic type")
+    .click();
+
   cy.get(".MB-Select")
     .scrollTo("top")
     .within(() => {
@@ -91,7 +106,13 @@ it("should configure data model settings", () => {
   cy.get(".AdminList").findByText("People").click();
 
   cy.intercept("PUT", `/api/field/${PEOPLE.PASSWORD}`).as("hidePassword");
-  cy.findByDisplayValue("Password").parent().contains("Everywhere").click();
+
+  cy.findByDisplayValue("Password")
+    .parent()
+    .parent()
+    .contains("Everywhere")
+    .click();
+
   // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
   cy.findByText("Do not include").click();
   cy.wait("@hidePassword");
diff --git a/e2e/test/scenarios/cross-version/source/03-questions.cy.spec.js b/e2e/test/scenarios/cross-version/source/03-questions.cy.spec.js
index 7a6a1c5a3c0..82d0c42ca9b 100644
--- a/e2e/test/scenarios/cross-version/source/03-questions.cy.spec.js
+++ b/e2e/test/scenarios/cross-version/source/03-questions.cy.spec.js
@@ -1,13 +1,22 @@
 import { visualize } from "e2e/support/helpers";
+import {
+  fillAreaUnderLineChart,
+  newQuestion,
+  saveQuestion,
+} from "e2e/test/scenarios/cross-version/helpers/cross-version-helpers.js";
+
+import { version } from "./helpers/cross-version-source-helpers";
 
 it("should create questions", () => {
   cy.signInAsAdmin();
 
   cy.visit("/question/new");
-  // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
-  cy.findByText("Custom question").click();
+
+  newQuestion(version);
+
   // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
   cy.findByText("Orders").click();
+
   cy.icon("join_left_outer").click();
   // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
   cy.findByText("Products").click();
@@ -31,7 +40,12 @@ it("should create questions", () => {
 
   // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
   cy.findByText("Pick a column to group by").click();
-  cy.get(".List-section-title").contains("Products").click();
+
+  // Older versions were Products, newer use Product
+  cy.get(".List-section-title")
+    .contains(/Products?/)
+    .click();
+
   // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
   cy.findByText("Category").click();
 
@@ -40,6 +54,13 @@ it("should create questions", () => {
   cy.get(".bar").should("have.length", 4);
 
   cy.findByTestId("viz-settings-button").click();
+
+  //NOTE: In older versions of Metabase, Display is selected by default. Newer
+  // versions default to Data. This will ensure we've selected the right tab
+  // either way.
+  // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
+  cy.findByText("Display").click();
+
   // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
   cy.contains("Show values on data points").next().click();
   // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
@@ -52,13 +73,16 @@ it("should create questions", () => {
     "The average rating of our top selling products broken down into categories.",
     { delay: 0 },
   );
-  cy.button("Save").click();
+
+  saveQuestion(version);
+
   // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
   cy.findByText("Not now").click();
 
   cy.visit("/question/new");
-  // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
-  cy.findByText("Custom question").click();
+
+  newQuestion(version);
+
   // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
   cy.findByText(/Sample (Dataset|Database)/).click();
   // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
@@ -86,8 +110,14 @@ it("should create questions", () => {
   visualize();
   cy.get("circle");
 
-  cy.findByTestId("viz-settings-button").click();
-  cy.icon("area").click();
+  cy.findByTestId("viz-type-button").click();
+  cy.findByTestId("Area-button").click();
+
+  // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
+  cy.findByText("Display").click();
+
+  fillAreaUnderLineChart(version);
+
   // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
   cy.findByText("Goal line").next().click();
   cy.findByDisplayValue("0").type("100000").blur();
@@ -98,7 +128,9 @@ it("should create questions", () => {
   // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
   cy.findByText("Save").click();
   cy.findByLabelText("Name").clear().type("Quarterly Revenue");
-  cy.button("Save").click();
+
+  saveQuestion(version);
+
   // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
   cy.findByText("Not now").click();
 });
diff --git a/e2e/test/scenarios/cross-version/source/helpers/cross-version-source-helpers.js b/e2e/test/scenarios/cross-version/source/helpers/cross-version-source-helpers.js
index cdfc9f4eb35..fd0d967def0 100644
--- a/e2e/test/scenarios/cross-version/source/helpers/cross-version-source-helpers.js
+++ b/e2e/test/scenarios/cross-version/source/helpers/cross-version-source-helpers.js
@@ -1,45 +1,3 @@
-export const version = Cypress.env("SOURCE_VERSION");
+import { parseVersionString } from "e2e/test/scenarios/cross-version/helpers/cross-version-helpers.js";
 
-export function setupLanguage() {
-  // Make sure English is the default selected language
-  cy.findByText("English")
-    .should("have.css", "background-color")
-    .and("eq", "rgb(80, 158, 227)");
-
-  cy.button("Next").click();
-  cy.findByText("Your language is set to English");
-}
-
-export function setupInstance(version) {
-  const companyLabel =
-    version === "v0.41.3.1"
-      ? "Your company or team name"
-      : "Company or team name";
-
-  const finalSetupButton = version === "v0.41.3.1" ? "Next" : "Finish";
-
-  cy.findByLabelText("First name").type("Superuser");
-  cy.findByLabelText("Last name").type("Tableton");
-  cy.findByLabelText("Email").type("admin@metabase.test");
-  cy.findByLabelText(companyLabel).type("Metabase");
-  cy.findByLabelText("Create a password").type("12341234");
-  cy.findByLabelText("Confirm your password").type("12341234");
-  cy.button("Next").click();
-  cy.findByText("Hi, Superuser. Nice to meet you!");
-
-  cy.findByText("I'll add my data later").click();
-  cy.findByText("I'll add my own data later");
-
-  // Collection defaults to on and describes data collection
-  cy.findByText("All collection is completely anonymous.");
-  // turn collection off, which hides data collection description
-  cy.findByLabelText(
-    "Allow Metabase to anonymously collect usage events",
-  ).click();
-  cy.findByText("All collection is completely anonymous.").should("not.exist");
-  cy.findByText(finalSetupButton).click();
-  cy.findByText("Take me to Metabase").click();
-
-  cy.location("pathname").should("eq", "/");
-  cy.contains("Reviews");
-}
+export const version = parseVersionString(Cypress.env("SOURCE_VERSION"));
diff --git a/e2e/test/scenarios/cross-version/target/helpers/cross-version-target-helpers.js b/e2e/test/scenarios/cross-version/target/helpers/cross-version-target-helpers.js
index a6c1895a31b..50008974091 100644
--- a/e2e/test/scenarios/cross-version/target/helpers/cross-version-target-helpers.js
+++ b/e2e/test/scenarios/cross-version/target/helpers/cross-version-target-helpers.js
@@ -1 +1,3 @@
-export const version = Cypress.env("TARGET_VERSION");
+import { parseVersionString } from "e2e/test/scenarios/cross-version/helpers/cross-version-helpers.js";
+
+export const version = parseVersionString(Cypress.env("TARGET_VERSION"));
diff --git a/e2e/test/scenarios/cross-version/target/smoke.cy.spec.js b/e2e/test/scenarios/cross-version/target/smoke.cy.spec.js
index 3ba74bd5d3a..04a7efd5dfd 100644
--- a/e2e/test/scenarios/cross-version/target/smoke.cy.spec.js
+++ b/e2e/test/scenarios/cross-version/target/smoke.cy.spec.js
@@ -1,4 +1,8 @@
-import { version } from "./helpers/cross-version-target-helpers";
+import {
+  assertTimelineData,
+  dismissOkToPlayWithQuestionsModal,
+} from "e2e/test/scenarios/cross-version/helpers/cross-version-helpers";
+import { version } from "e2e/test/scenarios/cross-version/source/helpers/cross-version-source-helpers.js";
 
 describe(`smoke test the migration to the version ${version}`, () => {
   it("should already be set up", () => {
@@ -20,9 +24,7 @@ describe(`smoke test the migration to the version ${version}`, () => {
     cy.findByText("Quarterly Revenue").click();
     cy.wait("@cardQuery");
 
-    // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
-    cy.findByText("It's okay to play around with saved questions");
-    cy.button("Okay").click();
+    dismissOkToPlayWithQuestionsModal(version);
 
     cy.get("circle");
     cy.get(".line");
@@ -30,11 +32,8 @@ describe(`smoke test the migration to the version ${version}`, () => {
     cy.findByText("Goal");
     cy.get(".x-axis-label").invoke("text").should("eq", "Created At");
     cy.get(".y-axis-label").invoke("text").should("eq", "Revenue");
-    cy.get(".x.axis .tick")
-      .should("contain", "Q1 2023")
-      .and("contain", "Q1 2024")
-      .and("contain", "Q1 2025")
-      .and("contain", "Q1 2026");
+
+    assertTimelineData(version);
 
     cy.get(".y.axis .tick")
       .should("contain", "20,000")
-- 
GitLab