diff --git a/e2e/tests/feature-page.spec.ts b/e2e/tests/feature-page.spec.ts index 980e02a60..2538b35a5 100644 --- a/e2e/tests/feature-page.spec.ts +++ b/e2e/tests/feature-page.spec.ts @@ -214,12 +214,19 @@ test('date range changes are preserved in the URL', async ({page}) => { test('redirects for a moved feature', async ({page, browserName}) => { test.skip(browserName === 'webkit', 'Skipping webkit due to flakiness'); - // Wait for the app to fetch the old feature data which triggers the redirect. - const responsePromise = page.waitForResponse(response => - response.url().includes('/v1/features/old-feature'), + const oldResponsePromise = page.waitForResponse( + response => + response.url().includes('/v1/features/old-feature') && + response.request().method() === 'GET', + ); + const newResponsePromise = page.waitForResponse( + response => + response.url().includes('/v1/features/new-feature') && + response.request().method() === 'GET', ); await page.goto('http://localhost:5555/features/old-feature'); - await responsePromise; + await oldResponsePromise; + await newResponsePromise; // Expect the URL to be updated to the new feature's URL. await expect(page).toHaveURL( @@ -248,7 +255,14 @@ test('redirects for a moved feature', async ({page, browserName}) => { }); test('shows gone page for a split feature', async ({page}) => { + // Wait for the API request to complete before checking for redirect. + const responsePromise = page.waitForResponse( + response => + response.url().includes('/v1/features/before-split-feature') && + response.request().method() === 'GET', + ); await page.goto('http://localhost:5555/features/before-split-feature'); + await responsePromise; // Expect to be redirected to the 'feature-gone-split' page. await expect(page).toHaveURL( diff --git a/e2e/tests/login.spec.ts b/e2e/tests/login.spec.ts index fd308bf24..8fe919fd9 100644 --- a/e2e/tests/login.spec.ts +++ b/e2e/tests/login.spec.ts @@ -28,9 +28,14 @@ test.describe('Login Component States', () => { test('displays spinner and is disabled during profile sync', async ({ page, }) => { - // Intercept the pingUser request to introduce a delay. + let resolvePing: (() => void) | undefined; + const pingPromise = new Promise(resolve => { + resolvePing = resolve; + }); + + // Intercept the pingUser request and hold it. await page.route('**/v1/users/me/ping', async route => { - await new Promise(resolve => setTimeout(resolve, 1000)); // Delay to ensure 'syncing' state is capturable + await pingPromise; await route.continue(); }); @@ -58,6 +63,11 @@ test.describe('Login Component States', () => { }, ); + // Resolve the ping to let the sync complete. + if (resolvePing) { + resolvePing(); + } + // Now, wait for the sync to complete and verify the final state. await expect(loginButton.locator('sl-spinner')).toBeHidden(); await expect(loginButton).not.toBeDisabled(); diff --git a/e2e/tests/notification-channels.spec.ts b/e2e/tests/notification-channels.spec.ts index a7d5ffa5f..0d418986d 100644 --- a/e2e/tests/notification-channels.spec.ts +++ b/e2e/tests/notification-channels.spec.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import {test, expect} from '@playwright/test'; +import {test, expect, type Page} from '@playwright/test'; import { loginAsUser, BASE_URL, @@ -23,6 +23,14 @@ import { resetUserData, } from './utils'; +function waitForChannelsRefetch(page: Page) { + return page.waitForResponse( + response => + response.url().includes('/v1/users/me/notification-channels') && + response.request().method() === 'GET', + ); +} + test('redirects unauthenticated user to home and shows toast', async ({ page, }) => { @@ -37,8 +45,19 @@ test.describe('Notification Channels Page', () => { test.beforeEach(async ({page}) => { await resetUserData(); await loginAsUser(page, 'test user 1'); + + // Listen for the initial data load GET request + const initialLoad = page.waitForResponse( + response => + response.url().includes('/v1/users/me/notification-channels') && + response.request().method() === 'GET', + ); + await page.goto(`${BASE_URL}/settings/notification-channels`); await waitForSidebarLoaded(page); + + // Wait for the initial load to complete + await initialLoad; }); test.afterAll(async () => { @@ -114,8 +133,14 @@ test.describe('Notification Channels Page', () => { .getByRole('textbox', {name: 'Slack Webhook URL'}) .fill(webhookUrl); + // Setup refetch listener before click + const refetch = waitForChannelsRefetch(page); + await dialog.getByRole('button', {name: 'Create', exact: true}).click(); + // Wait for refetch to complete + await refetch; + // Verify it's in the list. await expect(dialog.locator('sl-dialog')).not.toBeVisible(); const channelItem = webhookPanel.locator('.channel-item', { @@ -123,6 +148,9 @@ test.describe('Notification Channels Page', () => { }); await expect(channelItem).toBeVisible(); + // Setup refetch listener before delete click + const deleteRefetch = waitForChannelsRefetch(page); + await channelItem.locator('sl-button[label="Delete"]').click(); const deleteDialog = webhookPanel.locator('sl-dialog[open]'); @@ -131,6 +159,9 @@ test.describe('Notification Channels Page', () => { .getByRole('button', {name: 'Delete', exact: true}) .click(); + // Wait for deletion refetch + await deleteRefetch; + // Verify it's gone. await expect(channelItem).not.toBeVisible(); }); @@ -165,8 +196,15 @@ test.describe('Notification Channels Page', () => { await dialog .getByRole('textbox', {name: 'Slack Webhook URL'}) .fill(originalUrl); + + // Setup refetch listener for initial creation + const createRefetch = waitForChannelsRefetch(page); + await dialog.getByRole('button', {name: 'Create', exact: true}).click(); + // Wait for creation refetch + await createRefetch; + // Verify it was created. await expect(dialog.locator('sl-dialog')).not.toBeVisible(); const originalItem = webhookPanel.locator('.channel-item', { @@ -193,8 +231,14 @@ test.describe('Notification Channels Page', () => { .getByRole('textbox', {name: 'Slack Webhook URL'}) .fill(updatedUrl); + // Setup refetch listener for save + const saveRefetch = waitForChannelsRefetch(page); + await dialog.getByRole('button', {name: 'Save', exact: true}).click(); + // Wait for save refetch + await saveRefetch; + // Verify it was updated. await expect(dialog.locator('sl-dialog')).not.toBeVisible(); const updatedItem = webhookPanel.locator('.channel-item', { @@ -209,9 +253,17 @@ test.describe('Notification Channels Page', () => { const deleteDialog = webhookPanel.locator('sl-dialog[open]'); await expect(deleteDialog).toBeVisible(); + + // Setup refetch listener for delete + const deleteRefetch = waitForChannelsRefetch(page); + await deleteDialog .getByRole('button', {name: 'Delete', exact: true}) .click(); + + // Wait for deletion refetch + await deleteRefetch; + await expect(updatedItem).not.toBeVisible(); }); }); diff --git a/e2e/tests/utils.ts b/e2e/tests/utils.ts index b4345f452..79122238e 100644 --- a/e2e/tests/utils.ts +++ b/e2e/tests/utils.ts @@ -261,7 +261,14 @@ export async function freezeAnimations(page: Page) { } export async function goTo404Page(page: Page, query: string): Promise { + const responsePromise = page.waitForResponse( + response => + response.url().includes(`/v1/features/${query}`) && + response.request().method() === 'GET', + ); await page.goto(`${BASE_URL}/features/${query}`); + await responsePromise; + await expect(page).toHaveURL( `${BASE_URL}/errors-404/feature-not-found?q=${query}`, ); diff --git a/util/cmd/load_fake_data/main.go b/util/cmd/load_fake_data/main.go index f4b965f72..d3d1b9963 100644 --- a/util/cmd/load_fake_data/main.go +++ b/util/cmd/load_fake_data/main.go @@ -238,7 +238,7 @@ func (h featuresHelper) Features() []gcpspanner.WebFeature { } func resetTestData(ctx context.Context, spannerClient *gcpspanner.Client, authClient *auth.Client) error { - slog.InfoContext(ctx, "Resetting test user saved searches and bookmarks...") + slog.InfoContext(ctx, "Resetting test user data...") userIDs := make([]string, len(testUserEmails)) for idx, email := range testUserEmails { userID, err := findUserIDByEmail(ctx, email, authClient) @@ -257,6 +257,30 @@ func resetTestData(ctx context.Context, spannerClient *gcpspanner.Client, authCl return nil } + slog.InfoContext(ctx, "Resetting test user saved searches and bookmarks...") + if err := resetSavedSearches(ctx, spannerClient, userIDs); err != nil { + return err + } + slog.InfoContext(ctx, "Deleted saved searches for test users", "count", len(userIDs)) + + slog.InfoContext(ctx, "Resetting test user subscriptions...") + if err := resetSubscriptions(ctx, spannerClient, userIDs); err != nil { + return err + } + slog.InfoContext(ctx, "Test user subscriptions reset.") + + slog.InfoContext(ctx, "Resetting test user notification channels...") + if err := resetNotificationChannels(ctx, spannerClient, userIDs); err != nil { + return err + } + slog.InfoContext(ctx, "Test user notification channels reset.") + + slog.InfoContext(ctx, "Test user data reset complete.") + + return nil +} + +func resetSavedSearches(ctx context.Context, spannerClient *gcpspanner.Client, userIDs []string) error { for _, userID := range userIDs { page, err := spannerClient.ListUserSavedSearches(ctx, userID, 1000, nil) if err != nil { @@ -275,10 +299,11 @@ func resetTestData(ctx context.Context, spannerClient *gcpspanner.Client, authCl } } } - slog.InfoContext(ctx, "Deleted saved searches for test users", "count", len(userIDs)) - // Reset subscriptions for each test user. - slog.InfoContext(ctx, "Resetting test user subscriptions...") + return nil +} + +func resetSubscriptions(ctx context.Context, spannerClient *gcpspanner.Client, userIDs []string) error { for _, userID := range userIDs { // We don't need to handle pagination here, assuming a test user won't have more than 1000 subscriptions. req := gcpspanner.ListSavedSearchSubscriptionsRequest{ @@ -300,9 +325,39 @@ func resetTestData(ctx context.Context, spannerClient *gcpspanner.Client, authCl } } } - slog.InfoContext(ctx, "Test user subscriptions reset.") - slog.InfoContext(ctx, "Test user data reset complete.") + return nil +} + +func resetNotificationChannels(ctx context.Context, spannerClient *gcpspanner.Client, userIDs []string) error { + for _, userID := range userIDs { + if userID == "" { + continue + } + + // List all channels for the user + channels, _, err := spannerClient.ListNotificationChannels(ctx, gcpspanner.ListNotificationChannelsRequest{ + UserID: userID, + PageSize: 1000, + PageToken: nil, + }) + if err != nil { + return fmt.Errorf("failed to list notification channels for user %s: %w", userID, err) + } + + for _, channel := range channels { + // Only delete non-email channels. + if channel.Type == gcpspanner.NotificationChannelTypeEmail { + continue + } + + err := spannerClient.DeleteNotificationChannel(ctx, channel.ID, userID) + if err != nil { + slog.WarnContext(ctx, "failed to delete notification channel, continuing", + "channelID", channel.ID, "userID", userID, "error", err) + } + } + } return nil }