Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SWC-6605 #5235

Merged
merged 12 commits into from
Nov 28, 2023
Merged

SWC-6605 #5235

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
da4743b
SWC-6554: skipped is not the same as cancelled - ensure that job is s…
hallieswan Nov 2, 2023
90eff2c
SWC-6605: small tweaks to decrease flake
hallieswan Nov 15, 2023
f4e90fa
SWC-6605: increase test, worker, and expect timeouts in CI to handle …
hallieswan Nov 16, 2023
bec84a2
SWC-6605: await key network responses when logging in and loading das…
hallieswan Nov 16, 2023
ae69fa9
SWC-6605: increase timeout for slow tests
hallieswan Nov 16, 2023
dce739d
SWC-6605: bump playwright version, which fixes File's lastModified ti…
hallieswan Nov 17, 2023
fb22de1
SWC-6605: delete project via ui, otherwise delete via API
hallieswan Nov 17, 2023
62cb016
SWC-6605: allow time for teams afterAll to finish
hallieswan Nov 17, 2023
7452af7
SWC-6605: attempt fixes for common failure points
hallieswan Nov 22, 2023
7812a0f
SWC-6605: use macOS runner in Sage repo, but limit when workflow is r…
hallieswan Nov 27, 2023
c2f3cd7
SWC-6605: adjust timeouts based on test runs
hallieswan Nov 27, 2023
6fdcb49
SWC-6605: fix handling of intermittently appearing file upload alert
hallieswan Nov 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions .github/workflows/build-test-e2e.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only run e2e workflow for release-** branches in Sage repo, so macOS runners are available for other teams' workflows

runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
Expand All @@ -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'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use macOS runner for all jobs to decrease flakiness vs linux runners -- see 10x passing runs using macOS runner here.

timeout-minutes: 60
strategy:
max-parallel: ${{ github.repository_owner == 'Sage-Bionetworks' && 1 || 3 }}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only run one shard at a time in the Sage repo, so macOS runners are available for other teams' workflows

fail-fast: false
matrix:
shard: [1/3, 2/3, 3/3]
Expand Down Expand Up @@ -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' }}
Comment on lines +85 to +86
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"skipped" !== "cancelled" -- skip the job if the workflow was "skipped", as in this case.

needs: [playwright-tests]
runs-on: ubuntu-latest
steps:
Expand Down
64 changes: 26 additions & 38 deletions e2e/discussions.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -58,38 +61,6 @@ const expectThreadTableLoaded = async (
})
}

const expectDiscussionThreadLoaded = async (
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved into testUser.ts, so this can be reused in other tests

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,
Expand All @@ -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
})
})
}

Expand Down Expand Up @@ -153,6 +126,7 @@ test.describe('Discussions', () => {
})

testAuth.afterAll(async ({ browser }) => {
test.slow()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Allow extra time for the project and file handle to be cleaned up

if (userProject.id) {
await teardownProjectsAndFileHandles(
browser,
Expand All @@ -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),
)

Comment on lines -176 to +154
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Playwright's goto method waits for the 'load' event, which may be fired before SWC is ready for interaction. Instead, use custom goTo*** methods that wait for SWC to fully load, e.g. SynapseNavDrawer is loaded for authenticated users.

await expectDiscussionPageLoaded(userPage, userProject.id)
})

Expand Down Expand Up @@ -226,6 +204,7 @@ test.describe('Discussions', () => {
threadId,
threadTitle,
threadBody,
userProject.id,
)
})

Expand Down Expand Up @@ -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)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to the custom goTo*** methods, use a custom reload*** method that waits for SWC to fully load

await expectDiscussionPageLoaded(
userPage,
userProject.id,
defaultExpectTimeout * 0.5, // use shorter expect timeout, so reload is tried earlier
)
await expectThreadTableLoaded(
userPage,
threadTitle,
Expand All @@ -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,
Expand All @@ -288,6 +274,7 @@ test.describe('Discussions', () => {
threadId,
threadTitle,
threadBody,
userProject.id,
)
})

Expand Down Expand Up @@ -384,6 +371,7 @@ test.describe('Discussions', () => {
threadId,
threadTitle,
threadBody,
userProject.id,
)
})

Expand Down
14 changes: 8 additions & 6 deletions e2e/favorites.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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)
}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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()
Expand Down
Loading