.

dagster-io · Apr 26, 2024 · f88888f · f88888f
1 parent cb606c0
commit f88888f
Show file tree

Hide file tree

Showing 8 changed files with 78 additions and 39 deletions.
diff --git a/...ules/dagster-ui/packages/ui-core/src/assets/__tests__/LaunchAssetExecutionButton.test.tsx b/...ules/dagster-ui/packages/ui-core/src/assets/__tests__/LaunchAssetExecutionButton.test.tsx
@@ -1,13 +1,14 @@
 /* eslint-disable jest/expect-expect */
 import {MockedProvider, MockedResponse} from '@apollo/client/testing';
-import {render, screen, waitFor} from '@testing-library/react';
+import {act, render, screen, waitFor} from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
 
 import {CustomAlertProvider} from '../../app/CustomAlertProvider';
 import {CustomConfirmationProvider} from '../../app/CustomConfirmationProvider';
 import {displayNameForAssetKey} from '../../asset-graph/Utils';
 import {LaunchPartitionBackfillMutation} from '../../instance/backfill/types/BackfillUtils.types';
 import {LaunchPipelineExecutionMutation} from '../../runs/types/RunUtils.types';
+import {TestPermissionsProvider} from '../../testing/TestPermissions';
 import {TestProvider} from '../../testing/TestProvider';
 import * as WorkspaceContext from '../../workspace/WorkspaceContext';
 import {ADDITIONAL_REQUIRED_KEYS_WARNING} from '../AssetDefinedInMultipleReposNotice';
@@ -301,8 +302,9 @@ describe('LaunchAssetExecutionButton', () => {
         launchMock,
       });
       await clickMaterializeButton();
-      await screen.findByTestId('choose-partitions-dialog');
-      await userEvent.click(await screen.findByTestId('latest-partition-button'));
+      await act(async () => {
+        await userEvent.click(await screen.findByTestId('latest-partition-button'));
+      });
 
       await expectLaunchExecutesMutationAndCloses('Launch 1 run', launchMock);
     });
@@ -392,7 +394,9 @@ describe('LaunchAssetExecutionButton', () => {
       });
       await clickMaterializeButton();
       await screen.findByTestId('choose-partitions-dialog');
-      await userEvent.click(await screen.findByTestId('latest-partition-button'));
+      await act(async () => {
+        await userEvent.click(await screen.findByTestId('latest-partition-button'));
+      });
       await expectLaunchExecutesMutationAndCloses('Launch 1 run', launchMock);
     });
 
@@ -457,7 +461,9 @@ describe('LaunchAssetExecutionButton', () => {
 
         const rangesAsTags = screen.getByTestId('ranges-as-tags-true-radio');
         await waitFor(async () => expect(rangesAsTags).toBeEnabled());
-        await userEvent.click(rangesAsTags);
+        await act(async () => {
+          await userEvent.click(rangesAsTags);
+        });
         await expectLaunchExecutesMutationAndCloses('Launch 1 run', launchMock);
       });
 
@@ -610,8 +616,10 @@ describe('LaunchAssetExecutionButton', () => {
         await screen.findByText(displayNameForAssetKey(MULTI_ASSET_OUT_2.assetKey)),
       ).toBeDefined();
 
-      // Click Confirm
-      await userEvent.click(await screen.findByTestId('confirm-button-ok'));
+      await act(async () => {
+        // Click Confirm
+        await userEvent.click(await screen.findByTestId('confirm-button-ok'));
+      });
 
       // The launch should contain both MULTI_ASSET_OUT_1 and MULTI_ASSET_OUT_2
       await waitFor(() => expect(launchMock.result).toHaveBeenCalled());
@@ -654,20 +662,31 @@ function renderButton({
 
   render(
     <TestProvider>
-      <CustomConfirmationProvider>
-        <CustomAlertProvider />
-        <MockedProvider mocks={mocks}>
-          <LaunchAssetExecutionButton scope={scope} preferredJobName={preferredJobName} />
-        </MockedProvider>
-      </CustomConfirmationProvider>
+      <TestPermissionsProvider
+        locationOverrides={{
+          'test.py': {
+            canLaunchPipelineExecution: {enabled: true, disabledReason: ''},
+            canLaunchPartitionBackfill: {enabled: true, disabledReason: ''},
+          },
+        }}
+      >
+        <CustomConfirmationProvider>
+          <CustomAlertProvider />
+          <MockedProvider mocks={mocks}>
+            <LaunchAssetExecutionButton scope={scope} preferredJobName={preferredJobName} />
+          </MockedProvider>
+        </CustomConfirmationProvider>
+      </TestPermissionsProvider>
     </TestProvider>,
   );
 }
 
 async function clickMaterializeButton() {
   const materializeButton = await screen.findByTestId('materialize-button');
   expect(materializeButton).toBeVisible();
-  await userEvent.click(materializeButton);
+  await act(async () => {
+    await userEvent.click(materializeButton);
+  });
 }
 
 async function expectErrorShown(msg: string) {
@@ -681,9 +700,11 @@ async function expectLaunchExecutesMutationAndCloses(
     | MockedResponse<LaunchPartitionBackfillMutation>
     | MockedResponse<LaunchPipelineExecutionMutation>,
 ) {
-  const launchButton = await screen.findByTestId('launch-button');
+  const launchButton = await waitFor(() => screen.findByTestId('launch-button'));
   expect(launchButton.textContent).toEqual(label);
-  await userEvent.click(launchButton);
+  await act(async () => {
+    await userEvent.click(launchButton);
+  });
 
   // expect that it triggers the mutation (variables checked by mock matching)
   await waitFor(() => expect(mutation.result).toHaveBeenCalled());

diff --git a/js_modules/dagster-ui/packages/ui-core/src/instance/backfill/BackfillRow.tsx b/js_modules/dagster-ui/packages/ui-core/src/instance/backfill/BackfillRow.tsx
@@ -401,10 +401,13 @@ export const BackfillStatusTag = ({
       return (
         <Box margin={{bottom: 12}}>
           <TagButton
-            onClick={() =>
-              backfill.error &&
-              showCustomAlert({title: 'Error', body: <PythonErrorInfo error={backfill.error} />})
-            }
+            onClick={() => {
+              console.log('backfill error');
+              if (backfill.error) {
+                console.log('Show custom alert');
+                showCustomAlert({title: 'Error', body: <PythonErrorInfo error={backfill.error} />});
+              }
+            }}
           >
             <Tag intent="danger">Failed</Tag>
           </TagButton>
@@ -429,7 +432,6 @@ export const BackfillStatusTag = ({
     case BulkActionStatus.CANCELED:
       return <Tag>Canceled</Tag>;
   }
-  return <span />;
 };
 
 const TagButton = styled.button`

diff --git a/js_modules/dagster-ui/packages/ui-core/src/instance/backfill/__tests__/BackfillPage.test.tsx b/js_modules/dagster-ui/packages/ui-core/src/instance/backfill/__tests__/BackfillPage.test.tsx
@@ -97,7 +97,7 @@ describe('BackfillPage', () => {
     );
 
     expect(await screen.findByTestId('page-loading-indicator')).toBeInTheDocument();
-    expect(await screen.findByText('assetA')).toBeVisible();
+    await waitFor(async () => expect(await screen.findByText('assetA')).toBeVisible());
   });
 
   it('renders the error state', async () => {

diff --git a/...odules/dagster-ui/packages/ui-core/src/instance/backfill/__tests__/BackfillTable.test.tsx b/...odules/dagster-ui/packages/ui-core/src/instance/backfill/__tests__/BackfillTable.test.tsx
@@ -1,5 +1,5 @@
 import {MockedProvider} from '@apollo/client/testing';
-import {render, screen} from '@testing-library/react';
+import {act, render, screen, waitFor} from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
 import {MemoryRouter} from 'react-router-dom';
 
@@ -26,9 +26,11 @@ describe('BackfillTable', () => {
       </MemoryRouter>,
     );
 
-    expect(screen.getByRole('table')).toBeVisible();
+    await waitFor(() => expect(screen.getByRole('table')).toBeVisible());
     const statusLabel = await screen.findByText('Failed');
-    await userEvent.click(statusLabel);
-    expect(Alerting.showCustomAlert).toHaveBeenCalled();
+    await act(async () => {
+      await userEvent.click(statusLabel);
+    });
+    await waitFor(() => expect(Alerting.showCustomAlert).toHaveBeenCalled());
   });
 });
diff --git a/js_modules/dagster-ui/packages/ui-core/src/pipelines/__tests__/PipelineNav.test.tsx b/js_modules/dagster-ui/packages/ui-core/src/pipelines/__tests__/PipelineNav.test.tsx
@@ -1,4 +1,5 @@
-import {render, screen} from '@testing-library/react';
+import {render, screen, waitFor} from '@testing-library/react';
+import {Suspense} from 'react';
 import {MemoryRouter} from 'react-router-dom';
 
 import {PipelineNav} from '../../nav/PipelineNav';
@@ -38,13 +39,15 @@ describe('PipelineNav', () => {
       <JobFeatureProvider>
         <TestPermissionsProvider locationOverrides={locationOverrides}>
           <MemoryRouter initialEntries={['/locations/bar@baz/jobs/foo/overview']}>
-            <PipelineNav repoAddress={repoAddress} />
+            <Suspense>
+              <PipelineNav repoAddress={repoAddress} />
+            </Suspense>
           </MemoryRouter>
         </TestPermissionsProvider>
       </JobFeatureProvider>,
     );
 
-    const launchpadTab = await screen.findByRole('tab', {name: /launchpad/i});
+    const launchpadTab = await waitFor(() => screen.findByRole('tab', {name: /launchpad/i}));
     expect(launchpadTab).toHaveAttribute('aria-disabled', 'false');
   });
 
@@ -59,13 +62,15 @@ describe('PipelineNav', () => {
       <JobFeatureProvider>
         <TestPermissionsProvider locationOverrides={locationOverrides}>
           <MemoryRouter initialEntries={['/locations/bar@baz/jobs/foo/overview']}>
-            <PipelineNav repoAddress={repoAddress} />
+            <Suspense>
+              <PipelineNav repoAddress={repoAddress} />
+            </Suspense>
           </MemoryRouter>
         </TestPermissionsProvider>
       </JobFeatureProvider>,
     );
 
-    const launchpadTab = await screen.findByRole('tab', {name: /launchpad/i});
+    const launchpadTab = await waitFor(() => screen.findByRole('tab', {name: /launchpad/i}));
     expect(launchpadTab).toHaveAttribute('aria-disabled', 'true');
   });
 });
diff --git a/js_modules/dagster-ui/packages/ui-core/src/pipelines/__tests__/PipelineRoot.test.tsx b/js_modules/dagster-ui/packages/ui-core/src/pipelines/__tests__/PipelineRoot.test.tsx
@@ -1,4 +1,5 @@
-import {render, screen} from '@testing-library/react';
+import {render, screen, waitFor} from '@testing-library/react';
+import {Suspense} from 'react';
 import {MemoryRouter} from 'react-router-dom';
 
 import {TestPermissionsProvider} from '../../testing/TestPermissions';
@@ -53,7 +54,9 @@ describe('PipelineRoot', () => {
     render(
       <JobFeatureProvider>
         <MemoryRouter initialEntries={[path]}>
-          <PipelineRoot repoAddress={repoAddress} />
+          <Suspense>
+            <PipelineRoot repoAddress={repoAddress} />
+          </Suspense>
         </MemoryRouter>
       </JobFeatureProvider>,
     );
@@ -73,7 +76,9 @@ describe('PipelineRoot', () => {
       <JobFeatureProvider>
         <TestPermissionsProvider locationOverrides={locationPermissions}>
           <MemoryRouter initialEntries={[`${path}/playground`]}>
-            <PipelineRoot repoAddress={repoAddress} />
+            <Suspense>
+              <PipelineRoot repoAddress={repoAddress} />
+            </Suspense>
           </MemoryRouter>
         </TestPermissionsProvider>
       </JobFeatureProvider>,
@@ -94,13 +99,17 @@ describe('PipelineRoot', () => {
       <JobFeatureProvider>
         <TestPermissionsProvider locationOverrides={locationPermissions}>
           <MemoryRouter initialEntries={[`${path}/playground`]}>
-            <PipelineRoot repoAddress={repoAddress} />
+            <Suspense>
+              <PipelineRoot repoAddress={repoAddress} />
+            </Suspense>
           </MemoryRouter>
         </TestPermissionsProvider>
       </JobFeatureProvider>,
     );
 
-    const overviewDummy = await screen.findByText(/pipeline or job disambiguation placeholder/i);
+    const overviewDummy = await waitFor(() =>
+      screen.findByText(/pipeline or job disambiguation placeholder/i),
+    );
     expect(overviewDummy).toBeVisible();
   });
 });
diff --git a/js_modules/dagster-ui/packages/ui-core/src/testing/TestPermissions.tsx b/js_modules/dagster-ui/packages/ui-core/src/testing/TestPermissions.tsx
@@ -30,10 +30,10 @@ export const TestPermissionsProvider = (props: Props) => {
     };
   }, [locationOverrides, unscopedOverrides]);
 
-  const promise = React.useMemo(() => {
+  const wrapped = React.useMemo(() => {
     const p = Promise.resolve(value);
     return wrapPromise(p);
   }, [value]);
 
-  return <PermissionsContext.Provider value={promise}>{children}</PermissionsContext.Provider>;
+  return <PermissionsContext.Provider value={wrapped}>{children}</PermissionsContext.Provider>;
 };
diff --git a/js_modules/dagster-ui/packages/ui-core/src/ui/__tests__/autolinking.test.tsx b/js_modules/dagster-ui/packages/ui-core/src/ui/__tests__/autolinking.test.tsx
@@ -44,7 +44,7 @@ describe('autolinking', () => {
 
     const rendered = render(<Test message={message} useIdleCallback={false} />);
 
-    waitFor(() => expect(screen.getByRole('link')).toBeDefined());
+    waitFor(() => expect(screen.getAllByRole('link')).toBeDefined());
 
     const links = screen.getAllByRole('link');
     const hrefs = Array.from(links).map((el) => el.getAttribute('href'));