From 7ac6e8c424790291d8717f587461f4cfcc5e59f8 Mon Sep 17 00:00:00 2001 From: Nemanja Glumac <31325167+nemanjaglumac@users.noreply.github.com> Date: Fri, 25 Feb 2022 12:55:53 +0100 Subject: [PATCH] Global E2E Config: Solve circular dependency issue in helpers (#20741) * Add `SAMPLE_DB_TABLES` reference * Add notes to the cypress data references * Ensure table ids are correct Early warning mechanism. * Use SAMPLE_DB_TABLES in e2e helpers and custom commands * Use `SAMPLE_DB_ID` in the previously missed helper --- .../e2e/commands/permissions/sandboxTable.js | 6 ++-- frontend/test/__support__/e2e/cypress_data.js | 29 +++++++++++++++++++ .../helpers/e2e-ad-hoc-question-helpers.js | 17 +++++++---- .../helpers/e2e-database-metadata-helpers.js | 4 ++- .../test/snapshot-creators/default.cy.snap.js | 27 ++++++++++++++++- 5 files changed, 74 insertions(+), 9 deletions(-) diff --git a/frontend/test/__support__/e2e/commands/permissions/sandboxTable.js b/frontend/test/__support__/e2e/commands/permissions/sandboxTable.js index f6f2de2bf3f..61731b9160a 100644 --- a/frontend/test/__support__/e2e/commands/permissions/sandboxTable.js +++ b/frontend/test/__support__/e2e/commands/permissions/sandboxTable.js @@ -1,4 +1,6 @@ -import { USER_GROUPS } from "__support__/e2e/cypress_data"; +import { USER_GROUPS, SAMPLE_DB_TABLES } from "__support__/e2e/cypress_data"; + +const { STATIC_ORDERS_ID } = SAMPLE_DB_TABLES; const { COLLECTION_GROUP } = USER_GROUPS; @@ -8,7 +10,7 @@ Cypress.Commands.add( attribute_remappings = {}, card_id = null, group_id = COLLECTION_GROUP, - table_id = 2, + table_id = STATIC_ORDERS_ID, } = {}) => { // Extract the name of the table, as well as `schema` and `db_id` that we'll need later on for `cy.updatePermissionsSchemas()` cy.request("GET", "/api/table").then(({ body: tables }) => { diff --git a/frontend/test/__support__/e2e/cypress_data.js b/frontend/test/__support__/e2e/cypress_data.js index 432486bc761..ddf568cf521 100644 --- a/frontend/test/__support__/e2e/cypress_data.js +++ b/frontend/test/__support__/e2e/cypress_data.js @@ -1,5 +1,34 @@ +/** + * We are keeping the references to most commonly used ids and objects in this file. + * + * Please note that these ids are hard coded and might change if sample database changes in the future! + * For that reason, we have some sanity checks in the `default.cy.snap.js` spec. + * + * SAMPLE_DB_TABLES contains only the references to the four main tables ids in sample database. + * We need these references to avoid circular dependecy issue in custom commands and e2e helpers. + * That is the only place they should be used. NEVER use them in tests! + * + * USER_GROUPS + * Although they are also hard coded, the assertions are put in place in the default snapshot generator + * that would break if the actual ids change. Unlike SAMPLE_DB_TABLES which depend on the order of SQL + * commands used to create the sample database, USER_GROUPS depend on the order in which we create new user groups. + * + * As a general note, whenever you add a new reference to this file, please make sure there is a trigger somewhere + * that would break and alert us if expected and actual values don't match. + */ + export const SAMPLE_DB_ID = 1; +// Use only for e2e helpers and custom commands. Never in e2e tests directly! +export const SAMPLE_DB_TABLES = { + STATIC_PRODUCTS_ID: 1, + STATIC_ORDERS_ID: 2, + STATIC_PEOPLE_ID: 3, + STATIC_REVIEWS_ID: 4, +}; + +// All users and admin groups are the defaults that come with Metabase. +// The rest are the ones we choose the name and the order for. export const USER_GROUPS = { ALL_USERS_GROUP: 1, ADMIN_GROUP: 2, diff --git a/frontend/test/__support__/e2e/helpers/e2e-ad-hoc-question-helpers.js b/frontend/test/__support__/e2e/helpers/e2e-ad-hoc-question-helpers.js index 99c2671fa77..5f52fa7594c 100644 --- a/frontend/test/__support__/e2e/helpers/e2e-ad-hoc-question-helpers.js +++ b/frontend/test/__support__/e2e/helpers/e2e-ad-hoc-question-helpers.js @@ -1,4 +1,11 @@ -import { SAMPLE_DB_ID } from "__support__/e2e/cypress_data"; +import { SAMPLE_DB_ID, SAMPLE_DB_TABLES } from "__support__/e2e/cypress_data"; + +const { + STATIC_ORDERS_ID, + STATIC_PRODUCTS_ID, + STATIC_PEOPLE_ID, + STATIC_REVIEWS_ID, +} = SAMPLE_DB_TABLES; export function adhocQuestionHash(question) { if (question.display) { @@ -56,19 +63,19 @@ export function openTable({ } export function openProductsTable({ mode, limit, callback } = {}) { - return openTable({ table: 1, mode, limit, callback }); + return openTable({ table: STATIC_PRODUCTS_ID, mode, limit, callback }); } export function openOrdersTable({ mode, limit, callback } = {}) { - return openTable({ table: 2, mode, limit, callback }); + return openTable({ table: STATIC_ORDERS_ID, mode, limit, callback }); } export function openPeopleTable({ mode, limit, callback } = {}) { - return openTable({ table: 3, mode, limit, callback }); + return openTable({ table: STATIC_PEOPLE_ID, mode, limit, callback }); } export function openReviewsTable({ mode, limit, callback } = {}) { - return openTable({ table: 4, mode, limit, callback }); + return openTable({ table: STATIC_REVIEWS_ID, mode, limit, callback }); } function getInterceptDetails(question, mode) { diff --git a/frontend/test/__support__/e2e/helpers/e2e-database-metadata-helpers.js b/frontend/test/__support__/e2e/helpers/e2e-database-metadata-helpers.js index 4fd5b07c3f9..960d91dd0ee 100644 --- a/frontend/test/__support__/e2e/helpers/e2e-database-metadata-helpers.js +++ b/frontend/test/__support__/e2e/helpers/e2e-database-metadata-helpers.js @@ -1,3 +1,5 @@ +import { SAMPLE_DB_ID } from "__support__/e2e/cypress_data"; + export function withDatabase(databaseId, f) { cy.request("GET", `/api/database/${databaseId}/metadata`).then(({ body }) => { const database = {}; @@ -14,5 +16,5 @@ export function withDatabase(databaseId, f) { } export function withSampleDatabase(f) { - return withDatabase(1, f); + return withDatabase(SAMPLE_DB_ID, f); } diff --git a/frontend/test/snapshot-creators/default.cy.snap.js b/frontend/test/snapshot-creators/default.cy.snap.js index a433f0f82fd..32aa515acb8 100644 --- a/frontend/test/snapshot-creators/default.cy.snap.js +++ b/frontend/test/snapshot-creators/default.cy.snap.js @@ -1,6 +1,18 @@ import _ from "underscore"; import { snapshot, restore, withSampleDatabase } from "__support__/e2e/cypress"; -import { USERS, USER_GROUPS, SAMPLE_DB_ID } from "__support__/e2e/cypress_data"; +import { + USERS, + USER_GROUPS, + SAMPLE_DB_ID, + SAMPLE_DB_TABLES, +} from "__support__/e2e/cypress_data"; + +const { + STATIC_ORDERS_ID, + STATIC_PRODUCTS_ID, + STATIC_REVIEWS_ID, + STATIC_PEOPLE_ID, +} = SAMPLE_DB_TABLES; const { ALL_USERS_GROUP, @@ -20,6 +32,7 @@ describe("snapshots", () => { addUsersAndGroups(); createCollections(); withSampleDatabase(SAMPLE_DATABASE => { + ensureTableIdsAreCorrect(SAMPLE_DATABASE); createQuestionsAndDashboards(SAMPLE_DATABASE); cy.writeFile( "frontend/test/__support__/e2e/cypress_sample_database.json", @@ -188,6 +201,18 @@ describe("snapshots", () => { }); } + function ensureTableIdsAreCorrect({ + ORDERS_ID, + PRODUCTS_ID, + REVIEWS_ID, + PEOPLE_ID, + }) { + expect(ORDERS_ID).to.eq(STATIC_ORDERS_ID); + expect(PEOPLE_ID).to.eq(STATIC_PEOPLE_ID); + expect(REVIEWS_ID).to.eq(STATIC_REVIEWS_ID); + expect(PRODUCTS_ID).to.eq(STATIC_PRODUCTS_ID); + } + // TODO: It'd be nice to have one file per snapshot. // To do that we need to enforce execution order among them. describe("withSqlite", () => { -- GitLab