bito-code-review[bot] commented on code in PR #43805:
URL: https://github.com/apache/superset/pull/43805#discussion_r4085371051
##########
superset-frontend/src/dashboard/components/menu/DownloadMenuItems/DownloadMenuItems.test.tsx:
##########
@@ -368,3 +533,258 @@ test('Enabled screenshot items should not show tooltip
icon', () => {
mockIsFeatureEnabled.mockReset();
});
+
+// ---------------------------------------------------------------------------
+// Delivery follows the requester identity, not iframe presence: a guest or
+// anonymous session (no userId) has no email channel, so the toast must not
+// promise one, and the image export (webdriver-rendered, guests cannot open
+// Explore) is hidden.
+// ---------------------------------------------------------------------------
+
+const guestState = { user: {} };
+
+test('guest session: export toast promises auto-download, not an email', async
() => {
+ mockSupersetClient.post.mockResolvedValue({
+ json: { job_id: 'abc' },
+ } as never);
+
+ render(<MenuWrapper />, { useRedux: true, initialState: guestState });
+
+ await userEvent.click(screen.getByText('Export Data to Excel'));
+
+ await waitFor(() =>
+ expect(mockAddInfoToast).toHaveBeenCalledWith(
+ 'Your export is being generated. Please, do not leave the page.',
+ { noDuplicate: true },
+ ),
+ );
+});
+
+test('guest session: Export Images to Excel is hidden even with the webdriver
enabled', () => {
+ enableWebDriverScreenshot();
+
+ render(<MenuWrapper />, { useRedux: true, initialState: guestState });
+
+ expect(screen.getByText('Export Data to Excel')).toBeInTheDocument();
+ expect(screen.queryByText('Export Images to Excel')).not.toBeInTheDocument();
+});
+
+test('logged-in user without an email gets the delivery-neutral toast', async
() => {
+ mockSupersetClient.post.mockResolvedValue({
+ json: { job_id: 'abc' },
+ } as never);
+
+ render(<MenuWrapper />, {
+ useRedux: true,
+ initialState: { user: { userId: 1 } },
+ });
+
+ await userEvent.click(screen.getByText('Export Data to Excel'));
+
+ await waitFor(() =>
+ expect(mockAddInfoToast).toHaveBeenCalledWith(
+ 'Your export is being generated. Please, do not leave the page.',
+ { noDuplicate: true },
+ ),
+ );
+});
+
+test('a "running" status restarts the wait window, so queue delay is not
counted', async () => {
+ jest.useFakeTimers();
+ mockSupersetClient.post.mockResolvedValue({
+ json: { job_id: 'abc' },
+ } as never);
+ // Worker picks the job up on the first poll; still running just past the
+ // original 12 minute deadline; done on the poll after that.
+ mockSupersetClient.get
+ .mockResolvedValueOnce({ json: { status: 'running' } } as never)
+ .mockResolvedValueOnce({ json: { status: 'running' } } as never)
+ .mockResolvedValueOnce({
+ json: {
+ status: 'ready',
+ download_url: '/api/v1/dashboard/export_xlsx/download/abc/',
+ },
+ } as never);
+
+ render(<MenuWrapper />, { useRedux: true, initialState: loggedInState });
+
+ await userEvent.click(screen.getByText('Export Data to Excel'));
+ await waitFor(() => expect(mockSupersetClient.post).toHaveBeenCalled());
+
+ await act(async () => {
+ jest.advanceTimersByTime(3000);
+ });
+ await waitFor(() => expect(mockSupersetClient.get).toHaveBeenCalledTimes(1));
+
+ // t ~= 12m01s: past the enqueue-based deadline, within the restarted one
+ // (running was observed at t=3s). Without the restart this poll would give
+ // up with a danger toast instead of continuing.
+ await act(async () => {
+ jest.advanceTimersByTime(12 * 60 * 1000 - 2000);
+ });
+ await waitFor(() => expect(mockSupersetClient.get).toHaveBeenCalledTimes(2));
+ expect(mockAddDangerToast).not.toHaveBeenCalled();
+
+ await act(async () => {
+ jest.advanceTimersByTime(3000);
+ });
+ await waitFor(() => {
+
expect(lastIframeSrc()).toBe('/api/v1/dashboard/export_xlsx/download/abc/');
+ expect(mockAddSuccessToast).toHaveBeenCalledWith(
+ 'Your export is ready and downloading.',
+ );
+ });
+});
+
+test('a transient poll failure keeps polling and still downloads', async () =>
{
+ jest.useFakeTimers();
+ mockSupersetClient.post.mockResolvedValue({
+ json: { job_id: 'abc' },
+ } as never);
+ mockSupersetClient.get
+ .mockRejectedValueOnce(new Error('network blip'))
+ .mockResolvedValueOnce({
+ json: {
+ status: 'ready',
+ download_url: '/api/v1/dashboard/export_xlsx/download/abc/',
+ },
+ } as never);
+
+ render(<MenuWrapper />, { useRedux: true, initialState: loggedInState });
+
+ await userEvent.click(screen.getByText('Export Data to Excel'));
+ await waitFor(() => expect(mockSupersetClient.post).toHaveBeenCalled());
+
+ await act(async () => {
+ jest.advanceTimersByTime(3000);
+ });
+ await waitFor(() => expect(mockSupersetClient.get).toHaveBeenCalledTimes(1));
+ expect(mockAddDangerToast).not.toHaveBeenCalled();
+
+ await act(async () => {
+ jest.advanceTimersByTime(3000);
+ });
+ await waitFor(() => {
+
expect(lastIframeSrc()).toBe('/api/v1/dashboard/export_xlsx/download/abc/');
+ expect(mockAddSuccessToast).toHaveBeenCalledWith(
+ 'Your export is ready and downloading.',
+ );
+ });
+});
+
+test('poll failures past the deadline give up with an error toast', async ()
=> {
+ jest.useFakeTimers();
+ mockSupersetClient.post.mockResolvedValue({
+ json: { job_id: 'abc' },
+ } as never);
+ mockSupersetClient.get.mockRejectedValue(new Error('server down'));
+
+ render(<MenuWrapper />, { useRedux: true, initialState: loggedInState });
+
+ await userEvent.click(screen.getByText('Export Data to Excel'));
+ await waitFor(() => expect(mockSupersetClient.post).toHaveBeenCalled());
+
+ await act(async () => {
+ jest.advanceTimersByTime(3000);
+ });
+ await waitFor(() => expect(mockSupersetClient.get).toHaveBeenCalledTimes(1));
+
+ // Jump past the 12 minute deadline; the next failing poll must give up.
+ await act(async () => {
+ jest.advanceTimersByTime(13 * 60 * 1000);
+ });
+ await waitFor(() => {
+ expect(mockAddDangerToast).toHaveBeenCalledWith(
+ 'Sorry, something went wrong. Try again later.',
+ );
+ });
+ expect(mockAddSuccessToast).not.toHaveBeenCalledWith(
+ 'Your export is ready and downloading.',
+ );
+});
+
+test('a "ready" status with no download_url is an error, not a fake success',
async () => {
+ jest.useFakeTimers();
+ mockSupersetClient.post.mockResolvedValue({
+ json: { job_id: 'abc' },
+ } as never);
+ mockSupersetClient.get.mockResolvedValue({
+ json: { status: 'ready' },
+ } as never);
+
+ render(<MenuWrapper />, { useRedux: true, initialState: loggedInState });
+
+ await userEvent.click(screen.getByText('Export Data to Excel'));
+ await waitFor(() => expect(mockSupersetClient.post).toHaveBeenCalled());
+
+ await act(async () => {
+ jest.advanceTimersByTime(3000);
+ });
+
+ await waitFor(() => {
+ expect(mockAddDangerToast).toHaveBeenCalledWith(
+ 'Sorry, something went wrong. Try again later.',
+ );
+ });
+ expect(mockAddSuccessToast).not.toHaveBeenCalled();
+ expect(mockAddSuccessToast).not.toHaveBeenCalledWith(
+ 'Your export is ready and downloading.',
+ );
+});
+
+test('unmounting stops the polling loop', async () => {
+ jest.useFakeTimers();
+ mockSupersetClient.post.mockResolvedValue({
+ json: { job_id: 'abc' },
+ } as never);
+ mockSupersetClient.get.mockResolvedValue({
+ json: { status: 'pending' },
+ } as never);
+
+ const { unmount } = render(<MenuWrapper />, {
+ useRedux: true,
+ initialState: loggedInState,
+ });
+
+ await userEvent.click(screen.getByText('Export Data to Excel'));
+ await waitFor(() => expect(mockSupersetClient.post).toHaveBeenCalled());
+
+ await act(async () => {
+ jest.advanceTimersByTime(3000);
+ });
+ await waitFor(() => expect(mockSupersetClient.get).toHaveBeenCalledTimes(1));
+
+ unmount();
+
+ await act(async () => {
+ jest.advanceTimersByTime(30000);
+ });
+ expect(mockSupersetClient.get).toHaveBeenCalledTimes(1);
+});
+
+test('the pending toast is announced once, not re-emitted on every poll',
async () => {
+ jest.useFakeTimers();
Review Comment:
<!-- Bito Reply -->
The suggestion is appropriate for the local context. Since the test suite
uses fake timers and the user event helper relies on `setTimeout`, enabling
`advanceTimers` ensures the internal click delay resolves correctly, preventing
the test from hanging.
**superset-frontend/src/dashboard/components/menu/DownloadMenuItems/DownloadMenuItems.test.tsx**
```
jest.useFakeTimers({ advanceTimers: true });
```
##########
superset-frontend/src/dashboard/components/menu/DownloadMenuItems/DownloadMenuItems.test.tsx:
##########
@@ -368,3 +533,258 @@ test('Enabled screenshot items should not show tooltip
icon', () => {
mockIsFeatureEnabled.mockReset();
});
+
+// ---------------------------------------------------------------------------
+// Delivery follows the requester identity, not iframe presence: a guest or
+// anonymous session (no userId) has no email channel, so the toast must not
+// promise one, and the image export (webdriver-rendered, guests cannot open
+// Explore) is hidden.
+// ---------------------------------------------------------------------------
+
+const guestState = { user: {} };
+
+test('guest session: export toast promises auto-download, not an email', async
() => {
+ mockSupersetClient.post.mockResolvedValue({
+ json: { job_id: 'abc' },
+ } as never);
+
+ render(<MenuWrapper />, { useRedux: true, initialState: guestState });
+
+ await userEvent.click(screen.getByText('Export Data to Excel'));
+
+ await waitFor(() =>
+ expect(mockAddInfoToast).toHaveBeenCalledWith(
+ 'Your export is being generated. Please, do not leave the page.',
+ { noDuplicate: true },
+ ),
+ );
+});
+
+test('guest session: Export Images to Excel is hidden even with the webdriver
enabled', () => {
+ enableWebDriverScreenshot();
+
+ render(<MenuWrapper />, { useRedux: true, initialState: guestState });
+
+ expect(screen.getByText('Export Data to Excel')).toBeInTheDocument();
+ expect(screen.queryByText('Export Images to Excel')).not.toBeInTheDocument();
+});
+
+test('logged-in user without an email gets the delivery-neutral toast', async
() => {
+ mockSupersetClient.post.mockResolvedValue({
+ json: { job_id: 'abc' },
+ } as never);
+
+ render(<MenuWrapper />, {
+ useRedux: true,
+ initialState: { user: { userId: 1 } },
+ });
+
+ await userEvent.click(screen.getByText('Export Data to Excel'));
+
+ await waitFor(() =>
+ expect(mockAddInfoToast).toHaveBeenCalledWith(
+ 'Your export is being generated. Please, do not leave the page.',
+ { noDuplicate: true },
+ ),
+ );
+});
+
+test('a "running" status restarts the wait window, so queue delay is not
counted', async () => {
+ jest.useFakeTimers();
+ mockSupersetClient.post.mockResolvedValue({
+ json: { job_id: 'abc' },
+ } as never);
+ // Worker picks the job up on the first poll; still running just past the
+ // original 12 minute deadline; done on the poll after that.
+ mockSupersetClient.get
+ .mockResolvedValueOnce({ json: { status: 'running' } } as never)
+ .mockResolvedValueOnce({ json: { status: 'running' } } as never)
+ .mockResolvedValueOnce({
+ json: {
+ status: 'ready',
+ download_url: '/api/v1/dashboard/export_xlsx/download/abc/',
+ },
+ } as never);
+
+ render(<MenuWrapper />, { useRedux: true, initialState: loggedInState });
+
+ await userEvent.click(screen.getByText('Export Data to Excel'));
+ await waitFor(() => expect(mockSupersetClient.post).toHaveBeenCalled());
+
+ await act(async () => {
+ jest.advanceTimersByTime(3000);
+ });
+ await waitFor(() => expect(mockSupersetClient.get).toHaveBeenCalledTimes(1));
+
+ // t ~= 12m01s: past the enqueue-based deadline, within the restarted one
+ // (running was observed at t=3s). Without the restart this poll would give
+ // up with a danger toast instead of continuing.
+ await act(async () => {
+ jest.advanceTimersByTime(12 * 60 * 1000 - 2000);
+ });
+ await waitFor(() => expect(mockSupersetClient.get).toHaveBeenCalledTimes(2));
+ expect(mockAddDangerToast).not.toHaveBeenCalled();
+
+ await act(async () => {
+ jest.advanceTimersByTime(3000);
+ });
+ await waitFor(() => {
+
expect(lastIframeSrc()).toBe('/api/v1/dashboard/export_xlsx/download/abc/');
+ expect(mockAddSuccessToast).toHaveBeenCalledWith(
+ 'Your export is ready and downloading.',
+ );
+ });
+});
+
+test('a transient poll failure keeps polling and still downloads', async () =>
{
+ jest.useFakeTimers();
+ mockSupersetClient.post.mockResolvedValue({
+ json: { job_id: 'abc' },
+ } as never);
+ mockSupersetClient.get
+ .mockRejectedValueOnce(new Error('network blip'))
+ .mockResolvedValueOnce({
+ json: {
+ status: 'ready',
+ download_url: '/api/v1/dashboard/export_xlsx/download/abc/',
+ },
+ } as never);
+
+ render(<MenuWrapper />, { useRedux: true, initialState: loggedInState });
+
+ await userEvent.click(screen.getByText('Export Data to Excel'));
+ await waitFor(() => expect(mockSupersetClient.post).toHaveBeenCalled());
+
+ await act(async () => {
+ jest.advanceTimersByTime(3000);
+ });
+ await waitFor(() => expect(mockSupersetClient.get).toHaveBeenCalledTimes(1));
+ expect(mockAddDangerToast).not.toHaveBeenCalled();
+
+ await act(async () => {
+ jest.advanceTimersByTime(3000);
+ });
+ await waitFor(() => {
+
expect(lastIframeSrc()).toBe('/api/v1/dashboard/export_xlsx/download/abc/');
+ expect(mockAddSuccessToast).toHaveBeenCalledWith(
+ 'Your export is ready and downloading.',
+ );
+ });
+});
+
+test('poll failures past the deadline give up with an error toast', async ()
=> {
+ jest.useFakeTimers();
+ mockSupersetClient.post.mockResolvedValue({
+ json: { job_id: 'abc' },
+ } as never);
+ mockSupersetClient.get.mockRejectedValue(new Error('server down'));
+
+ render(<MenuWrapper />, { useRedux: true, initialState: loggedInState });
+
+ await userEvent.click(screen.getByText('Export Data to Excel'));
+ await waitFor(() => expect(mockSupersetClient.post).toHaveBeenCalled());
+
+ await act(async () => {
+ jest.advanceTimersByTime(3000);
+ });
+ await waitFor(() => expect(mockSupersetClient.get).toHaveBeenCalledTimes(1));
+
+ // Jump past the 12 minute deadline; the next failing poll must give up.
+ await act(async () => {
+ jest.advanceTimersByTime(13 * 60 * 1000);
+ });
+ await waitFor(() => {
+ expect(mockAddDangerToast).toHaveBeenCalledWith(
+ 'Sorry, something went wrong. Try again later.',
+ );
+ });
+ expect(mockAddSuccessToast).not.toHaveBeenCalledWith(
+ 'Your export is ready and downloading.',
+ );
+});
+
+test('a "ready" status with no download_url is an error, not a fake success',
async () => {
+ jest.useFakeTimers();
+ mockSupersetClient.post.mockResolvedValue({
+ json: { job_id: 'abc' },
+ } as never);
+ mockSupersetClient.get.mockResolvedValue({
+ json: { status: 'ready' },
+ } as never);
+
+ render(<MenuWrapper />, { useRedux: true, initialState: loggedInState });
+
+ await userEvent.click(screen.getByText('Export Data to Excel'));
+ await waitFor(() => expect(mockSupersetClient.post).toHaveBeenCalled());
+
+ await act(async () => {
+ jest.advanceTimersByTime(3000);
+ });
+
+ await waitFor(() => {
+ expect(mockAddDangerToast).toHaveBeenCalledWith(
+ 'Sorry, something went wrong. Try again later.',
+ );
+ });
+ expect(mockAddSuccessToast).not.toHaveBeenCalled();
+ expect(mockAddSuccessToast).not.toHaveBeenCalledWith(
+ 'Your export is ready and downloading.',
+ );
+});
+
+test('unmounting stops the polling loop', async () => {
+ jest.useFakeTimers();
+ mockSupersetClient.post.mockResolvedValue({
+ json: { job_id: 'abc' },
+ } as never);
+ mockSupersetClient.get.mockResolvedValue({
+ json: { status: 'pending' },
+ } as never);
+
+ const { unmount } = render(<MenuWrapper />, {
+ useRedux: true,
+ initialState: loggedInState,
+ });
+
+ await userEvent.click(screen.getByText('Export Data to Excel'));
+ await waitFor(() => expect(mockSupersetClient.post).toHaveBeenCalled());
+
+ await act(async () => {
+ jest.advanceTimersByTime(3000);
+ });
+ await waitFor(() => expect(mockSupersetClient.get).toHaveBeenCalledTimes(1));
+
+ unmount();
+
+ await act(async () => {
+ jest.advanceTimersByTime(30000);
+ });
+ expect(mockSupersetClient.get).toHaveBeenCalledTimes(1);
+});
+
+test('the pending toast is announced once, not re-emitted on every poll',
async () => {
+ jest.useFakeTimers();
Review Comment:
<!-- Bito Reply -->
Bito provides suggestions based on common patterns and known issues in
libraries like user-event. If the test suite is already passing in CI and you
have confirmed that the specific timing behavior is handled correctly in your
environment, you may choose to ignore this suggestion. You can manage these
types of suggestions through the provided link if you find them consistently
inapplicable to your project's configuration.
**superset-frontend/src/dashboard/components/menu/DownloadMenuItems/DownloadMenuItems.test.tsx**
```
jest.useFakeTimers({ advanceTimers: true });
```
##########
superset-frontend/src/dashboard/components/menu/DownloadMenuItems/DownloadMenuItems.test.tsx:
##########
@@ -368,3 +533,258 @@ test('Enabled screenshot items should not show tooltip
icon', () => {
mockIsFeatureEnabled.mockReset();
});
+
+// ---------------------------------------------------------------------------
+// Delivery follows the requester identity, not iframe presence: a guest or
+// anonymous session (no userId) has no email channel, so the toast must not
+// promise one, and the image export (webdriver-rendered, guests cannot open
+// Explore) is hidden.
+// ---------------------------------------------------------------------------
+
+const guestState = { user: {} };
+
+test('guest session: export toast promises auto-download, not an email', async
() => {
+ mockSupersetClient.post.mockResolvedValue({
+ json: { job_id: 'abc' },
+ } as never);
+
+ render(<MenuWrapper />, { useRedux: true, initialState: guestState });
+
+ await userEvent.click(screen.getByText('Export Data to Excel'));
+
+ await waitFor(() =>
+ expect(mockAddInfoToast).toHaveBeenCalledWith(
+ 'Your export is being generated. Please, do not leave the page.',
+ { noDuplicate: true },
+ ),
+ );
+});
+
+test('guest session: Export Images to Excel is hidden even with the webdriver
enabled', () => {
+ enableWebDriverScreenshot();
+
+ render(<MenuWrapper />, { useRedux: true, initialState: guestState });
+
+ expect(screen.getByText('Export Data to Excel')).toBeInTheDocument();
+ expect(screen.queryByText('Export Images to Excel')).not.toBeInTheDocument();
+});
+
+test('logged-in user without an email gets the delivery-neutral toast', async
() => {
+ mockSupersetClient.post.mockResolvedValue({
+ json: { job_id: 'abc' },
+ } as never);
+
+ render(<MenuWrapper />, {
+ useRedux: true,
+ initialState: { user: { userId: 1 } },
+ });
+
+ await userEvent.click(screen.getByText('Export Data to Excel'));
+
+ await waitFor(() =>
+ expect(mockAddInfoToast).toHaveBeenCalledWith(
+ 'Your export is being generated. Please, do not leave the page.',
+ { noDuplicate: true },
+ ),
+ );
+});
+
+test('a "running" status restarts the wait window, so queue delay is not
counted', async () => {
+ jest.useFakeTimers();
+ mockSupersetClient.post.mockResolvedValue({
+ json: { job_id: 'abc' },
+ } as never);
+ // Worker picks the job up on the first poll; still running just past the
+ // original 12 minute deadline; done on the poll after that.
+ mockSupersetClient.get
+ .mockResolvedValueOnce({ json: { status: 'running' } } as never)
+ .mockResolvedValueOnce({ json: { status: 'running' } } as never)
+ .mockResolvedValueOnce({
+ json: {
+ status: 'ready',
+ download_url: '/api/v1/dashboard/export_xlsx/download/abc/',
+ },
+ } as never);
+
+ render(<MenuWrapper />, { useRedux: true, initialState: loggedInState });
+
+ await userEvent.click(screen.getByText('Export Data to Excel'));
+ await waitFor(() => expect(mockSupersetClient.post).toHaveBeenCalled());
+
+ await act(async () => {
+ jest.advanceTimersByTime(3000);
+ });
+ await waitFor(() => expect(mockSupersetClient.get).toHaveBeenCalledTimes(1));
+
+ // t ~= 12m01s: past the enqueue-based deadline, within the restarted one
+ // (running was observed at t=3s). Without the restart this poll would give
+ // up with a danger toast instead of continuing.
+ await act(async () => {
+ jest.advanceTimersByTime(12 * 60 * 1000 - 2000);
+ });
+ await waitFor(() => expect(mockSupersetClient.get).toHaveBeenCalledTimes(2));
+ expect(mockAddDangerToast).not.toHaveBeenCalled();
+
+ await act(async () => {
+ jest.advanceTimersByTime(3000);
+ });
+ await waitFor(() => {
+
expect(lastIframeSrc()).toBe('/api/v1/dashboard/export_xlsx/download/abc/');
+ expect(mockAddSuccessToast).toHaveBeenCalledWith(
+ 'Your export is ready and downloading.',
+ );
+ });
+});
+
+test('a transient poll failure keeps polling and still downloads', async () =>
{
+ jest.useFakeTimers();
+ mockSupersetClient.post.mockResolvedValue({
+ json: { job_id: 'abc' },
+ } as never);
+ mockSupersetClient.get
+ .mockRejectedValueOnce(new Error('network blip'))
+ .mockResolvedValueOnce({
+ json: {
+ status: 'ready',
+ download_url: '/api/v1/dashboard/export_xlsx/download/abc/',
+ },
+ } as never);
+
+ render(<MenuWrapper />, { useRedux: true, initialState: loggedInState });
+
+ await userEvent.click(screen.getByText('Export Data to Excel'));
+ await waitFor(() => expect(mockSupersetClient.post).toHaveBeenCalled());
+
+ await act(async () => {
+ jest.advanceTimersByTime(3000);
+ });
+ await waitFor(() => expect(mockSupersetClient.get).toHaveBeenCalledTimes(1));
+ expect(mockAddDangerToast).not.toHaveBeenCalled();
+
+ await act(async () => {
+ jest.advanceTimersByTime(3000);
+ });
+ await waitFor(() => {
+
expect(lastIframeSrc()).toBe('/api/v1/dashboard/export_xlsx/download/abc/');
+ expect(mockAddSuccessToast).toHaveBeenCalledWith(
+ 'Your export is ready and downloading.',
+ );
+ });
+});
+
+test('poll failures past the deadline give up with an error toast', async ()
=> {
+ jest.useFakeTimers();
+ mockSupersetClient.post.mockResolvedValue({
+ json: { job_id: 'abc' },
+ } as never);
+ mockSupersetClient.get.mockRejectedValue(new Error('server down'));
+
+ render(<MenuWrapper />, { useRedux: true, initialState: loggedInState });
+
+ await userEvent.click(screen.getByText('Export Data to Excel'));
+ await waitFor(() => expect(mockSupersetClient.post).toHaveBeenCalled());
+
+ await act(async () => {
+ jest.advanceTimersByTime(3000);
+ });
+ await waitFor(() => expect(mockSupersetClient.get).toHaveBeenCalledTimes(1));
+
+ // Jump past the 12 minute deadline; the next failing poll must give up.
+ await act(async () => {
+ jest.advanceTimersByTime(13 * 60 * 1000);
+ });
+ await waitFor(() => {
+ expect(mockAddDangerToast).toHaveBeenCalledWith(
+ 'Sorry, something went wrong. Try again later.',
+ );
+ });
+ expect(mockAddSuccessToast).not.toHaveBeenCalledWith(
+ 'Your export is ready and downloading.',
+ );
+});
+
+test('a "ready" status with no download_url is an error, not a fake success',
async () => {
+ jest.useFakeTimers();
+ mockSupersetClient.post.mockResolvedValue({
+ json: { job_id: 'abc' },
+ } as never);
+ mockSupersetClient.get.mockResolvedValue({
+ json: { status: 'ready' },
+ } as never);
+
+ render(<MenuWrapper />, { useRedux: true, initialState: loggedInState });
+
+ await userEvent.click(screen.getByText('Export Data to Excel'));
+ await waitFor(() => expect(mockSupersetClient.post).toHaveBeenCalled());
+
+ await act(async () => {
+ jest.advanceTimersByTime(3000);
+ });
+
+ await waitFor(() => {
+ expect(mockAddDangerToast).toHaveBeenCalledWith(
+ 'Sorry, something went wrong. Try again later.',
+ );
+ });
+ expect(mockAddSuccessToast).not.toHaveBeenCalled();
+ expect(mockAddSuccessToast).not.toHaveBeenCalledWith(
+ 'Your export is ready and downloading.',
+ );
+});
+
+test('unmounting stops the polling loop', async () => {
+ jest.useFakeTimers();
+ mockSupersetClient.post.mockResolvedValue({
+ json: { job_id: 'abc' },
+ } as never);
+ mockSupersetClient.get.mockResolvedValue({
+ json: { status: 'pending' },
+ } as never);
+
+ const { unmount } = render(<MenuWrapper />, {
+ useRedux: true,
+ initialState: loggedInState,
+ });
+
+ await userEvent.click(screen.getByText('Export Data to Excel'));
+ await waitFor(() => expect(mockSupersetClient.post).toHaveBeenCalled());
+
+ await act(async () => {
+ jest.advanceTimersByTime(3000);
+ });
+ await waitFor(() => expect(mockSupersetClient.get).toHaveBeenCalledTimes(1));
+
+ unmount();
+
+ await act(async () => {
+ jest.advanceTimersByTime(30000);
+ });
+ expect(mockSupersetClient.get).toHaveBeenCalledTimes(1);
+});
+
+test('the pending toast is announced once, not re-emitted on every poll',
async () => {
+ jest.useFakeTimers();
Review Comment:
<!-- Bito Reply -->
The suggestion is appropriate for environments where fake timers are used
with user-event libraries that rely on internal timeouts. By setting
`advanceTimers: true`, you ensure that the fake timer implementation correctly
advances the timers triggered by the user-event interaction, preventing
potential hangs in the test suite.
**superset-frontend/src/dashboard/components/menu/DownloadMenuItems/DownloadMenuItems.test.tsx**
```
jest.useFakeTimers({ advanceTimers: true });
```
##########
superset-frontend/src/dashboard/components/menu/DownloadMenuItems/DownloadMenuItems.test.tsx:
##########
@@ -368,3 +533,258 @@ test('Enabled screenshot items should not show tooltip
icon', () => {
mockIsFeatureEnabled.mockReset();
});
+
+// ---------------------------------------------------------------------------
+// Delivery follows the requester identity, not iframe presence: a guest or
+// anonymous session (no userId) has no email channel, so the toast must not
+// promise one, and the image export (webdriver-rendered, guests cannot open
+// Explore) is hidden.
+// ---------------------------------------------------------------------------
+
+const guestState = { user: {} };
+
+test('guest session: export toast promises auto-download, not an email', async
() => {
+ mockSupersetClient.post.mockResolvedValue({
+ json: { job_id: 'abc' },
+ } as never);
+
+ render(<MenuWrapper />, { useRedux: true, initialState: guestState });
+
+ await userEvent.click(screen.getByText('Export Data to Excel'));
+
+ await waitFor(() =>
+ expect(mockAddInfoToast).toHaveBeenCalledWith(
+ 'Your export is being generated. Please, do not leave the page.',
+ { noDuplicate: true },
+ ),
+ );
+});
+
+test('guest session: Export Images to Excel is hidden even with the webdriver
enabled', () => {
+ enableWebDriverScreenshot();
+
+ render(<MenuWrapper />, { useRedux: true, initialState: guestState });
+
+ expect(screen.getByText('Export Data to Excel')).toBeInTheDocument();
+ expect(screen.queryByText('Export Images to Excel')).not.toBeInTheDocument();
+});
+
+test('logged-in user without an email gets the delivery-neutral toast', async
() => {
+ mockSupersetClient.post.mockResolvedValue({
+ json: { job_id: 'abc' },
+ } as never);
+
+ render(<MenuWrapper />, {
+ useRedux: true,
+ initialState: { user: { userId: 1 } },
+ });
+
+ await userEvent.click(screen.getByText('Export Data to Excel'));
+
+ await waitFor(() =>
+ expect(mockAddInfoToast).toHaveBeenCalledWith(
+ 'Your export is being generated. Please, do not leave the page.',
+ { noDuplicate: true },
+ ),
+ );
+});
+
+test('a "running" status restarts the wait window, so queue delay is not
counted', async () => {
+ jest.useFakeTimers();
+ mockSupersetClient.post.mockResolvedValue({
+ json: { job_id: 'abc' },
+ } as never);
+ // Worker picks the job up on the first poll; still running just past the
+ // original 12 minute deadline; done on the poll after that.
+ mockSupersetClient.get
+ .mockResolvedValueOnce({ json: { status: 'running' } } as never)
+ .mockResolvedValueOnce({ json: { status: 'running' } } as never)
+ .mockResolvedValueOnce({
+ json: {
+ status: 'ready',
+ download_url: '/api/v1/dashboard/export_xlsx/download/abc/',
+ },
+ } as never);
+
+ render(<MenuWrapper />, { useRedux: true, initialState: loggedInState });
+
+ await userEvent.click(screen.getByText('Export Data to Excel'));
+ await waitFor(() => expect(mockSupersetClient.post).toHaveBeenCalled());
+
+ await act(async () => {
+ jest.advanceTimersByTime(3000);
+ });
+ await waitFor(() => expect(mockSupersetClient.get).toHaveBeenCalledTimes(1));
+
+ // t ~= 12m01s: past the enqueue-based deadline, within the restarted one
+ // (running was observed at t=3s). Without the restart this poll would give
+ // up with a danger toast instead of continuing.
+ await act(async () => {
+ jest.advanceTimersByTime(12 * 60 * 1000 - 2000);
+ });
+ await waitFor(() => expect(mockSupersetClient.get).toHaveBeenCalledTimes(2));
+ expect(mockAddDangerToast).not.toHaveBeenCalled();
+
+ await act(async () => {
+ jest.advanceTimersByTime(3000);
+ });
+ await waitFor(() => {
+
expect(lastIframeSrc()).toBe('/api/v1/dashboard/export_xlsx/download/abc/');
+ expect(mockAddSuccessToast).toHaveBeenCalledWith(
+ 'Your export is ready and downloading.',
+ );
+ });
+});
+
+test('a transient poll failure keeps polling and still downloads', async () =>
{
+ jest.useFakeTimers();
+ mockSupersetClient.post.mockResolvedValue({
+ json: { job_id: 'abc' },
+ } as never);
+ mockSupersetClient.get
+ .mockRejectedValueOnce(new Error('network blip'))
+ .mockResolvedValueOnce({
+ json: {
+ status: 'ready',
+ download_url: '/api/v1/dashboard/export_xlsx/download/abc/',
+ },
+ } as never);
+
+ render(<MenuWrapper />, { useRedux: true, initialState: loggedInState });
+
+ await userEvent.click(screen.getByText('Export Data to Excel'));
+ await waitFor(() => expect(mockSupersetClient.post).toHaveBeenCalled());
+
+ await act(async () => {
+ jest.advanceTimersByTime(3000);
+ });
+ await waitFor(() => expect(mockSupersetClient.get).toHaveBeenCalledTimes(1));
+ expect(mockAddDangerToast).not.toHaveBeenCalled();
+
+ await act(async () => {
+ jest.advanceTimersByTime(3000);
+ });
+ await waitFor(() => {
+
expect(lastIframeSrc()).toBe('/api/v1/dashboard/export_xlsx/download/abc/');
+ expect(mockAddSuccessToast).toHaveBeenCalledWith(
+ 'Your export is ready and downloading.',
+ );
+ });
+});
+
+test('poll failures past the deadline give up with an error toast', async ()
=> {
+ jest.useFakeTimers();
+ mockSupersetClient.post.mockResolvedValue({
+ json: { job_id: 'abc' },
+ } as never);
+ mockSupersetClient.get.mockRejectedValue(new Error('server down'));
+
+ render(<MenuWrapper />, { useRedux: true, initialState: loggedInState });
+
+ await userEvent.click(screen.getByText('Export Data to Excel'));
+ await waitFor(() => expect(mockSupersetClient.post).toHaveBeenCalled());
+
+ await act(async () => {
+ jest.advanceTimersByTime(3000);
+ });
+ await waitFor(() => expect(mockSupersetClient.get).toHaveBeenCalledTimes(1));
+
+ // Jump past the 12 minute deadline; the next failing poll must give up.
+ await act(async () => {
+ jest.advanceTimersByTime(13 * 60 * 1000);
+ });
+ await waitFor(() => {
+ expect(mockAddDangerToast).toHaveBeenCalledWith(
+ 'Sorry, something went wrong. Try again later.',
+ );
+ });
+ expect(mockAddSuccessToast).not.toHaveBeenCalledWith(
+ 'Your export is ready and downloading.',
+ );
+});
+
+test('a "ready" status with no download_url is an error, not a fake success',
async () => {
+ jest.useFakeTimers();
+ mockSupersetClient.post.mockResolvedValue({
+ json: { job_id: 'abc' },
+ } as never);
+ mockSupersetClient.get.mockResolvedValue({
+ json: { status: 'ready' },
+ } as never);
+
+ render(<MenuWrapper />, { useRedux: true, initialState: loggedInState });
+
+ await userEvent.click(screen.getByText('Export Data to Excel'));
+ await waitFor(() => expect(mockSupersetClient.post).toHaveBeenCalled());
+
+ await act(async () => {
+ jest.advanceTimersByTime(3000);
+ });
+
+ await waitFor(() => {
+ expect(mockAddDangerToast).toHaveBeenCalledWith(
+ 'Sorry, something went wrong. Try again later.',
+ );
+ });
+ expect(mockAddSuccessToast).not.toHaveBeenCalled();
+ expect(mockAddSuccessToast).not.toHaveBeenCalledWith(
+ 'Your export is ready and downloading.',
+ );
+});
+
+test('unmounting stops the polling loop', async () => {
+ jest.useFakeTimers();
+ mockSupersetClient.post.mockResolvedValue({
+ json: { job_id: 'abc' },
+ } as never);
+ mockSupersetClient.get.mockResolvedValue({
+ json: { status: 'pending' },
+ } as never);
+
+ const { unmount } = render(<MenuWrapper />, {
+ useRedux: true,
+ initialState: loggedInState,
+ });
+
+ await userEvent.click(screen.getByText('Export Data to Excel'));
+ await waitFor(() => expect(mockSupersetClient.post).toHaveBeenCalled());
+
+ await act(async () => {
+ jest.advanceTimersByTime(3000);
+ });
+ await waitFor(() => expect(mockSupersetClient.get).toHaveBeenCalledTimes(1));
+
+ unmount();
+
+ await act(async () => {
+ jest.advanceTimersByTime(30000);
+ });
+ expect(mockSupersetClient.get).toHaveBeenCalledTimes(1);
+});
+
+test('the pending toast is announced once, not re-emitted on every poll',
async () => {
+ jest.useFakeTimers();
Review Comment:
<!-- Bito Reply -->
The suggestion provided by Bito is a valid and recommended practice when
using `jest.useFakeTimers()` in tests that involve `user-event` interactions.
By setting `{ advanceTimers: true }`, you ensure that the fake timers correctly
handle the internal `setTimeout` calls used by `user-event`, preventing
potential test hangs or race conditions. Since your suite is already passing in
CI, applying this change is a safe and proactive way to maintain test stability
as you upgrade or modify your testing environment.
**superset-frontend/src/dashboard/components/menu/DownloadMenuItems/DownloadMenuItems.test.tsx**
```
jest.useFakeTimers({ advanceTimers: true });
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
