-
Notifications
You must be signed in to change notification settings - Fork 639
Use Vitest for browser tests #3236
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
Changes from 22 commits
25d84a4
59ababa
e537963
66d2820
1f3881c
b4c80a7
18cfa57
4be8c06
b04bf6d
29a840e
09db8a4
624ad7b
d998f8e
353fd46
4eaba31
4c5bddb
6df7a6c
38dd91f
e28669a
704a701
45b124c
6e2400a
0a4dda8
5f54a7f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -108,7 +108,7 @@ exports[`plugin generates a source map 1`] = ` | |
| }; | ||
| }, {}] | ||
| }, {}, [1]); | ||
| //# sourceMappingURL=data:application/json;charset=utf-8;base64,eyJ2ZXJzaW9uIjozLCJuYW1lcyI6WyJyIiwiZSIsIm4iLCJ0IiwibyIsImkiLCJmIiwiYyIsInJlcXVpcmUiLCJ1IiwiYSIsIkVycm9yIiwiY29kZSIsInAiLCJleHBvcnRzIiwiY2FsbCIsImxlbmd0aCIsIm1vZHVsZSIsIm9uUnBjUmVxdWVzdCIsInJlcXVlc3QiLCJjb25zb2xlIiwibG9nIiwibWV0aG9kIiwiaWQiXSwic291cmNlcyI6WyIuLi8uLi9ub2RlX21vZHVsZXMvYnJvd3Nlci1wYWNrL19wcmVsdWRlLmpzIiwiX3N0cmVhbV8wLmpzIl0sInNvdXJjZXNDb250ZW50IjpbIihmdW5jdGlvbigpe2Z1bmN0aW9uIHIoZSxuLHQpe2Z1bmN0aW9uIG8oaSxmKXtpZighbltpXSl7aWYoIWVbaV0pe3ZhciBjPVwiZnVuY3Rpb25cIj09dHlwZW9mIHJlcXVpcmUmJnJlcXVpcmU7aWYoIWYmJmMpcmV0dXJuIGMoaSwhMCk7aWYodSlyZXR1cm4gdShpLCEwKTt2YXIgYT1uZXcgRXJyb3IoXCJDYW5ub3QgZmluZCBtb2R1bGUgJ1wiK2krXCInXCIpO3Rocm93IGEuY29kZT1cIk1PRFVMRV9OT1RfRk9VTkRcIixhfXZhciBwPW5baV09e2V4cG9ydHM6e319O2VbaV1bMF0uY2FsbChwLmV4cG9ydHMsZnVuY3Rpb24ocil7dmFyIG49ZVtpXVsxXVtyXTtyZXR1cm4gbyhufHxyKX0scCxwLmV4cG9ydHMscixlLG4sdCl9cmV0dXJuIG5baV0uZXhwb3J0c31mb3IodmFyIHU9XCJmdW5jdGlvblwiPT10eXBlb2YgcmVxdWlyZSYmcmVxdWlyZSxpPTA7aTx0Lmxlbmd0aDtpKyspbyh0W2ldKTtyZXR1cm4gb31yZXR1cm4gcn0pKCkiLCJcbiAgbW9kdWxlLmV4cG9ydHMub25ScGNSZXF1ZXN0ID0gKHsgcmVxdWVzdCB9KSA9PiB7XG4gICAgY29uc29sZS5sb2coXCJIZWxsbywgd29ybGQhXCIpO1xuXG4gICAgY29uc3QgeyBtZXRob2QsIGlkIH0gPSByZXF1ZXN0O1xuICAgIHJldHVybiBtZXRob2QgKyBpZDtcbiAgfTtcbiJdLCJtYXBwaW5ncyI6IkFBQUE7RUFBQSxTQUFBQSxFQUFBQyxDQUFBLEVBQUFDLENBQUEsRUFBQUMsQ0FBQTtJQUFBLFNBQUFDLEVBQUFDLENBQUEsRUFBQUMsQ0FBQTtNQUFBLEtBQUFKLENBQUEsQ0FBQUcsQ0FBQTtRQUFBLEtBQUFKLENBQUEsQ0FBQUksQ0FBQTtVQUFBLElBQUFFLENBQUEsd0JBQUFDLE9BQUEsSUFBQUEsT0FBQTtVQUFBLEtBQUFGLENBQUEsSUFBQUMsQ0FBQSxTQUFBQSxDQUFBLENBQUFGLENBQUE7VUFBQSxJQUFBSSxDQUFBLFNBQUFBLENBQUEsQ0FBQUosQ0FBQTtVQUFBLElBQUFLLENBQUEsT0FBQUMsS0FBQSwwQkFBQU4sQ0FBQTtVQUFBLE1BQUFLLENBQUEsQ0FBQUUsSUFBQSx1QkFBQUYsQ0FBQTtRQUFBO1FBQUEsSUFBQUcsQ0FBQSxHQUFBWCxDQUFBLENBQUFHLENBQUE7VUFBQVMsT0FBQTtRQUFBO1FBQUFiLENBQUEsQ0FBQUksQ0FBQSxLQUFBVSxJQUFBLENBQUFGLENBQUEsQ0FBQUMsT0FBQSxZQUFBZCxDQUFBO1VBQUEsSUFBQUUsQ0FBQSxHQUFBRCxDQUFBLENBQUFJLENBQUEsS0FBQUwsQ0FBQTtVQUFBLE9BQUFJLENBQUEsQ0FBQUYsQ0FBQSxJQUFBRixDQUFBO1FBQUEsR0FBQWEsQ0FBQSxFQUFBQSxDQUFBLENBQUFDLE9BQUEsRUFBQWQsQ0FBQSxFQUFBQyxDQUFBLEVBQUFDLENBQUEsRUFBQUMsQ0FBQTtNQUFBO01BQUEsT0FBQUQsQ0FBQSxDQUFBRyxDQUFBLEVBQUFTLE9BQUE7SUFBQTtJQUFBLFNBQUFMLENBQUEsd0JBQUFELE9BQUEsSUFBQUEsT0FBQSxFQUFBSCxDQUFBLE1BQUFBLENBQUEsR0FBQUYsQ0FBQSxDQUFBYSxNQUFBLEVBQUFYLENBQUEsSUFBQUQsQ0FBQSxDQUFBRCxDQUFBLENBQUFFLENBQUE7SUFBQSxPQUFBRCxDQUFBO0VBQUE7RUFBQSxPQUFBSixDQUFBO0FBQUE7RUFBQSxjQUFBUSxPQUFBLEVBQUFTLE1BQUEsRUFBQUgsT0FBQTtJQ0NBRyxNQUFBLENBQUFILE9BQUEsQ0FBQUksWUFBQTtNQUFBQztJQUFBO01BQ0FDLE9BQUEsQ0FBQUMsR0FBQTtNQUVBO1FBQUFDLE1BQUE7UUFBQUM7TUFBQSxJQUFBSixPQUFBO01BQ0EsT0FBQUcsTUFBQSxHQUFBQyxFQUFBO0lBQ0EifQ==" | ||
|
Member
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. This changed because of a bump in dependencies responsible for source maps. |
||
| //# sourceMappingURL=data:application/json;charset=utf-8;base64,eyJ2ZXJzaW9uIjozLCJuYW1lcyI6WyJyIiwiZSIsIm4iLCJ0IiwibyIsImkiLCJmIiwiYyIsInJlcXVpcmUiLCJ1IiwiYSIsIkVycm9yIiwiY29kZSIsInAiLCJleHBvcnRzIiwiY2FsbCIsImxlbmd0aCIsIm1vZHVsZSIsIm9uUnBjUmVxdWVzdCIsInJlcXVlc3QiLCJjb25zb2xlIiwibG9nIiwibWV0aG9kIiwiaWQiXSwic291cmNlcyI6WyIuLi8uLi9ub2RlX21vZHVsZXMvYnJvd3Nlci1wYWNrL19wcmVsdWRlLmpzIiwiX3N0cmVhbV8wLmpzIl0sInNvdXJjZXNDb250ZW50IjpbIihmdW5jdGlvbigpe2Z1bmN0aW9uIHIoZSxuLHQpe2Z1bmN0aW9uIG8oaSxmKXtpZighbltpXSl7aWYoIWVbaV0pe3ZhciBjPVwiZnVuY3Rpb25cIj09dHlwZW9mIHJlcXVpcmUmJnJlcXVpcmU7aWYoIWYmJmMpcmV0dXJuIGMoaSwhMCk7aWYodSlyZXR1cm4gdShpLCEwKTt2YXIgYT1uZXcgRXJyb3IoXCJDYW5ub3QgZmluZCBtb2R1bGUgJ1wiK2krXCInXCIpO3Rocm93IGEuY29kZT1cIk1PRFVMRV9OT1RfRk9VTkRcIixhfXZhciBwPW5baV09e2V4cG9ydHM6e319O2VbaV1bMF0uY2FsbChwLmV4cG9ydHMsZnVuY3Rpb24ocil7dmFyIG49ZVtpXVsxXVtyXTtyZXR1cm4gbyhufHxyKX0scCxwLmV4cG9ydHMscixlLG4sdCl9cmV0dXJuIG5baV0uZXhwb3J0c31mb3IodmFyIHU9XCJmdW5jdGlvblwiPT10eXBlb2YgcmVxdWlyZSYmcmVxdWlyZSxpPTA7aTx0Lmxlbmd0aDtpKyspbyh0W2ldKTtyZXR1cm4gb31yZXR1cm4gcn0pKCkiLCJcbiAgbW9kdWxlLmV4cG9ydHMub25ScGNSZXF1ZXN0ID0gKHsgcmVxdWVzdCB9KSA9PiB7XG4gICAgY29uc29sZS5sb2coXCJIZWxsbywgd29ybGQhXCIpO1xuXG4gICAgY29uc3QgeyBtZXRob2QsIGlkIH0gPSByZXF1ZXN0O1xuICAgIHJldHVybiBtZXRob2QgKyBpZDtcbiAgfTtcbiJdLCJtYXBwaW5ncyI6IkFBQUE7RUFBQSxTQUFBQSxFQUFBQyxDQUFBLEVBQUFDLENBQUEsRUFBQUMsQ0FBQTtJQUFBLFNBQUFDLEVBQUFDLENBQUEsRUFBQUMsQ0FBQTtNQUFBLEtBQUFKLENBQUEsQ0FBQUcsQ0FBQTtRQUFBLEtBQUFKLENBQUEsQ0FBQUksQ0FBQTtVQUFBLElBQUFFLENBQUEsd0JBQUFDLE9BQUEsSUFBQUEsT0FBQTtVQUFBLEtBQUFGLENBQUEsSUFBQUMsQ0FBQSxTQUFBQSxDQUFBLENBQUFGLENBQUE7VUFBQSxJQUFBSSxDQUFBLFNBQUFBLENBQUEsQ0FBQUosQ0FBQTtVQUFBLElBQUFLLENBQUEsT0FBQUMsS0FBQSwwQkFBQU4sQ0FBQTtVQUFBLE1BQUFLLENBQUEsQ0FBQUUsSUFBQSx1QkFBQUYsQ0FBQTtRQUFBO1FBQUEsSUFBQUcsQ0FBQSxHQUFBWCxDQUFBLENBQUFHLENBQUE7VUFBQVMsT0FBQTtRQUFBO1FBQUFiLENBQUEsQ0FBQUksQ0FBQSxLQUFBVSxJQUFBLENBQUFGLENBQUEsQ0FBQUMsT0FBQSxZQUFBZCxDQUFBO1VBQUEsSUFBQUUsQ0FBQSxHQUFBRCxDQUFBLENBQUFJLENBQUEsS0FBQUwsQ0FBQTtVQUFBLE9BQUFJLENBQUEsQ0FBQUYsQ0FBQSxJQUFBRixDQUFBO1FBQUEsR0FBQWEsQ0FBQSxFQUFBQSxDQUFBLENBQUFDLE9BQUEsRUFBQWQsQ0FBQSxFQUFBQyxDQUFBLEVBQUFDLENBQUEsRUFBQUMsQ0FBQTtNQUFBO01BQUEsT0FBQUQsQ0FBQSxDQUFBRyxDQUFBLEVBQUFTLE9BQUE7SUFBQTtJQUFBLFNBQUFMLENBQUEsd0JBQUFELE9BQUEsSUFBQUEsT0FBQSxFQUFBSCxDQUFBLE1BQUFBLENBQUEsR0FBQUYsQ0FBQSxDQUFBYSxNQUFBLEVBQUFYLENBQUEsSUFBQUQsQ0FBQSxDQUFBRCxDQUFBLENBQUFFLENBQUE7SUFBQSxPQUFBRCxDQUFBO0VBQUE7RUFBQSxPQUFBSixDQUFBO0FBQUE7RUFBQSxjQUFBUSxPQUFBLEVBQUFTLE1BQUEsRUFBQUgsT0FBQTtJQ0NBRyxNQUFBLENBQUFILE9BQUEsQ0FBQUksWUFBQTtNQUFBQztJQUFBO01BQ0FDLE9BQUEsQ0FBQUMsR0FBQTtNQUVBO1FBQUFDLE1BQUE7UUFBQUM7TUFBQSxJQUFBSixPQUFBO01BQ0EsT0FBQUcsTUFBQSxHQUFBQyxFQUFBO0lBQ0EiLCJpZ25vcmVMaXN0IjpbXX0=" | ||
| `; | ||
|
|
||
| exports[`plugin processes files using Browserify 1`] = ` | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| { | ||
| "branches": 93.42, | ||
| "branches": 93.44, | ||
| "functions": 97.38, | ||
| "lines": 98.34, | ||
| "statements": 98.08 | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,9 +18,9 @@ const JEST_COVERAGE_FILE = resolve( | |
| 'coverage-final.json', | ||
| ); | ||
|
|
||
| const WDIO_COVERAGE_FILE = resolve( | ||
| const VITE_COVERAGE_FILE = resolve( | ||
| COVERAGE_PATH, | ||
| 'wdio', | ||
| 'vite', | ||
| 'coverage-final.json', | ||
| ); | ||
|
|
||
|
|
@@ -54,36 +54,37 @@ function generateSummaryReport<Report extends ReportType>( | |
| } | ||
|
|
||
| /** | ||
| * Merge the coverage reports from Jest and WebdriverIO. This checks if the | ||
| * coverage for a given file is higher in WebdriverIO than in Jest. If it is, | ||
| * it replaces the Jest coverage with the WebdriverIO coverage. | ||
| * Merge the coverage reports from Jest and Vite. This checks if the coverage | ||
| * for a given file is higher in Vite than in Jest. If it is, it replaces the | ||
| * Jest coverage with the Vite coverage. | ||
| * | ||
| * This is a workaround for WebdriverIO's coverage reporting having inaccurate | ||
| * line numbers. | ||
| * This is a workaround for Vite's coverage reporting having inaccurate line | ||
| * numbers. | ||
| * | ||
| * @returns The summary of the merged coverage. | ||
| */ | ||
| // TODO: Check if Vite's coverage is actually inaccurate. | ||
|
Contributor
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. I think it's worth looking into in this PR, especially if we can avoid unnecessary computation.
Member
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. Seems like either Jest or Vite is inaccurate. I checked the iframe execution service for example: Vite shows it as completely covered, but after merging with Jest, one line is not covered. The actual computation done here isn't really significant anyway though, and long term we can likely remove this once we migrate all tests to Vitest. |
||
| async function mergeReports() { | ||
| const jestMap = await fs | ||
| .readFile(JEST_COVERAGE_FILE, 'utf8') | ||
| .then(JSON.parse) | ||
| .then(createCoverageMap); | ||
|
|
||
| const wdioMap = await fs | ||
| .readFile(WDIO_COVERAGE_FILE, 'utf8') | ||
| const viteMap = await fs | ||
| .readFile(VITE_COVERAGE_FILE, 'utf8') | ||
| .then(JSON.parse) | ||
| .then(createCoverageMap); | ||
|
|
||
| const jestFiles = jestMap.files(); | ||
|
|
||
| wdioMap.files().forEach((file) => { | ||
| const coverage = wdioMap.fileCoverageFor(file); | ||
| const wdioSummary = coverage.toSummary(); | ||
| viteMap.files().forEach((file) => { | ||
| const coverage = viteMap.fileCoverageFor(file); | ||
| const viteSummary = coverage.toSummary(); | ||
| const jestSummary = jestMap.fileCoverageFor(file).toSummary(); | ||
|
|
||
| if ( | ||
| !jestFiles.includes(file) || | ||
| COVERAGE_KEYS.every((key) => wdioSummary[key].pct >= jestSummary[key].pct) | ||
| COVERAGE_KEYS.every((key) => viteSummary[key].pct >= jestSummary[key].pct) | ||
| ) { | ||
| jestMap.filter((jestFile) => jestFile !== file); | ||
| jestMap.addFileCoverage(coverage); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,11 +6,14 @@ import { | |
| spy, | ||
| } from '@metamask/snaps-utils/test-utils'; | ||
| import { assert } from '@metamask/utils'; | ||
| import { describe, it, expect } from 'vitest'; | ||
|
|
||
| import { IframeExecutionService } from './IframeExecutionService'; | ||
| import { createService, MOCK_BLOCK_NUMBER } from '../../test-utils'; | ||
| import { MOCK_BLOCK_NUMBER } from '../../test-utils/constants'; | ||
| import { createService } from '../../test-utils/service'; | ||
|
|
||
| const IFRAME_URL = 'http://localhost:4567'; | ||
| const IFRAME_URL = 'http://localhost:63315/iframe/executor/index.html'; | ||
| const IFRAME_SANDBOX_URL = 'http://localhost:63315/iframe/test/index.html'; | ||
|
|
||
| describe('IframeExecutionService', () => { | ||
| it('can boot', async () => { | ||
|
|
@@ -88,7 +91,7 @@ describe('IframeExecutionService', () => { | |
| }); | ||
|
|
||
| it('can detect outbound requests', async () => { | ||
| expect.assertions(4); | ||
| expect.assertions(5); | ||
|
Comment on lines
-91
to
+94
Member
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. This number was wrong initially, it seems WDIO just ignores it. |
||
|
|
||
| const { service, messenger } = createService(IframeExecutionService, { | ||
| iframeUrl: new URL(IFRAME_URL), | ||
|
|
@@ -157,11 +160,11 @@ describe('IframeExecutionService', () => { | |
| // Creates an iframe attempts to access the iframe created by the execution | ||
| // service. This should fail due to the sandboxing. | ||
| const testFrame = document.createElement('iframe'); | ||
| testFrame.src = `${IFRAME_URL}/test/sandbox`; | ||
| testFrame.src = IFRAME_SANDBOX_URL; | ||
| document.body.appendChild(testFrame); | ||
|
|
||
| expect(await message).toContain( | ||
| 'Failed to access document of the snap iframe: SecurityError', | ||
| 'Failed to access document of the snap iframe.', | ||
| ); | ||
| }); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,26 +11,35 @@ | |
| <script> | ||
| (function () { | ||
| try { | ||
| const snap = top.querySelector('iframe[data-testid="snaps-iframe"]'); | ||
| // Parent is the iframe created by Vitest containing the Snap iframe | ||
| // and this page as a sibling iframe. We try to access the Snap iframe | ||
| // from the parent iframe. | ||
| const snap = parent.document.querySelector( | ||
| 'iframe[data-testid="snaps-iframe"]', | ||
| ); | ||
| if (!snap) { | ||
| window.parent.postMessage('Failed to find snap iframe.', '*'); | ||
| window.parent.postMessage('Failed to find Snap iframe.', '*'); | ||
| return; | ||
| } | ||
|
|
||
| // We try to access the snap iframe document. If this works, it means | ||
| // that the iframe is not sandboxed. Otherwise it will throw an error. | ||
| snap.document; | ||
|
|
||
| window.parent.postMessage( | ||
| 'Same origin frames access worked which might suggest sandboxing failed.', | ||
| '*', | ||
| ); | ||
|
Comment on lines
-14
to
-27
Member
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. This test was wrong in the first place. The iframe was served from a different origin, and trying to access
So the I confirmed that the new test properly tests the sandboxing behaviour, as it's now served from the same origin. Removing the sandbox property from the iframe now results in a test failure. |
||
| if (snap.contentDocument === null) { | ||
| window.parent.postMessage( | ||
| 'Failed to access document of the snap iframe.', | ||
| '*', | ||
| ); | ||
| return; | ||
| } | ||
| } catch (e) { | ||
| window.parent.postMessage( | ||
| `Failed to access document of the snap iframe: ${e.toString()}`, | ||
| `An unknown error occurred: ${e.toString()}`, | ||
| '*', | ||
| ); | ||
| } | ||
|
|
||
| // Fall through case, which should not happen. | ||
| window.parent.postMessage('Unexpected success.', '*'); | ||
| })(); | ||
| </script> | ||
| </body> | ||
|
|
||
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.
How will this work locally? Do you need to run any commands besides
yarnandyarn testto run browser tests locally?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.
Yes, Vitest will tell you to run
yarn playwright installon the first time runningyarn test.