From e57f1a40d2f43ff60406829ebcdd96f2cf363b19 Mon Sep 17 00:00:00 2001
From: Ryan Laurie <30528226+iethree@users.noreply.github.com>
Date: Wed, 16 Aug 2023 09:10:13 -0600
Subject: [PATCH] Fix DB Connection Test flake (#33177)

---
 .../databases/add-new-database.cy.spec.js     | 80 +++++++++++--------
 .../components/DatabaseForm/DatabaseForm.tsx  |  2 +-
 2 files changed, 47 insertions(+), 35 deletions(-)

diff --git a/e2e/test/scenarios/admin/databases/add-new-database.cy.spec.js b/e2e/test/scenarios/admin/databases/add-new-database.cy.spec.js
index 1e45a83b444..944814a919a 100644
--- a/e2e/test/scenarios/admin/databases/add-new-database.cy.spec.js
+++ b/e2e/test/scenarios/admin/databases/add-new-database.cy.spec.js
@@ -16,6 +16,7 @@ describe("admin > database > add", () => {
     cy.signInAsAdmin();
 
     cy.intercept("POST", "/api/database").as("createDatabase");
+    cy.intercept("GET", "/api/database").as("getDatabases");
 
     cy.visit("/admin/databases/create");
     // should display a setup help card
@@ -58,38 +59,35 @@ describe("admin > database > add", () => {
         }
       });
 
-      // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
-      cy.contains("PostgreSQL").click({ force: true });
-
-      // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
-      cy.findByText("Show advanced options").click();
-
-      // Reproduces (metabase#14334)
-      cy.findByLabelText("Rerun queries for simple explorations").should(
-        "have.attr",
-        "aria-checked",
-        "true",
-      );
-      // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
-      cy.contains("Additional JDBC connection string options");
-      // Reproduces (metabase#17450)
-      cy.findByLabelText("Choose when syncs and scans happen")
-        .click()
-        .should("have.attr", "aria-checked", "true");
+      popover().contains("PostgreSQL").click({ force: true });
 
-      cy.findByLabelText("Never, I'll do this manually if I need to").should(
-        "have.attr",
-        "aria-selected",
-        "true",
-      );
+      cy.findByTestId("database-form").within(() => {
+        cy.findByText("Show advanced options").click();
+        cy.findByLabelText("Rerun queries for simple explorations").should(
+          "have.attr",
+          "aria-checked",
+          "true",
+        );
+        // Reproduces (metabase#14334)
+        cy.findByText("Additional JDBC connection string options");
+        // Reproduces (metabase#17450)
+        cy.findByLabelText("Choose when syncs and scans happen")
+          .click()
+          .should("have.attr", "aria-checked", "true");
+        cy.findByLabelText("Never, I'll do this manually if I need to").should(
+          "have.attr",
+          "aria-selected",
+          "true",
+        );
 
-      // make sure fields needed to connect to the database are properly trimmed (metabase#12972)
-      typeAndBlurUsingLabel("Display name", "QA Postgres12");
-      typeAndBlurUsingLabel("Host", "localhost");
-      typeAndBlurUsingLabel("Port", QA_POSTGRES_PORT);
-      typeAndBlurUsingLabel("Database name", "sample");
-      typeAndBlurUsingLabel("Username", "metabase");
-      typeAndBlurUsingLabel("Password", "metasample123");
+        // make sure fields needed to connect to the database are properly trimmed (metabase#12972)
+        typeAndBlurUsingLabel("Display name", "QA Postgres12");
+        typeAndBlurUsingLabel("Host", "localhost");
+        typeAndBlurUsingLabel("Port", QA_POSTGRES_PORT);
+        typeAndBlurUsingLabel("Database name", "sample");
+        typeAndBlurUsingLabel("Username", "metabase");
+        typeAndBlurUsingLabel("Password", "metasample123");
+      });
 
       const confirmSSLFields = (visible, hidden) => {
         visible.forEach(field => cy.findByText(field));
@@ -148,12 +146,14 @@ describe("admin > database > add", () => {
 
       cy.url().should("match", /\/admin\/databases\?created=true$/);
 
-      // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
-      cy.findByText("We're taking a look at your database!");
-      cy.findByLabelText("close icon").click();
+      waitForDbSync();
+
+      cy.findByRole("dialog").within(() => {
+        cy.findByText("We're taking a look at your database!");
+        cy.icon("close").click();
+      });
 
       cy.findByRole("status").within(() => {
-        cy.findByText("Syncing…");
         cy.findByText("Done!");
       });
 
@@ -368,3 +368,15 @@ function mockSuccessfulDatabaseSave() {
   cy.button("Save").click();
   return cy.wait("@createDatabase");
 }
+
+// we need to check for an indefinite number of these requests because we don't know how many polls it's going to take
+function waitForDbSync(maxRetries = 10) {
+  if (maxRetries === 0) {
+    throw new Error("Timed out waiting for database sync");
+  }
+  cy.wait("@getDatabases").then(({ response }) => {
+    if (response.body.data.some(db => db.initial_sync_status !== "complete")) {
+      waitForDbSync(maxRetries - 1);
+    }
+  });
+}
diff --git a/frontend/src/metabase/databases/components/DatabaseForm/DatabaseForm.tsx b/frontend/src/metabase/databases/components/DatabaseForm/DatabaseForm.tsx
index 3aae416ff02..eb3e78d5210 100644
--- a/frontend/src/metabase/databases/components/DatabaseForm/DatabaseForm.tsx
+++ b/frontend/src/metabase/databases/components/DatabaseForm/DatabaseForm.tsx
@@ -134,7 +134,7 @@ const DatabaseFormBody = ({
   }, [engine, values, isAdvanced]);
 
   return (
-    <Form>
+    <Form data-testid="database-form">
       <DatabaseEngineField
         engineKey={engineKey}
         engines={engines}
-- 
GitLab