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

Migrate from Jest to Playwright #62

Merged
merged 5 commits into from
Feb 17, 2023
Merged

Migrate from Jest to Playwright #62

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
998304e
Migrate from Jest to Playwright
Gudahtt Feb 16, 2023
2355649
Add comments to `setupStreamInitialization`
Gudahtt Feb 17, 2023
aea1b1e
Remove leftover console.log
Gudahtt Feb 17, 2023
c065dfa
Remove unnecessary array
Gudahtt Feb 17, 2023
160154a
Improve README instructions
Gudahtt Feb 17, 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
12 changes: 0 additions & 12 deletions .eslintrc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,6 @@ module.exports = {
},
extends: ['@metamask/eslint-config-nodejs'],
},

{
files: ['*.test.ts'],
extends: ['@metamask/eslint-config-jest'],
parserOptions: {
sourceType: 'module',
},
rules: {
// disabled to allow use of Jest tags to set inline test environment options
'jsdoc/check-tag-names': 'off',
},
},
],

ignorePatterns: [
Expand Down
14 changes: 12 additions & 2 deletions .github/workflows/build-lint-test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ jobs:
fi

test:
name: Test
name: Run Playwright tests
timeout-minutes: 60
runs-on: ubuntu-latest
needs:
- prepare
Expand All @@ -89,7 +90,16 @@ jobs:
node-version: ${{ matrix.node-version }}
cache: 'yarn'
- run: yarn --immutable --immutable-cache
- run: yarn test
- name: Install Playwright Browsers
run: yarn playwright install --with-deps chrome chromium firefox msedge
- name: Run Playwright tests
run: yarn test
- uses: actions/upload-artifact@v3
if: always()
with:
name: playwright-report
path: playwright-report/
retention-days: 30
- name: Require clean working directory
shell: bash
run: |
Expand Down
5 changes: 5 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,3 +75,8 @@ node_modules/
!.yarn/releases
!.yarn/sdks
!.yarn/versions

# Playwright
/test-results/
/playwright-report/
/playwright/.cache/
28 changes: 25 additions & 3 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,34 @@ This package is published to npm so that we can host it locally when running end
- Install [Yarn v3](https://yarnpkg.com/getting-started/install)
- Run `yarn install` to install dependencies and run any required post-install scripts

### Testing and Linting

Run `yarn test` to run the tests once. To run tests on file changes, run `yarn test:watch`.
### Linting

Run `yarn lint` to run the linter, or run `yarn lint:fix` to run the linter and fix any automatically fixable issues.

### Testing

#### Installing Playwright

Before running tests using Playright, you will need to install the required browsers using `yarn playwright install [space-separated browsers]`. Use the `--with-deps` flag as well to prompt Playwright to install OS dependencies as well if possible, though be warned that this may require elevated privileges.

The browsers we test with in CI are `chrome`, `chromium`, `firefox`, and `msedge`.

To install all browsers we use on CI and any OS dependencies, run:

```
yarn playwright install --with-deps chrome chromium firefox msedge
Copy link
Contributor

Choose a reason for hiding this comment

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

I kind of wish that we'd kept the yarn setup convention for this and other similar use cases, but I guess this makes sense as a one-off case. Might be something that people gloss over though. Is it worth it to figure out a way to bake it into yarn install? Or maybe we start employing postinstall again?

Copy link
Member Author

@Gudahtt Gudahtt Feb 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I kinda see this being a separate step as a feature rather than a bug. This is an arduous install process; huge download sizes and long install times, and all totally unnecessary unless you're working with e2e tests.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've just improved these instructions: 160154a

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, okay. If you're working on this repo, wouldn't you want to run all tests locally, including the e2e tests? But, I see your point. Someone would probably have to read the README to understand how the e2e tests work, anyway, so that's fair to have a separate step.

Copy link
Member Author

Choose a reason for hiding this comment

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

Personally I've only been testing with firefox and chromium. There was an issue with installing Google Chrome that I hadn't bothered look into. I could understand contributors maybe only testing with one or two browsers, or not bothering test at all if their contribution isn't changing any test expectations.

```

You may want to consider using just one or two browsers for local testing to speed things up. These install steps are long and require a decent amount of disk space.

#### Running tests

Tests can be run using `yarn test`. This will run all tests using all browsers.

To run with just a single browser, you'll need to use the flag `--project=[browser name]`. For example, `yarn test --project=chromium` to run all tests with Chromium. See the Playwright configuration (`playwright.config.ts`) to see the other project names.

If you want to run a single test suite, pass in the test filename as a parameter. For example, `yarn test ./tests/defaults.spec.ts` will run just the "defaults" test suite.

### Release & Publishing

The project follows the same release process as the other libraries in the MetaMask organization. The GitHub Actions [`action-create-release-pr`](https://github.com/MetaMask/action-create-release-pr) and [`action-publish-release`](https://github.com/MetaMask/action-publish-release) are used to automate the release process; see those repositories for more information about how they work.
Expand Down
209 changes: 0 additions & 209 deletions jest.config.ts
View file
Open in desktop

This file was deleted.

16 changes: 7 additions & 9 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
"lint:misc": "prettier '**/*.json' '**/*.md' '!CHANGELOG.md' '**/*.yml' '!.yarnrc.yml' --ignore-path .gitignore --no-error-on-unmatched-pattern",
"prepack": "yarn build:prod && yarn lint && yarn test",
"setup": "yarn install && yarn allow-scripts",
"test": "jest",
"test:watch": "jest --watch"
"start": "yarn build && yarn http-server ./dist",
"test": "yarn playwright test"
},
"dependencies": {
"@metamask/design-tokens": "^1.6.0",
Expand All @@ -37,12 +37,11 @@
"@lavamoat/allow-scripts": "^2.3.0",
"@metamask/auto-changelog": "^3.0.0",
"@metamask/eslint-config": "^9.0.0",
"@metamask/eslint-config-jest": "^9.0.0",
"@metamask/eslint-config-nodejs": "^9.0.0",
"@metamask/eslint-config-typescript": "^9.0.1",
"@playwright/test": "^1.30.0",
"@testing-library/dom": "^8.13.0",
"@testing-library/user-event": "^14.1.1",
"@types/jest": "^26.0.13",
"@types/node": "^17.0.23",
"@types/pump": "^1.1.1",
"@types/readable-stream": "^2.3.13",
Expand All @@ -53,19 +52,17 @@
"eslint": "^7.24.0",
"eslint-config-prettier": "^8.5.0",
"eslint-plugin-import": "^2.22.1",
"eslint-plugin-jest": "^24.1.5",
"eslint-plugin-jsdoc": "^36.1.0",
"eslint-plugin-node": "^11.1.0",
"eslint-plugin-prettier": "^3.4.1",
"exorcist": "^2.0.0",
"fs-extra": "^10.1.0",
"jest": "^28.0.0",
"jest-environment-jsdom": "^28.1.0",
"http-server": "^14.1.1",
"minify-stream": "^2.1.0",
"playwright": "^1.30.0",
"prettier": "^2.6.2",
"prettier-plugin-packagejson": "^2.2.17",
"terser": "^5.13.1",
"ts-jest": "^28.0.1",
"ts-node": "^10.7.0",
"typescript": "~4.4.4",
"workbox-build": "^6.5.3"
Expand All @@ -80,7 +77,8 @@
},
"lavamoat": {
"allowScripts": {
"@lavamoat/preinstall-always-fail": false
"@lavamoat/preinstall-always-fail": false,
"playwright": false
}
}
}
Loading