-
Notifications
You must be signed in to change notification settings - Fork 5.5k
test(firefox): extract xpi cache helper #41453
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
Merged
Changes from 8 commits
Commits
Show all changes
26 commits
Select commit
Hold shift + click to select a range
6d66ea1
test(firefox): spike xpi cache metadata in zip comment
davidmurdoch f5dba5c
test(firefox): simplify xpi manifest patching
davidmurdoch 8558906
test(firefox): simplify xpi archive walk
davidmurdoch 54a12f1
test(firefox): extract xpi cache helper
davidmurdoch 8e7ab08
Merge branch 'main' into codex/firefox-xpi-comment-metadata-spike
davidmurdoch ce49951
test(firefox): remove xpi benchmark script
davidmurdoch 3a117b0
perf(firefox): compress cached xpi payload
davidmurdoch 701e1c9
build(webpack): add test-only manifest build ids
davidmurdoch 36f7d7b
test(webpack): simplify manifest build id watch test
davidmurdoch 747a4e7
chore(e2e): remove unused xpi helper import
davidmurdoch 0b1269e
fix(e2e): validate cached manifest size
davidmurdoch ef2c178
fix(e2e): invalidate xpi cache on test rebuilds
davidmurdoch 6296b80
Merge branch 'main' into codex/firefox-xpi-comment-metadata-spike
davidmurdoch 2b643c9
docs(e2e): trim xpi helper jsdoc
davidmurdoch 763766f
fix(e2e): close xpi stream before rename
davidmurdoch b552779
fix(build): serialize manifest watch updates
davidmurdoch 838e6d9
types
davidmurdoch be54697
fix(e2e): rebuild xpi when build changes
davidmurdoch 65b6045
Merge branch 'main' into codex/firefox-xpi-comment-metadata-spike
davidmurdoch 46760b6
Merge branch 'main' into codex/firefox-xpi-comment-metadata-spike
davidmurdoch 5f360d8
clarify
davidmurdoch 2b96682
test(webpack): fail watch helper on build errors
davidmurdoch 1034418
Merge branch 'main' into codex/firefox-xpi-comment-metadata-spike
davidmurdoch ddeadaf
chore(build): ignore manifest watch events
davidmurdoch d9bb0f2
refactor(webpack): simplify manifest build id emit
davidmurdoch f1cf4a7
Merge branch 'main' into codex/firefox-xpi-comment-metadata-spike
davidmurdoch 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
Some comments aren't visible on the classic Files Changed page.
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
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 |
|---|---|---|
|
|
@@ -9,8 +9,9 @@ import { | |
| webpack, | ||
| Compiler, | ||
| WebpackPluginInstance, | ||
| type Stats, | ||
| } from 'webpack'; | ||
| import { noop } from '../utils/helpers'; | ||
| import { noop, type Manifest } from '../utils/helpers'; | ||
| import { ManifestPlugin } from '../utils/plugins/ManifestPlugin'; | ||
| import { getLatestCommit } from '../utils/git'; | ||
| import { ManifestPluginOptions } from '../utils/plugins/ManifestPlugin/types'; | ||
|
|
@@ -25,6 +26,88 @@ function getWebpackInstance(config: Configuration) { | |
| return webpack(config); | ||
| } | ||
|
|
||
| function createDeferred<T>() { | ||
| let resolveDeferred: ((value: T | PromiseLike<T>) => void) | undefined; | ||
| let rejectDeferred: ((reason?: unknown) => void) | undefined; | ||
| const promise = new Promise<T>((nextResolve, nextReject) => { | ||
| resolveDeferred = nextResolve; | ||
| rejectDeferred = nextReject; | ||
| }); | ||
|
|
||
| return { | ||
| promise, | ||
| resolve(value: T | PromiseLike<T>) { | ||
| if (!resolveDeferred) { | ||
| throw new Error('Deferred promise was not initialized.'); | ||
| } | ||
| resolveDeferred(value); | ||
| }, | ||
| reject(reason?: unknown) { | ||
| if (!rejectDeferred) { | ||
| throw new Error('Deferred promise was not initialized.'); | ||
| } | ||
| rejectDeferred(reason); | ||
| }, | ||
| }; | ||
| } | ||
|
|
||
| async function withWatching<T>( | ||
| config: Configuration, | ||
| callback: (context: { | ||
| waitForNextBuild: (trigger?: () => void) => Promise<Stats>; | ||
| invalidateAfter: (trigger: () => void) => Promise<Stats>; | ||
| }) => Promise<T>, | ||
| ) { | ||
| const compiler = webpack(config); | ||
| let deferredBuild = createDeferred<Stats>(); | ||
| const watchHandle = compiler.watch({}, (error, stats) => { | ||
| if (error) { | ||
| deferredBuild.reject(error); | ||
| return; | ||
| } | ||
| if (!stats) { | ||
| deferredBuild.reject( | ||
| new Error('Webpack finished watch build without returning stats.'), | ||
| ); | ||
| return; | ||
| } | ||
| deferredBuild.resolve(stats); | ||
| }); | ||
cursor[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if (!watchHandle) { | ||
| throw new Error('Webpack did not return a watch handle.'); | ||
| } | ||
| const watching = watchHandle; | ||
|
|
||
| const waitForNextBuild = async (trigger?: () => void) => { | ||
| const currentBuild = deferredBuild; | ||
| trigger?.(); | ||
| const stats = await currentBuild.promise; | ||
| deferredBuild = createDeferred<Stats>(); | ||
| return stats; | ||
| }; | ||
|
|
||
| try { | ||
| return await callback({ | ||
| waitForNextBuild, | ||
| invalidateAfter: async (trigger) => | ||
| await waitForNextBuild(() => { | ||
| trigger(); | ||
| watching.invalidate(); | ||
| }), | ||
| }); | ||
| } finally { | ||
| await new Promise<void>((resolveClose, rejectClose) => { | ||
| watching.close((error) => { | ||
| if (error) { | ||
| rejectClose(error); | ||
| return; | ||
| } | ||
| resolveClose(); | ||
| }); | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * These tests are aimed at testing conditional branches in webpack.config.ts. | ||
| * These tests do *not* test the actual webpack build process itself, or that | ||
|
|
@@ -166,6 +249,7 @@ ${Object.entries(env) | |
| ], | ||
| key: CHROME_MANIFEST_KEY_NON_PRODUCTION, | ||
| }); | ||
| assert.strictEqual(manifestPlugin.options.setBuildId, false); | ||
| assert.strictEqual(manifestPlugin.options.zip, false); | ||
| const manifestOpts = manifestPlugin.options as ManifestPluginOptions<true>; | ||
| assert.strictEqual(manifestOpts.zipOptions, undefined); | ||
|
|
@@ -256,6 +340,114 @@ ${Object.entries(env) | |
| assert.strictEqual(instance.options.devtool, false); | ||
| }); | ||
|
|
||
| it('enables manifest build IDs for test builds', () => { | ||
| const config: Configuration = getWebpackConfig(['--test']); | ||
| const instance = getWebpackInstance(config); | ||
| const manifestPlugin = instance.options.plugins.find( | ||
| (plugin) => plugin && plugin.constructor.name === 'ManifestPlugin', | ||
| ) as ManifestPlugin<boolean>; | ||
|
|
||
| assert(manifestPlugin, 'Manifest plugin should be present'); | ||
| assert.strictEqual(manifestPlugin.options.setBuildId, true); | ||
| }); | ||
|
|
||
| it('keeps build_id stable for no-op watch rebuilds and changes it for real edits', async () => { | ||
|
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. a bit complicated, but I couldn't figure out how to make it any less so :-( |
||
| const tempDirectory = fs.mkdtempSync( | ||
| join(tmpdir(), 'manifest-plugin-watch-test-'), | ||
| ); | ||
| const manifestDirectory = join(tempDirectory, 'manifest', 'v3'); | ||
| const sourceFilePath = join(tempDirectory, 'index.js'); | ||
| const outputPath = join(tempDirectory, 'dist'); | ||
|
|
||
| const readBuildId = () => | ||
| ( | ||
| JSON.parse( | ||
| fs.readFileSync(join(outputPath, 'chrome', 'manifest.json'), 'utf8'), | ||
| ) as Manifest & { build_id?: string } | ||
| ).build_id; | ||
|
|
||
| try { | ||
| fs.mkdirSync(manifestDirectory, { recursive: true }); | ||
| fs.writeFileSync( | ||
| join(manifestDirectory, '_base.json'), | ||
| JSON.stringify( | ||
| { | ||
| manifest_version: 3, | ||
| name: 'watch-test', | ||
| version: '1.0.0', | ||
| }, | ||
| null, | ||
| 2, | ||
| ), | ||
| ); | ||
| fs.writeFileSync(sourceFilePath, 'console.log("v1");\n'); | ||
|
|
||
| await withWatching( | ||
| { | ||
| mode: 'development', | ||
| context: tempDirectory, | ||
| cache: false, | ||
| devtool: false, | ||
| entry: { | ||
| app: { | ||
| import: [sourceFilePath], | ||
| filename: 'scripts/app.js', | ||
| }, | ||
| }, | ||
| output: { | ||
| path: outputPath, | ||
| clean: true, | ||
| filename: '[name].js', | ||
| }, | ||
| plugins: [ | ||
| new ManifestPlugin({ | ||
| browsers: ['chrome'], | ||
| manifest_version: 3, | ||
| version: '1.0.0.0', | ||
| versionName: '1.0.0', | ||
| description: null, | ||
| buildType: 'main', | ||
| zip: false, | ||
| setBuildId: true, | ||
| }), | ||
| ], | ||
| infrastructureLogging: { | ||
| level: 'error', | ||
| }, | ||
| }, | ||
| async ({ waitForNextBuild, invalidateAfter }) => { | ||
| await waitForNextBuild(); | ||
| const firstBuildId = readBuildId(); | ||
| assert.ok(firstBuildId, 'expected initial build_id'); | ||
|
|
||
| await invalidateAfter(() => { | ||
| const current = fs.readFileSync(sourceFilePath, 'utf8'); | ||
| fs.writeFileSync(sourceFilePath, current); | ||
| }); | ||
| const secondBuildId = readBuildId(); | ||
|
|
||
| await invalidateAfter(() => { | ||
| fs.writeFileSync(sourceFilePath, 'console.log("v2");\n'); | ||
| }); | ||
| const thirdBuildId = readBuildId(); | ||
|
|
||
| assert.strictEqual( | ||
| secondBuildId, | ||
| firstBuildId, | ||
| 'expected no-op watch rebuild to keep the same build_id', | ||
| ); | ||
| assert.notStrictEqual( | ||
| thirdBuildId, | ||
| secondBuildId, | ||
| 'expected real file changes to produce a new build_id', | ||
| ); | ||
| }, | ||
| ); | ||
| } finally { | ||
| fs.rmSync(tempDirectory, { recursive: true, force: true }); | ||
| } | ||
| }); | ||
|
|
||
| it('keeps the MYX provider ignore only while the warning still exists', async () => { | ||
| const config: Configuration = getWebpackConfig(); | ||
| const myxWarningPattern = /MYXProvider\.mjs/u; | ||
|
|
||
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
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
Oops, something went wrong.
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.
i fixed the types i guess, as these casts weren't needed anymore.