e2e: Fix flaky login and redirect tests

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(

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<void>(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();

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,15 +133,24 @@ 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', {
       hasText: webhookName,
     });
     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();
   });
 });

e2e/tests/utils.ts

@@ -261,7 +261,14 @@ export async function freezeAnimations(page: Page) {
 }
 
 export async function goTo404Page(page: Page, query: string): Promise<void> {
+  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}`,
   );

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
 }