From ebb5500c15e1ab652e1ab5eb493207262c6d052d Mon Sep 17 00:00:00 2001
From: Kamil Mielnik <kamil@kamilmielnik.com>
Date: Tue, 20 Feb 2024 13:29:31 +0700
Subject: [PATCH] Fix - Flaky QueryBuilder tests (#38860)
* Use fetchMock.flush() to prevent tests from interfering with each other
* Revert "Use fetchMock.flush() to prevent tests from interfering with each other"
This reverts commit 03f1277cf2bf481ac36db212e15b4f29511a386b.
* Add typing for requests in the redux store
* Fix QueryBuilder flakes by introducing waitForLoadingRequests
* Simplify interface
* Add mocks for requests in redux store, update types
* Simplify code
* Increase timeout
---
.../advanced_permissions/types.ts | 4 +--
.../metabase-enterprise/sandboxes/types.ts | 4 +--
frontend/src/metabase-types/store/index.ts | 3 +-
.../metabase-types/store/mocks/requests.ts | 8 ++++++
.../src/metabase-types/store/mocks/state.ts | 2 ++
frontend/src/metabase-types/store/requests.ts | 19 ++++++++++++-
frontend/src/metabase-types/store/state.ts | 2 ++
.../dashboard/actions/cards.unit.spec.ts | 1 -
.../query_builder/containers/test-utils.tsx | 28 +++++++++++++++++--
9 files changed, 62 insertions(+), 9 deletions(-)
create mode 100644 frontend/src/metabase-types/store/mocks/requests.ts
diff --git a/enterprise/frontend/src/metabase-enterprise/advanced_permissions/types.ts b/enterprise/frontend/src/metabase-enterprise/advanced_permissions/types.ts
index 5600ef333d0..929f2061ebc 100644
--- a/enterprise/frontend/src/metabase-enterprise/advanced_permissions/types.ts
+++ b/enterprise/frontend/src/metabase-enterprise/advanced_permissions/types.ts
@@ -1,7 +1,7 @@
import type { DatabaseId, GroupId, Impersonation } from "metabase-types/api";
import type { PartialBy } from "metabase/core/types";
-import type { RequestState } from "metabase-types/store/requests";
+import type { RequestsState, RequestState } from "metabase-types/store";
import type { EnterpriseState } from "metabase-enterprise/shared/types";
import type { EnterpriseSharedState } from "metabase-enterprise/shared/reducer";
import type { AdvancedPermissionsState } from "./reducer";
@@ -9,7 +9,7 @@ import type { AdvancedPermissionsState } from "./reducer";
export type ImpersonationParams = { groupId: GroupId; databaseId: DatabaseId };
export interface AdvancedPermissionsStoreState extends EnterpriseState {
- requests: {
+ requests: RequestsState & {
plugins: {
advancedPermissionsPlugin: {
policies: Record<string, RequestState>;
diff --git a/enterprise/frontend/src/metabase-enterprise/sandboxes/types.ts b/enterprise/frontend/src/metabase-enterprise/sandboxes/types.ts
index a8e331ea8cb..cb232410b2d 100644
--- a/enterprise/frontend/src/metabase-enterprise/sandboxes/types.ts
+++ b/enterprise/frontend/src/metabase-enterprise/sandboxes/types.ts
@@ -1,12 +1,12 @@
import type { EnterpriseSharedState } from "metabase-enterprise/shared/reducer";
import type { EnterpriseState } from "metabase-enterprise/shared/types";
import type { GroupTableAccessPolicy } from "metabase-types/api";
-import type { RequestState } from "metabase-types/store/requests";
+import type { RequestsState, RequestState } from "metabase-types/store";
export type GroupTableAccessPolicyParams = { groupId: string; tableId: string };
export interface SandboxesState extends EnterpriseState {
- requests: {
+ requests: RequestsState & {
plugins: {
sandboxesPlugin: {
policies: Record<string, RequestState>;
diff --git a/frontend/src/metabase-types/store/index.ts b/frontend/src/metabase-types/store/index.ts
index 4bce48841c9..da52d6d1589 100644
--- a/frontend/src/metabase-types/store/index.ts
+++ b/frontend/src/metabase-types/store/index.ts
@@ -6,7 +6,8 @@ export * from "./dashboard";
export * from "./embed";
export * from "./entities";
export * from "./metabot";
-export * from "./settings";
export * from "./qb";
+export * from "./requests";
+export * from "./settings";
export * from "./setup";
export * from "./state";
diff --git a/frontend/src/metabase-types/store/mocks/requests.ts b/frontend/src/metabase-types/store/mocks/requests.ts
new file mode 100644
index 00000000000..70bba8a0145
--- /dev/null
+++ b/frontend/src/metabase-types/store/mocks/requests.ts
@@ -0,0 +1,8 @@
+import type { RequestsState } from "../requests";
+
+export const createMockRequestsState = (): RequestsState => {
+ return {
+ entities: {},
+ plugins: {},
+ };
+};
diff --git a/frontend/src/metabase-types/store/mocks/state.ts b/frontend/src/metabase-types/store/mocks/state.ts
index 2e12f7df64e..96e324120fc 100644
--- a/frontend/src/metabase-types/store/mocks/state.ts
+++ b/frontend/src/metabase-types/store/mocks/state.ts
@@ -9,6 +9,7 @@ import { createMockEmbedState } from "./embed";
import { createMockMetabotState } from "./metabot";
import { createMockParametersState } from "./parameters";
import { createMockQueryBuilderState } from "./qb";
+import { createMockRequestsState } from "./requests";
import { createMockRoutingState } from "./routing";
import { createMockSettingsState } from "./settings";
import { createMockSetupState } from "./setup";
@@ -28,6 +29,7 @@ export const createMockState = (
metabot: createMockMetabotState(),
parameters: createMockParametersState(),
qb: createMockQueryBuilderState(),
+ requests: createMockRequestsState(),
routing: createMockRoutingState(),
settings: createMockSettingsState(),
setup: createMockSetupState(),
diff --git a/frontend/src/metabase-types/store/requests.ts b/frontend/src/metabase-types/store/requests.ts
index 5a0770611ed..4a85f7c193a 100644
--- a/frontend/src/metabase-types/store/requests.ts
+++ b/frontend/src/metabase-types/store/requests.ts
@@ -1,7 +1,24 @@
+type EntityKey = string;
+
+type QueryKey = string;
+
+type RequestType = string; // only "fetch"?
+
+// See initialRequestState in metabase/redux/requests.js
export interface RequestState {
loading: boolean;
loaded: boolean;
fetched: boolean;
- error: any | null;
+ error: unknown | null;
_isRequestState: true;
}
+
+type RequestsGroupState = Record<
+ EntityKey,
+ Record<QueryKey, Record<RequestType, RequestState>>
+>;
+
+export type RequestsState = {
+ plugins: RequestsGroupState;
+ entities: RequestsGroupState;
+};
diff --git a/frontend/src/metabase-types/store/state.ts b/frontend/src/metabase-types/store/state.ts
index 6cd47a440ad..6aebf59cf2c 100644
--- a/frontend/src/metabase-types/store/state.ts
+++ b/frontend/src/metabase-types/store/state.ts
@@ -9,6 +9,7 @@ import type { EntitiesState } from "./entities";
import type { MetabotState } from "./metabot";
import type { QueryBuilderState } from "./qb";
import type { ParametersState } from "./parameters";
+import type { RequestsState } from "./requests";
import type { SettingsState } from "./settings";
import type { SetupState } from "./setup";
import type { FileUploadState } from "./upload";
@@ -25,6 +26,7 @@ export interface State {
metabot: MetabotState;
parameters: ParametersState;
qb: QueryBuilderState;
+ requests: RequestsState;
routing: RouterState;
settings: SettingsState;
setup: SetupState;
diff --git a/frontend/src/metabase/dashboard/actions/cards.unit.spec.ts b/frontend/src/metabase/dashboard/actions/cards.unit.spec.ts
index 001968043fa..e9a8e3297ef 100644
--- a/frontend/src/metabase/dashboard/actions/cards.unit.spec.ts
+++ b/frontend/src/metabase/dashboard/actions/cards.unit.spec.ts
@@ -164,7 +164,6 @@ function setup({
dashcards: _.indexBy(dashcards, "id"),
});
- // @ts-expect-error we need better redux test tooling
const store = getStore(
mainReducers,
createMockState({ dashboard: dashboardState }),
diff --git a/frontend/src/metabase/query_builder/containers/test-utils.tsx b/frontend/src/metabase/query_builder/containers/test-utils.tsx
index 3f73a10d7b8..a177ad67d17 100644
--- a/frontend/src/metabase/query_builder/containers/test-utils.tsx
+++ b/frontend/src/metabase/query_builder/containers/test-utils.tsx
@@ -45,6 +45,7 @@ import {
SAMPLE_DB_ID,
createSampleDatabase,
} from "metabase-types/api/mocks/presets";
+import type { State, RequestState } from "metabase-types/store";
import NewItemMenu from "metabase/containers/NewItemMenu";
import { LOAD_COMPLETE_FAVICON } from "metabase/hoc/Favicon";
import { serializeCardForUrl } from "metabase/lib/card";
@@ -252,7 +253,11 @@ export const setup = async ({
const mockEventListener = jest.spyOn(window, "addEventListener");
- const { container, history } = renderWithProviders(
+ const {
+ store: { getState },
+ container,
+ history,
+ } = renderWithProviders(
<Route>
<Route path="/" component={TestHome} />
<Route path="/model">
@@ -278,7 +283,7 @@ export const setup = async ({
},
);
- await waitForLoaderToBeRemoved();
+ await waitForLoadingRequests(getState);
return {
container,
@@ -287,6 +292,25 @@ export const setup = async ({
};
};
+const waitForLoadingRequests = async (getState: () => State) => {
+ await waitFor(
+ () => {
+ const requests = getRequests(getState());
+ const areRequestsLoading = requests.some(request => request.loading);
+ expect(areRequestsLoading).toBe(false);
+ },
+ { timeout: 5000 },
+ );
+};
+
+const getRequests = (state: State): RequestState[] => {
+ return Object.values(state.requests).flatMap(group =>
+ Object.values(group).flatMap(entity =>
+ Object.values(entity).flatMap(request => Object.values(request)),
+ ),
+ );
+};
+
export const startNewNotebookModel = async () => {
userEvent.click(screen.getByText("Use the notebook editor"));
await waitForLoaderToBeRemoved();
--
GitLab