Merge pull request #5235 from hallieswan/SWC-6605

SWC-6605
Sage-Bionetworks · Nov 28, 2023 · 880b7f2 · 880b7f2
2 parents 091f516 + 6fdcb49
commit 880b7f2
Show file tree

Hide file tree

Showing 12 changed files with 305 additions and 170 deletions.
diff --git a/.github/workflows/build-test-e2e.yml b/.github/workflows/build-test-e2e.yml
@@ -9,9 +9,9 @@ env:
   PW_ALL_BLOBS_DIR: all-blob-reports
 jobs:
   build:
-    # Run in Sage repo on develop or release- branches
+    # Run in Sage repo on release- branches
     # and on all branches in user-owned forks
-    if: ${{ github.ref_name == 'develop' || startsWith(github.ref_name, 'release-') || github.actor == github.repository_owner }}
+    if: ${{ startsWith(github.ref_name, 'release-') || github.actor == github.repository_owner }}
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v3
@@ -25,9 +25,10 @@ jobs:
           retention-days: 1
   playwright-tests:
     needs: [build]
-    runs-on: ${{ github.repository_owner == 'Sage-Bionetworks' && 'ubuntu-22.04-4core-16GBRAM-150GBSSD' || 'macos-latest' }}
+    runs-on: 'macos-latest'
     timeout-minutes: 60
     strategy:
+      max-parallel: ${{ github.repository_owner == 'Sage-Bionetworks' && 1 || 3 }}
       fail-fast: false
       matrix:
         shard: [1/3, 2/3, 3/3]
@@ -81,8 +82,8 @@ jobs:
         run: colima stop
   merge-reports:
     # Merge reports after playwright-tests, even if some shards have failed
-    # But skip this job if the previous job was cancelled
-    if: ${{ !cancelled() }}
+    # But skip this job if the previous job was cancelled or skipped
+    if: ${{ !cancelled() && needs.playwright-tests.result != 'skipped' }}
     needs: [playwright-tests]
     runs-on: ubuntu-latest
     steps:

diff --git a/e2e/discussions.spec.ts b/e2e/discussions.spec.ts
@@ -1,14 +1,17 @@
 import { Page, expect, test } from '@playwright/test'
+import { defaultExpectTimeout } from '../playwright.config'
 import { testAuth } from './fixtures/authenticatedUserPages'
-import { entityUrlPathname } from './helpers/entities'
 import {
   setupProjectWithPermissions,
   teardownProjectsAndFileHandles,
 } from './helpers/setupTeardown'
 import {
   dismissAlert,
   expectDiscussionPageLoaded,
+  expectDiscussionThreadLoaded,
   getDefaultDiscussionPath,
+  goToDashboardPage,
+  reloadDashboardPage,
 } from './helpers/testUser'
 import { Project } from './helpers/types'
 import { UserPrefix, userConfigs } from './helpers/userConfig'
@@ -58,38 +61,6 @@ const expectThreadTableLoaded = async (
   })
 }
 
-const expectDiscussionThreadLoaded = async (
-  page: Page,
-  threadId: number,
-  threadTitle: string,
-  threadBody: string,
-) => {
-  await testAuth.step('Discussion thread has loaded', async () => {
-    await page.waitForURL(
-      `${entityUrlPathname(userProject.id)}/discussion/threadId=${threadId}`,
-    )
-    await expect(
-      page.getByRole('heading', { name: 'Discussion' }),
-    ).toBeVisible()
-
-    await expect(
-      page.getByRole('button', { name: /show all threads/i }),
-    ).toBeVisible({ timeout: 60_000 })
-    await expect(
-      page.getByRole('button', { name: 'Date Posted' }),
-    ).toBeVisible()
-    await expect(
-      page.getByRole('button', { name: 'Most Recent' }),
-    ).toBeVisible()
-
-    const discussionThread = page.locator(discussionThreadSelector)
-    await expect(discussionThread.getByText(threadTitle)).toBeVisible()
-    await expect(discussionThread.getByText(threadBody)).toBeVisible({
-      timeout: 60_000,
-    })
-  })
-}
-
 const expectThreadReplyVisible = async (
   page: Page,
   threadReply: string,
@@ -102,7 +73,9 @@ const expectThreadReplyVisible = async (
         name: `@${userConfigs[replierPrefix].username}`,
       }),
     ).toBeVisible()
-    await expect(discussionReply.getByText(threadReply)).toBeVisible()
+    await expect(discussionReply.getByText(threadReply)).toBeVisible({
+      timeout: defaultExpectTimeout * 2, // allow extra time for reply to become visible
+    })
   })
 }
 
@@ -153,6 +126,7 @@ test.describe('Discussions', () => {
   })
 
   testAuth.afterAll(async ({ browser }) => {
+    test.slow()
     if (userProject.id) {
       await teardownProjectsAndFileHandles(
         browser,
@@ -173,7 +147,11 @@ test.describe('Discussions', () => {
       const threadReplyEdited = 'An edited reply to the Thread'
 
       await testAuth.step('First user goes to Project', async () => {
-        await userPage.goto(getDefaultDiscussionPath(userProject.id))
+        await goToDashboardPage(
+          userPage,
+          getDefaultDiscussionPath(userProject.id),
+        )
+
         await expectDiscussionPageLoaded(userPage, userProject.id)
       })
 
@@ -226,6 +204,7 @@ test.describe('Discussions', () => {
               threadId,
               threadTitle,
               threadBody,
+              userProject.id,
             )
           })
 
@@ -257,8 +236,12 @@ test.describe('Discussions', () => {
         // retry if the view count hasn't updated
         await expect(async () => {
           // reload is necessary for view count to update
-          await userPage.reload()
-          await expectDiscussionPageLoaded(userPage, userProject.id)
+          await reloadDashboardPage(userPage)
+          await expectDiscussionPageLoaded(
+            userPage,
+            userProject.id,
+            defaultExpectTimeout * 0.5, // use shorter expect timeout, so reload is tried earlier
+          )
           await expectThreadTableLoaded(
             userPage,
             threadTitle,
@@ -270,7 +253,10 @@ test.describe('Discussions', () => {
       })
 
       await testAuth.step('Second user can view discussion', async () => {
-        await validatedUserPage.goto(getDefaultDiscussionPath(userProject.id))
+        await goToDashboardPage(
+          validatedUserPage,
+          getDefaultDiscussionPath(userProject.id),
+        )
         await expectDiscussionPageLoaded(validatedUserPage, userProject.id)
         await expectThreadTableLoaded(
           userPage,
@@ -288,6 +274,7 @@ test.describe('Discussions', () => {
           threadId,
           threadTitle,
           threadBody,
+          userProject.id,
         )
       })
 
@@ -384,6 +371,7 @@ test.describe('Discussions', () => {
               threadId,
               threadTitle,
               threadBody,
+              userProject.id,
             )
           })
 

diff --git a/e2e/favorites.spec.ts b/e2e/favorites.spec.ts
@@ -1,9 +1,13 @@
 import { Page, expect, test } from '@playwright/test'
 import { testAuth } from './fixtures/authenticatedUserPages'
 import { entityUrlPathname } from './helpers/entities'
-import { expectDiscussionPageLoaded, goToDashboard } from './helpers/testUser'
+import {
+  expectDiscussionPageLoaded,
+  goToDashboard,
+  goToDashboardPage,
+  reloadDashboardPage,
+} from './helpers/testUser'
 import { Project } from './helpers/types'
-import { waitForInitialPageLoad } from './helpers/utils'
 
 // Public project owned by swc-e2e-admin on backend dev stack.
 // In the future, consider creating/deleting a new public project within the test instead.
@@ -29,7 +33,6 @@ async function expectProjectPageLoaded(
   projectName: string,
   projectId: string,
 ) {
-  await page.waitForURL(entityUrlPathname(projectId))
   await expect(page.getByRole('heading', { name: projectName })).toBeVisible()
   await expectDiscussionPageLoaded(page, projectId)
 }
@@ -58,8 +61,7 @@ test.describe('Favorites', () => {
       })
 
       await testAuth.step('user goes to public project', async () => {
-        await userPage.goto(entityUrlPathname(PUBLIC_PROJECT.id))
-        await waitForInitialPageLoad(userPage)
+        await goToDashboardPage(userPage, entityUrlPathname(PUBLIC_PROJECT.id))
         await expectProjectPageLoaded(
           userPage,
           PUBLIC_PROJECT.name,
@@ -118,7 +120,7 @@ test.describe('Favorites', () => {
           await goToFavorites(userPage)
 
           // The page needs to be reloaded for the favorites list to update
-          await userPage.reload()
+          await reloadDashboardPage(userPage)
           await expectFavoritesPageLoaded(userPage)
 
           await expect(projectLink).not.toBeVisible()