-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
test(playwright): add snapshots, refactor utils, coverage #9078
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
Merged
+2,134
−1,297
Merged
Changes from all commits
Commits
Show all changes
77 commits
Select commit
Hold shift + click to select a range
9dbcc6d
step
ShaMan123 693ba03
Update text-editing.spec.ts
ShaMan123 31316ca
cute
ShaMan123 2d9c037
snapshots!
ShaMan123 885a85c
great
ShaMan123 5b92828
update CHANGELOG.md
github-actions[bot] dfcd4c8
beautiful
ShaMan123 5eedece
Merge branch 'test-playwright' of https://github.com/fabricjs/fabric.…
ShaMan123 da7004a
Update tests.yml
ShaMan123 a2d8bbe
Update tests.yml
ShaMan123 ad95201
Update tests.yml
ShaMan123 f696407
Update playwright.config.ts
ShaMan123 0f7264c
Update tests.yml
ShaMan123 eabe24c
Update tests.yml
ShaMan123 43f20be
update CHANGELOG.md
github-actions[bot] 36aa953
Update playwright.config.ts
ShaMan123 7f1b2ee
Update tests.yml
ShaMan123 072be5d
Merge branch 'master' into test-playwright
ShaMan123 77d7bb7
Update tests.yml
ShaMan123 d1d6fa9
Update tests.yml
ShaMan123 28bfaa7
Update playwright.config.ts
ShaMan123 daf1846
coverage
ShaMan123 3f3b314
fix snapshot naming
ShaMan123 0b15bcd
Update .npmignore
ShaMan123 3a56887
Update text-editing.spec.ts
ShaMan123 f1d1b58
again
ShaMan123 1676c42
Update tests.yml
ShaMan123 ce5d820
fix coverage reporting
ShaMan123 bb8b5e4
try cache playwright browsers
ShaMan123 ea51a06
Update tests.yml
ShaMan123 d76c5b6
update CHANGELOG.md
github-actions[bot] 7a8ae28
perf(): install only chromium
ShaMan123 cd2f566
cache playwright browsers
ShaMan123 92e58eb
update CHANGELOG.md
github-actions[bot] ac351ad
perf(): install only chromium
ShaMan123 042bc8e
Update tests.yml
ShaMan123 15e8f86
Merge branch 'test-playwright' of https://github.com/fabricjs/fabric.…
ShaMan123 203a8bc
extract methods for reuse
ShaMan123 9e72d8e
Update playwright.config.ts
ShaMan123 a3ac0d6
Create startVanillaApp.ts
ShaMan123 948874b
Create setupApp.ts
ShaMan123 484544b
Update setupApp.ts
ShaMan123 32406ca
Update package.json
ShaMan123 9135347
config
ShaMan123 a03f22c
feat(): allow vanilla app in tests
ShaMan123 b1927e3
Revert "extract methods for reuse"
ShaMan123 ab7c15a
progress
ShaMan123 90aabcc
point
ShaMan123 056170a
progress
ShaMan123 98c5a13
rename
ShaMan123 40e0f0f
Update tsconfig.json
ShaMan123 66f8d7d
Update index.html
ShaMan123 4d56f0f
Update setupTest.ts
ShaMan123 a76d408
fix race condition
ShaMan123 614ba02
mv
ShaMan123 524f3c1
stable
ShaMan123 696c766
cleanup
ShaMan123 a47d078
Update setupCoverage.ts
ShaMan123 8bd78b3
Merge branch 'master' into test-playwright
ShaMan123 40e91b2
Update setupApp.ts
ShaMan123 e61db33
Update tsconfig.json
ShaMan123 0892b4c
finalize hopefully
ShaMan123 69b1d2c
watch mode
ShaMan123 ef3d685
Update setupCoverage.ts
ShaMan123 a47e09f
template
ShaMan123 7a0da31
body
ShaMan123 7e806d4
selectors
ShaMan123 2048de7
multiple canvas init
ShaMan123 fe6d6d7
Update setupTest.ts
ShaMan123 562b234
Update playwright.config.ts
ShaMan123 acd274d
run CI in headed mode
ShaMan123 c1222e6
mv
ShaMan123 8cef2bf
comments
ShaMan123 5d49b4c
cleanup
ShaMan123 3fba4c3
Update index.ts
ShaMan123 78cc1ee
Update index.ts
ShaMan123 3e38676
extract
ShaMan123 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -68,9 +68,7 @@ jobs: | |
| - name: Build fabric.js | ||
| uses: ./.github/actions/build-fabric-cached | ||
| - name: Run ${{ matrix.target }} ${{ matrix.suite }} headless test | ||
| uses: coactions/setup-xvfb@v1 | ||
| with: | ||
| run: npm run test -- -c ${{ matrix.target }} -s ${{ matrix.suite }} | ||
| run: xvfb-run npm run test -- -c ${{ matrix.target }} -s ${{ matrix.suite }} | ||
| node: | ||
| needs: [prime-build] | ||
| runs-on: ubuntu-latest | ||
|
|
@@ -109,8 +107,33 @@ jobs: | |
| with: | ||
| name: coverage-jest | ||
| path: .nyc_output/*.json | ||
| e2e: | ||
| needs: [prime-build] | ||
| name: Playwright tests | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v3 | ||
| - uses: ./.github/actions/cached-install | ||
| - name: Build fabric.js | ||
| uses: ./.github/actions/build-fabric-cached | ||
| # Playwright suggests against caching the browser install | ||
| - name: Install Playwright Browsers | ||
| run: npx playwright install --with-deps chromium | ||
| - name: Run Playwright tests | ||
| run: xvfb-run npm run test:e2e | ||
| - name: Upload Test Output | ||
| uses: actions/upload-artifact@v3 | ||
| if: failure() | ||
| with: | ||
| name: e2e-report | ||
| path: ./e2e/test-report/ | ||
| - name: Upload test coverage | ||
| uses: actions/upload-artifact@v3 | ||
| with: | ||
| name: coverage-e2e | ||
| path: ./e2e/test-results/**/coverage.json | ||
| coverage: | ||
| needs: [node-coverage] | ||
| needs: [node-coverage, e2e] | ||
| name: Coverage reporting | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
|
|
@@ -127,6 +150,10 @@ jobs: | |
| with: | ||
| name: coverage-visual | ||
| path: .nyc_output | ||
| - uses: actions/download-artifact@v3 | ||
| with: | ||
| name: coverage-e2e | ||
| path: .nyc_output | ||
| - run: ls -l .nyc_output | ||
| - run: npm run coverage:report | ||
| - uses: ShaMan123/[email protected] | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,7 +7,7 @@ change-output.md | |
| before_commit | ||
| /coverage/ | ||
| .idea/ | ||
| /dist | ||
| **/dist | ||
| /cli_output/ | ||
| e2e/test-report/ | ||
| e2e/test-results/ | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,7 @@ lib/*.png | |
| coverage/ | ||
| test/ | ||
| test* | ||
| e2e/ | ||
| .DS_Store | ||
| .codesandbox/ | ||
| .devcontainer/ | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| import { readJSONSync } from 'fs-extra'; | ||
|
|
||
| /** | ||
| * The import map used by `./utils/setupApp` to inject into the page | ||
| * so test scripts can use modules (relative imports don't seem to work out of the box) | ||
| * | ||
| * **IMPORTANT**: be sure to update the paths field in `./tsconfig.json` to reflect imports correctly | ||
| */ | ||
| export default { | ||
| fabric: readJSONSync('./package.json').module.slice(1), | ||
| test: '/e2e/dist/test.js', | ||
| }; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| // call first | ||
| import './setupSelectors'; | ||
| // call before using fabric | ||
| import './setupCoverage'; | ||
| // call at the end - navigates the page | ||
| import './setupApp'; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| import { test } from '@playwright/test'; | ||
| import { existsSync, readFileSync } from 'fs'; | ||
| import path from 'path'; | ||
| import imports from '../imports'; | ||
| import { JSDOM } from 'jsdom'; | ||
|
|
||
| test.beforeEach(async ({ page }, { file }) => { | ||
| await page.goto('/e2e/site'); | ||
| // expose imports for consumption | ||
| page.addScriptTag({ | ||
| type: 'importmap', | ||
| content: JSON.stringify({ | ||
| imports, | ||
| }), | ||
| }); | ||
| // add test script | ||
| const testDir = path.relative( | ||
| path.resolve(process.cwd(), 'e2e', 'tests'), | ||
| path.resolve(file, '..') | ||
| ); | ||
| const pathToHTML = path.resolve( | ||
| process.cwd(), | ||
| 'e2e', | ||
| 'tests', | ||
| testDir, | ||
| 'index.html' | ||
| ); | ||
| if (existsSync(pathToHTML)) { | ||
| const doc = new JSDOM(readFileSync(pathToHTML).toString()).window.document; | ||
| await page.evaluate((html) => { | ||
| document.body.innerHTML = `${html}`; | ||
| }, doc.body.innerHTML); | ||
| } | ||
| const pathToApp = path.resolve( | ||
| process.cwd(), | ||
| 'e2e', | ||
| 'tests', | ||
| testDir, | ||
| 'index.ts' | ||
| ); | ||
| const pathToBuiltApp = path.resolve( | ||
| process.cwd(), | ||
| 'e2e', | ||
| 'dist', | ||
| testDir, | ||
| 'index.js' | ||
| ); | ||
| const exists = existsSync(pathToBuiltApp); | ||
| if (!exists && existsSync(pathToApp)) { | ||
| throw new Error( | ||
| `test script '${pathToBuiltApp}' not found: global setup script probably did not run` | ||
| ); | ||
| } else if (exists) { | ||
| // used to avoid a race condition that occurs because of script loading | ||
| const trigger = page.evaluate( | ||
| () => | ||
| new Promise((resolve) => { | ||
| window.addEventListener('fabric:setup', resolve, { once: true }); | ||
| }) | ||
| ); | ||
| await page.addScriptTag({ | ||
| type: 'module', | ||
| content: `${readFileSync( | ||
| path.relative(process.cwd(), pathToApp) | ||
| ).toString()} | ||
| window.dispatchEvent(new CustomEvent('fabric:setup')); | ||
| `, | ||
| }); | ||
| await trigger; | ||
| await page.evaluate(() => window.__setupFabricHook()); | ||
| } | ||
| }); | ||
|
|
||
| test.afterEach(async ({ page }) => { | ||
| await page.evaluate(() => window.__teardownFabricHook()); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| import { test } from '@playwright/test'; | ||
| import { ensureDirSync, writeFileSync } from 'fs-extra'; | ||
| import _ from 'lodash'; | ||
| import path from 'path'; | ||
| import { URL } from 'url'; | ||
| import v8toIstanbul from 'v8-to-istanbul'; | ||
|
|
||
| // https://playwright.dev/docs/api/class-coverage | ||
|
|
||
| test.beforeEach(async ({ page }) => { | ||
| await page.coverage.startJSCoverage({ reportAnonymousScripts: false }); | ||
| }); | ||
|
|
||
| test.afterEach(async ({ page }, { outputDir }) => { | ||
| const coverage = await page.coverage.stopJSCoverage(); | ||
| const nyc = _.fromPairs( | ||
| await Promise.all( | ||
| coverage.map(async ({ url, source, functions }) => { | ||
| let pathname = url; | ||
| try { | ||
| // remove url origin | ||
| pathname = pathname.slice(new URL(url).origin.length + 1); | ||
| } catch (error) {} | ||
| const pathTo = path.resolve(process.cwd(), pathname); | ||
| const converter = v8toIstanbul(pathTo, 0, { | ||
| source: source!, | ||
| }); | ||
| await converter.load(); | ||
| converter.applyCoverage(functions); | ||
| return [pathTo, converter.toIstanbul()]; | ||
| }) | ||
| ) | ||
| ); | ||
| ensureDirSync(outputDir); | ||
| writeFileSync( | ||
| path.resolve(outputDir, 'coverage-v8.json'), | ||
| JSON.stringify(coverage, null, 2) | ||
| ); | ||
| writeFileSync( | ||
| path.resolve(outputDir, 'coverage.json'), | ||
| JSON.stringify(nyc, null, 2) | ||
| ); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| import { selectors, test } from '@playwright/test'; | ||
|
|
||
| test.beforeAll(async () => { | ||
| await selectors.register('canvas_wrapper', () => { | ||
| return { | ||
| query(root: Document, selector: string) { | ||
| return root.querySelector(selector).parentElement; | ||
| }, | ||
|
|
||
| queryAll(root: Document, selector: string) { | ||
| return Array.from(root.querySelectorAll(selector)).map( | ||
| (el) => el.parentElement | ||
| ); | ||
| }, | ||
| }; | ||
| }); | ||
|
|
||
| await selectors.register('canvas_top', () => { | ||
| return { | ||
| query(root: Document, selector: string) { | ||
| return root | ||
| .querySelector(selector) | ||
| .parentElement.querySelector('canvas.upper-canvas'); | ||
| }, | ||
|
|
||
| queryAll(root: Document, selector: string) { | ||
| return Array.from(root.querySelectorAll(selector)).map((el) => | ||
| el.parentElement.querySelector('canvas.upper-canvas') | ||
| ); | ||
| }, | ||
| }; | ||
| }); | ||
| }); |
Binary file not shown.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,25 +1,19 @@ | ||
| <html> | ||
| <head> | ||
| <script type="text/javascript" src="/dist/index.js"></script> | ||
| <title>Fabric e2e Testing Suite</title> | ||
| <link rel="icon" type="image/x-icon" href="./favicon.ico" /> | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. to silence playwright raising an error |
||
| <style> | ||
| body { | ||
| padding: 0; | ||
| margin: 0; | ||
| background-color: black; | ||
| } | ||
| .canvas-container { | ||
| border: 1px solid black; | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. was causing snapshot size mismatch |
||
| [data-fabric='wrapper'] { | ||
| background-color: white; | ||
| } | ||
| </style> | ||
| </head> | ||
| <body> | ||
| <canvas id="test-canvas" width="800" height="600"></canvas> | ||
| <canvas id="canvas" width="800" height="600"></canvas> | ||
| </body> | ||
| <script type="text/javascript"> | ||
| const fabricCanvas = new fabric.Canvas( | ||
| document.getElementById('test-canvas'), | ||
| { enableRetinaScaling: false } | ||
| ); | ||
| window.fabricCanvas = fabricCanvas; | ||
| window.setupScene = (json) => fabricCanvas.loadFromJSON(json); | ||
| </script> | ||
| </html> | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| <!-- | ||
| This file overrides the `body` of the test app as defined in `e2e/site/index.html`. | ||
| Add it only if there is a need to change `body` | ||
| --> | ||
|
|
||
| <canvas id="canvas" width="800" height="600"></canvas> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| import { test } from '@playwright/test'; | ||
| import { CanvasUtil } from '../../utils/CanvasUtil'; | ||
| import { ObjectUtil } from '../../utils/ObjectUtil'; | ||
|
|
||
| import '../../setup'; | ||
|
|
||
| test('TEST NAME', async ({ page }) => { | ||
| const canvasUtil = new CanvasUtil(page); | ||
| // note that `textbox` correlates to the returned key in `index.ts` => `beforeAll` | ||
| const textboxUtil = new ObjectUtil(page, 'textbox'); | ||
| // write the test | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| /** | ||
| * Runs in the **BROWSER** | ||
| * Use absolute imports defined in 'e2e/imports.ts' | ||
| */ | ||
|
|
||
| import * as fabric from 'fabric'; | ||
| import { beforeAll } from 'test'; | ||
|
|
||
| beforeAll((canvas) => { | ||
| const textbox = new fabric.Textbox('fabric.js test', { | ||
| width: 200, | ||
| top: 20, | ||
| }); | ||
| canvas.add(textbox); | ||
| canvas.centerObjectH(textbox); | ||
|
|
||
| // playwright will be able to access the passed objects by their keys | ||
| // make sure to pass unique keys across the entire app | ||
| return { textbox }; | ||
| }); |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we want/need to run CI in headed mode?
@asturur ?
@mxschmitt would love your input
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For browsers test as unit tests in browsers is not needed.
For playwright i don't know the framework so well yet.
i assume that to take screenshots you need xvfb ready unless you are doing them through the browser with an internal browser buffer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you look before this commit you will see I didn't use the headed option thus nkt needing xvfb
But I don't know what the diff is, tests pass in both cases
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's keep what works, at some point we will optimize instance usage and strain, for now starting the X server on linux who cares