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.

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-7242 #5630

Open
wants to merge 11 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 8 additions & 8 deletions .github/workflows/build-test-e2e.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ env:
BUILD_ARTIFACT_NAME: swc-war
BUILD_DIR: target
BUILD_NAME: portal-develop-SNAPSHOT.war
NODE_VERSION: 18.16.0
NODE_VERSION: 22
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Update Node to 22 (latest LTS) since it includes the latest version of corepack, which we will use to install pnpm.

PW_ALL_BLOBS_DIR: all-blob-reports
jobs:
lint-e2e:
Expand All @@ -19,9 +19,9 @@ jobs:
with:
node-version: ${{ env.NODE_VERSION }}
- name: Install dependencies
run: yarn install --frozen-lockfile
run: pnpm install --frozen-lockfile
- name: Lint E2E tests
run: yarn e2e:lint
run: pnpm e2e:lint
build:
# Run in Sage repo on develop or release- branches
# and on all branches in user-owned forks
Expand Down Expand Up @@ -98,9 +98,9 @@ jobs:
with:
node-version: ${{ env.NODE_VERSION }}
- name: Install dependencies
run: yarn install --frozen-lockfile --network-timeout 1000000
run: pnpm install --frozen-lockfile --network-timeout 1000000
- name: Install Playwright Browsers
run: yarn playwright install --with-deps
run: pnpm playwright install --with-deps
- name: Check for common errors
run: |
if [ ! -e "/$(pwd)/${{ env.BUILD_DIR }}/${{ env.BUILD_NAME }}" ]; then
Expand Down Expand Up @@ -135,7 +135,7 @@ jobs:
env:
ADMIN_PAT: ${{ secrets.ADMIN_PAT }}
TRACE_TOGGLE: ${{ github.repository_owner == 'Sage-Bionetworks' && 'on' || 'off'}}
run: yarn playwright test --shard ${{ matrix.shard }} --trace=${{ env.TRACE_TOGGLE }}
run: pnpm playwright test --shard ${{ matrix.shard }} --trace=${{ env.TRACE_TOGGLE }}
- name: Assume AWS Role
if: ${{ github.repository_owner == 'Sage-Bionetworks' && always() }}
uses: aws-actions/configure-aws-credentials@v2
Expand Down Expand Up @@ -173,14 +173,14 @@ jobs:
with:
node-version: ${{ env.NODE_VERSION }}
- name: Install dependencies
run: yarn install --frozen-lockfile
run: pnpm install --frozen-lockfile
- name: Download blob reports from GitHub Actions Artifacts
uses: actions/download-artifact@v3
with:
name: ${{ env.PW_ALL_BLOBS_DIR }}
path: ${{ env.PW_ALL_BLOBS_DIR }}
- name: Merge into HTML Report
run: yarn playwright merge-reports --reporter html ./"${{ env.PW_ALL_BLOBS_DIR }}"
run: pnpm playwright merge-reports --reporter html ./"${{ env.PW_ALL_BLOBS_DIR }}"
- name: Upload HTML report
uses: actions/upload-artifact@v3
with:
Expand Down
2 changes: 1 addition & 1 deletion .husky/pre-commit
Original file line number Diff line number Diff line change
@@ -1 +1 @@
yarn lint-staged
pnpm lint-staged
1 change: 1 addition & 0 deletions .prettierignore
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ src/main/webapp/research/**/*

# Other checked-in files that can/should not be formatted.
yarn.lock
pnpm-lock.yaml
.prettierignore
.gitignore
.git-blame-ignore-revs
Expand Down
60 changes: 17 additions & 43 deletions e2e/files.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,37 +120,41 @@ const uploadFile = async (
: 'Upload a New Version of File'
await page.getByRole('button', { name: uploadButtonText }).click()

await expect(page.getByRole('link', { name: 'Upload File' })).toBeVisible()
await expect(page.getByRole('link', { name: 'Link to URL' })).toBeVisible()
await expect(page.getByRole('button', { name: 'Browse...' })).toBeVisible()
await expect(page.getByRole('tab', { name: 'Upload File' })).toBeVisible()
await expect(page.getByRole('tab', { name: 'Link to URL' })).toBeVisible()
await expect(page.getByText('Click to upload')).toBeVisible()
})

await testAuth.step('choose file', async () => {
// Ensure that upload type is specified before attempting upload
// to prevent upload error: "Unsupported external upload type specified: undefined"
await expect(
page.getByText(/all uploaded files will be stored in synapsestorage/i),
page.getByText(/All uploaded files will be stored in Synapse storage/i),
).toBeVisible()

const fileChooserPromise = page.waitForEvent('filechooser')
await page.getByRole('button', { name: 'Browse...' }).click()
await page.getByText('Click to upload').click()
if (uploadType === 'initialUpload') {
await page
.getByRole('menu')
.getByRole('link')
.getByRole('menuitem')
.filter({ hasText: 'Files' })
.click()
}
const fileChooser = await fileChooserPromise
await fileChooser.setFiles(path.join(__dirname, filePath))
await fileChooser.setFiles(path.join(import.meta.dirname, filePath))
})

await testAuth.step('wait for file upload modal to close', async () => {
await expect(page.getByText('Initializing......')).not.toBeVisible()
await expect(
page.getByRole('heading', { name: 'Upload or Link to File' }),
).not.toBeVisible()
})
await testAuth.step(
'wait for file upload to finish and close modal',
async () => {
await expect(page.getByText('Uploaded 1 Item')).toBeVisible()
await page.getByRole('button', { name: 'Finish' }).click()
await expect(
page.getByRole('heading', { name: 'Upload or Link to File' }),
).not.toBeVisible()
},
)

await testAuth.step('ensure there was not an upload error', async () => {
const uploadError = page.getByRole('heading', { name: 'Upload Error' })
Expand All @@ -161,12 +165,6 @@ const uploadFile = async (
})
}

const dismissFileUploadAlert = async (page: Page) => {
await testAuth.step('dismiss file upload alert', async () => {
await dismissAlert(page, 'File successfully uploaded')
})
}

const getFileMD5 = async (page: Page) => {
return await testAuth.step('get file MD5', async () => {
const row = page.getByRole('row').filter({ hasText: 'MD5' })
Expand Down Expand Up @@ -364,7 +362,6 @@ testAuth.describe('Files', () => {

await testAuth.step('upload file', async () => {
await uploadFile(userPage, filePath, 'initialUpload')
await dismissFileUploadAlert(userPage)
})

const fileLink = await testAuth.step('get link to file', async () => {
Expand Down Expand Up @@ -408,32 +405,9 @@ testAuth.describe('Files', () => {
},
)

// Upload success alert intermittently appears when re-uploading the same file
await testAuth.step(
'dismiss file upload alert for re-uploaded file, if visible',
async () => {
if (
await userPage.getByText('File successfully uploaded').isVisible()
) {
await dismissFileUploadAlert(userPage)
}
},
)

await testAuth.step('upload a new file', async () => {
await uploadFile(userPage, updatedFilePath, 'newVersion')
await expectFilePageLoaded(fileName, fileEntityId, userPage)
// Upload success alert intermittently appears when uploading a new version of the same file
await testAuth.step(
'dismiss file upload alert for new version of file, if visible',
async () => {
if (
await userPage.getByText('File successfully uploaded').isVisible()
) {
await dismissFileUploadAlert(userPage)
}
},
)
})

await testAuth.step(
Expand Down
2 changes: 1 addition & 1 deletion e2e/helpers/tables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export const expectTablePageLoaded = async (
await expect(page.getByRole('button', { name: 'Table Tools' })).toBeVisible(
{ timeout: defaultExpectTimeout * 2 },
)
await expect(page.locator('p').filter({ hasText: tableName })).toBeVisible()
await expect(page.getByRole('heading', { name: tableName })).toBeVisible()
await expect(
page
.locator('div')
Expand Down
20 changes: 10 additions & 10 deletions e2e_workflow/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@
1. Install Docker, if not already installed.
2. Create a new admin user on the backend dev stack named: `swc-e2e-admin-{your-name}`, then create a new PAT with View, Download, and Modify permissions.
3. Create a `.env` file with the following environment variables: `ADMIN_PAT`.
4. Build SWC: `yarn build`
5. Serve SWC via Tomcat Docker container: `yarn docker:start`
6. Run Tests: `yarn e2e`. Tests can be run multiple times against the same Docker container.
7. When finished testing, stop and remove Docker container: `yarn docker:stop`
4. Build SWC: `pnpm build`
5. Serve SWC via Tomcat Docker container: `pnpm docker:start`
6. Run Tests: `pnpm e2e`. Tests can be run multiple times against the same Docker container.
7. When finished testing, stop and remove Docker container: `pnpm docker:stop`

Notes:

Expand All @@ -21,12 +21,12 @@ Notes:
- Writing new tests:
- Review a simliar [backend integration test](https://github.com/Sage-Bionetworks/Synapse-Repository-Services/tree/develop/integration-test/src/test/java/org/sagebionetworks) for the order in which to clean up associated objects.
- Create static test users with known username and password, so that issues can be debugged by logging into the user accounts. See comments in `e2e/helpers/userConfig.ts`.
- Start by running the new test against one browser with one worker, trace on, and no retries: `yarn e2e --project=firefox --workers=1 --retries=0 --trace=on e2e/{new_test}.spec.ts`.
- Start by running the new test against one browser with one worker, trace on, and no retries: `pnpm e2e --project=firefox --workers=1 --retries=0 --trace=on e2e/{new_test}.spec.ts`.
- Before pushing changes to CI, run tests with the same configuration as CI by adding `CI=true` to the `.env` file.
- Running tests without installing Docker:
- Ensure that your maven settings file (usually located at `~/.m2/settings.xml`) points at the backend development stack. See endpoint parameters [in this guide](https://sagebionetworks.jira.com/wiki/spaces/SWC/pages/15597754/Developer+Bootstrap).
- Run `mvn clean install` followed by `mvn gwt:run` instead of steps 2 and 3.
- If you would like to run tests repeatedly without changing SWC, it will be faster to run SWC in a separate terminal (`mvn gwt:run`) and then run tests (`yarn e2e`), since Playwright will use the existing server and SWC won't need to recompile before tests are run.
- If you would like to run tests repeatedly without changing SWC, it will be faster to run SWC in a separate terminal (`mvn gwt:run`) and then run tests (`pnpm e2e`), since Playwright will use the existing server and SWC won't need to recompile before tests are run.

### CI

Expand All @@ -42,7 +42,7 @@ The GitHub UI or CLI can be used to view the reports:
- Navigate to the Action run summary page.
- Download the report named "html-report--attempt-{number}". _Note:_ only the report from the latest attempt will be available.
- Unzip the file and move the "index.html" file into the `playwright-report` directory in SWC.
- Run `yarn e2e:report` to view the HTML report in the browser.
- Run `pnpm e2e:report` to view the HTML report in the browser.
- GitHub CLI
- Install [GitHub CLI](https://cli.github.com/), if necessary.
- Install [jq](https://jqlang.github.io/jq/download/), if necessary.
Expand All @@ -60,7 +60,7 @@ The AWS console or CLI can be used to view the reports:
- Navigate to the S3 bucket: `s3://e2e-reports-bucket-bucket-1p1qz6p48t4uy`
- Download the shard reports locally.
- Move the files into the `blob-report` directory in SWC.
- Run `yarn e2e:report:blob` to merge the shard reports and open the resulting HTML report in the browser.
- Run `pnpm e2e:report:blob` to merge the shard reports and open the resulting HTML report in the browser.
- AWS CLI
- Install [AWS CLI](https://docs.aws.amazon.com/cli/latest/userguide/getting-started-install.html), if necessary.
- Configure access credentials for AWS CLI SSO with the `org-sagebase-synapsedev` account.
Expand Down Expand Up @@ -99,7 +99,7 @@ Playwright supports the [`DEBUG` environment variable](https://playwright.dev/do
- `pw:test`: setting up and tearing down tests
- `pw:api`: verbose logging of each playwright test call -- will log typed values, so can expose user credentials and should not be used on public CI

Multiple debug variables can be passed via a comma separated list, e.g. `DEBUG="pw:webserver,pw:browser" yarn e2e`.
Multiple debug variables can be passed via a comma separated list, e.g. `DEBUG="pw:webserver,pw:browser" pnpm e2e`.

### Common Issues

Expand Down Expand Up @@ -201,7 +201,7 @@ on: workflow_dispatch
- name: Run Playwright tests
env:
ADMIN_PAT: ${{ secrets.ADMIN_PAT }}
run: DEBUG="pw:api" yarn playwright test --trace on
run: DEBUG="pw:api" pnpm playwright test --trace on
```

7. Manually [trigger](https://docs.github.com/en/actions/using-workflows/manually-running-a-workflow) the workflow.
Expand Down
2 changes: 1 addition & 1 deletion e2e_workflow/view_github_report.sh
Original file line number Diff line number Diff line change
Expand Up @@ -54,4 +54,4 @@ gh api \
unzip "${ARTIFACT_ZIP}" -d "${REPORT_DIR}"

# Show report
yarn e2e:report
pnpm e2e:report
2 changes: 1 addition & 1 deletion e2e_workflow/view_s3_report.sh
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,4 @@ aws s3 sync \
--delete

# Merge blob reports, then serve as HTML report
yarn e2e:report:blob
pnpm e2e:report:blob
26 changes: 26 additions & 0 deletions js/main.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/**
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Vite will bundle this file, and the bundled asset will be emitted in the Portal HTML.

This will replace the various UMD bundles that many packages are no longer providing.

* Synapse Web Client JavaScript modules
*
* JavaScript dependencies that should be loaded to the global scope in SWC can be added here.
*
* This file will be bundled and loaded in the SWC application. Excess imports should be avoided to keep this
* bundle as small as possible.
*/
// vite/modulepreload-polyfill should be included per https://vite.dev/guide/backend-integration
import 'vite/modulepreload-polyfill'
import React from 'react'
import ReactDOM from 'react-dom'
import ReactDOMClient from 'react-dom/client'
import * as ReactQuery from '@tanstack/react-query'
import * as SRC from 'synapse-react-client'
import './mui.js'

// Append to global scope so these libraries can be accessed in JsInterop classes

window.React = React
window.ReactDOM = ReactDOM
window.ReactDOMClient = ReactDOMClient

window.ReactQuery = ReactQuery

window.SRC = SRC
6 changes: 6 additions & 0 deletions js/mui.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// Import and expose only the MUI components used in JsInterop
import Unstable_Grid2 from '@mui/material/Unstable_Grid2'

self.MaterialUI = {
Unstable_Grid2,
}
42 changes: 28 additions & 14 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

While it looks like we added a bunch of new dependencies, most of these are not new. With yarn v1, we could directly depend on artifacts from transitive dependencies. pnpm does not let you do this, so we have to add add those previously-transitive dependencies that we currently directly depend on.

"type": "module",
"dependencies": {
"@emotion/react": "^11.14.0",
"@emotion/styled": "^11.14.0",
"@mui/material": "^5.16.14",
"@sage-bionetworks/markdown-it-container": "^4.0.1",
"@sage-bionetworks/synapse-types": "0.0.4",
"@tanstack/react-query": "5.22.2",
"animate.css": "^4.1.1",
"bootstrap3": "npm:[email protected]",
"croppie": "2.6.5",
"font-awesome": "4.7.0",
Expand All @@ -15,10 +22,11 @@
"markdown-it-inline-comments": "^1.0.1",
"markdown-it-math": "^4.1.1",
"markdown-it-strikethrough-alt": "^1.0.0",
"markdown-it-sub": "^2.0.0",
"markdown-it-sup": "^2.0.0",
Comment on lines -18 to -19
Copy link
Contributor Author

@nickgros nickgros Feb 12, 2025

Choose a reason for hiding this comment

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

These were not used. We were already using the -alt packages, they just were not explicitly listed as dependencies.

"markdown-it-sub-alt": "^1.0.0",
"markdown-it-sup-alt": "^1.0.2",
"markdown-it-synapse": "^1.1.17",
"markdown-it-synapse-heading": "^1.0.1",
"markdown-it-synapse-math": "^3.0.5",
"markdown-it-synapse-table": "^1.0.8",
"moment": "^2.29.4",
"papaparse": "^5.4.1",
Expand All @@ -27,15 +35,17 @@
"prop-types": "^15.8.1",
"react": "18.2.0",
"react-dom": "18.2.0",
"react-measure": "2.5.2",
"react-plotly.js": "^2.6.0",
"react-transition-group": "2.6.0",
"sass": "^1.63.6",
"spark-md5": "^3.0.2",
"synapse-react-client": "3.4.3",
"universal-cookie": "^4.0.4",
"xss": "^1.0.15"
},
"resolutions": {
"react": "18.2.0",
"react-dom": "18.2.0"
},
"devDependencies": {
"@playwright/test": "^1.48.0",
"@prettier/plugin-xml": "^3.1.0",
Expand All @@ -54,27 +64,31 @@
"prettier-plugin-java": "2.5.0",
"typescript": "5.1.6",
"uuid": "^9.0.0",
"vite": "^6.0.11",
"vite-plugin-node-polyfills": "0.17.0",
Comment on lines +67 to +68
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add Vite

"wait-on": "^8.0.0"
},
"lint-staged": {
"*.{*}": "prettier --write"
},
"scripts": {
"build": "mvn -B package --file pom.xml",
"dev": "concurrently -k -n \"CODESERVER,WATCHER,TOMCAT\" -c \"auto,auto,auto\" \"yarn dev:codeserver\" \"yarn dev:watcher\" \"yarn dev:tomcat\"",
"dev": "pnpm concurrently -k -n \"CODESERVER,WATCHER,TOMCAT,VITE\" -c \"auto,auto,auto,auto\" \"pnpm dev:codeserver\" \"pnpm dev:watcher\" \"pnpm dev:tomcat\" \"pnpm dev:vite\"",
"dev:codeserver": "mvn clean gwt:run-codeserver",
"dev:watcher": "wait-on tcp:127.0.0.1:9876 && mvn fizzed-watcher:run",
"dev:tomcat": "wait-on tcp:127.0.0.1:9876 && docker pull tomcat:9.0; docker run --name swc-dev --rm -p 8888:8080 -v \"/$(pwd)/target/portal-develop-SNAPSHOT/:/usr/local/tomcat/webapps/ROOT/\" -v \"/$HOME/.m2/settings.xml\":/root/.m2/settings.xml tomcat:9.0",
"dev:tomcat": "wait-on tcp:127.0.0.1:9876 && docker pull tomcat:9.0; docker run --name swc-dev --rm -p 8888:8080 -v \"/$(pwd)/target/portal-develop-SNAPSHOT/:/usr/local/tomcat/webapps/ROOT/\" -v \"/$HOME/.m2/settings.xml\":/root/.m2/settings.xml -e CATALINA_OPTS=\"-Dorg.sagebionetworks.web.client.dev.mode=true\" tomcat:9.0",
Copy link
Contributor Author

@nickgros nickgros Feb 12, 2025

Choose a reason for hiding this comment

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

Add environment variable org.sagebionetworks.web.client.dev.mode=true, which tells the servlet that we are running in dev mode and to use the Vite dev server.

"dev:vite": "vite dev",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add a script that runs the Vite dev server (live reloading of assets bundled by Vite)

"docker:start": "docker pull tomcat:9.0; docker run --name swc-tomcat -d --rm -p 8888:8080 -v \"/$(pwd)/target/portal-develop-SNAPSHOT.war:/usr/local/tomcat/webapps/ROOT.war\" -v \"/$(pwd)/e2e_workflow/settings.xml\":/root/.m2/settings.xml tomcat:9.0",
"docker:stop": "docker stop swc-tomcat",
"prepare": "husky install",
"e2e": "yarn playwright test",
"e2e:chromium": "yarn playwright test --project=chromium",
"e2e": "pnpm playwright test",
"e2e:chromium": "pnpm playwright test --project=chromium",
"e2e:lint": "eslint e2e",
"e2e:ui": "yarn playwright test --ui",
"e2e:codegen": "yarn playwright codegen",
"e2e:report": "yarn playwright show-report",
"e2e:report:blob": "yarn playwright merge-reports --reporter html ./blob-report && yarn e2e:report",
"playwright:update": "yarn add -D @playwright/test@latest; yarn playwright install"
}
"e2e:ui": "pnpm playwright test --ui",
"e2e:codegen": "pnpm playwright codegen",
"e2e:report": "pnpm playwright show-report",
"e2e:report:blob": "pnpm playwright merge-reports --reporter html ./blob-report && pnpm e2e:report",
"playwright:update": "pnpm add -D @playwright/test@latest; pnpm playwright install"
},
"packageManager": "[email protected]+sha512.b2dc20e2fc72b3e18848459b37359a32064663e5627a51e4c74b2c29dd8e8e0491483c3abb40789cfd578bf362fb6ba8261b05f0387d76792ed6e23ea3b1b6a0"
}
Loading