-
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
Changes from 24 commits
6d66ea1
f5dba5c
8558906
54a12f1
8e7ab08
ce49951
3a117b0
701e1c9
36f7d7b
747a4e7
0b1269e
ef2c178
6296b80
2b643c9
763766f
b552779
838e6d9
be54697
65b6045
46760b6
5f360d8
2b96682
1034418
ddeadaf
d9bb0f2
f1cf4a7
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 |
|---|---|---|
|
|
@@ -401,6 +401,34 @@ describe('ManifestPlugin', () => { | |
| assert.strictEqual(zipEntries.has('filename.js.map'), false); | ||
| } | ||
| }); | ||
|
|
||
| it('sets build_id in the emitted manifest when setBuildId is enabled', async () => { | ||
| const { compiler, compilation, promise } = mockWebpack([], [], []); | ||
| compiler.context = join(__dirname, 'fixtures/ManifestPlugin/empty'); | ||
| compilation.fullHash = 'test-full-hash'; | ||
|
|
||
| const manifestPlugin = new ManifestPlugin({ | ||
| browsers: ['chrome', 'firefox'], | ||
| manifest_version: 3, | ||
| version: '1.0.0.0', | ||
| versionName: '1.0.0', | ||
| description: null, | ||
| buildType: 'main', | ||
| zip: false, | ||
| setBuildId: true, | ||
| }); | ||
|
|
||
| manifestPlugin.apply(compiler); | ||
| await promise; | ||
|
|
||
| for (const browser of ['chrome', 'firefox'] as const) { | ||
| const manifest = JSON.parse( | ||
| compilation.assets[`${browser}/manifest.json`].source().toString(), | ||
| ); | ||
|
|
||
| assert.strictEqual(manifest.build_id, 'test-full-hash'); | ||
| } | ||
| }); | ||
| }); | ||
|
|
||
| describe('zip helpers', () => { | ||
|
|
@@ -775,6 +803,11 @@ describe('ManifestPlugin', () => { | |
| 'overridden_by_beta', | ||
| 'should override base property with beta value', | ||
| ); | ||
| assert.strictEqual( | ||
| json.build_id, | ||
| undefined, | ||
| 'should not emit build_id unless setBuildId is enabled', | ||
| ); | ||
| }); | ||
|
|
||
| it('should apply build type browser manifest overrides on top of all previous layers', async () => { | ||
|
|
@@ -1562,19 +1595,17 @@ describe('ManifestPlugin', () => { | |
| ); | ||
|
|
||
| // Simulate a watch rebuild where no watched files were modified | ||
| (compiler as unknown as Record<string, unknown>).modifiedFiles = new Set([ | ||
| '/some/unrelated/file.ts', | ||
| ]); | ||
| compiler.modifiedFiles = new Set(['/some/unrelated/file.ts']); | ||
|
|
||
| // Trigger a second compilation so resolveEntrypoints runs again | ||
| const { compilation: compilation2, promise: promise2 } = mockWebpack( | ||
| [], | ||
| [], | ||
| [], | ||
| ); | ||
| (compilation2 as unknown as Record<string, unknown>).compiler = compiler; | ||
| compilation2.compiler = compiler; | ||
|
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. i fixed the types i guess, as these casts weren't needed anymore. |
||
| // eslint-disable-next-line dot-notation | ||
| plugin['hookIntoPipelines'](compilation2 as unknown as Compilation); | ||
| plugin['hookIntoPipelines'](compilation2); | ||
| await promise2; | ||
|
|
||
| // Manifest reference should be the same since prepareManifests was NOT called | ||
|
|
@@ -1615,20 +1646,18 @@ describe('ManifestPlugin', () => { | |
|
|
||
| // Simulate a watch rebuild where the base manifest was modified. | ||
| // resolveEntrypoints checks compiler.modifiedFiles. | ||
| (compiler as unknown as Record<string, unknown>).modifiedFiles = new Set([ | ||
| baseManifestDep, | ||
| ]); | ||
| compiler.modifiedFiles = new Set([baseManifestDep]); | ||
|
|
||
| // Run apply again to trigger a new compilation with modifiedFiles set | ||
| const { compilation: compilation2, promise: promise2 } = mockWebpack( | ||
| [], | ||
| [], | ||
| [], | ||
| ); | ||
| (compilation2 as unknown as Record<string, unknown>).compiler = compiler; | ||
| compilation2.compiler = compiler; | ||
| // hookIntoPipelines registers processAssets which calls resolveEntrypoints | ||
| // eslint-disable-next-line dot-notation | ||
| plugin['hookIntoPipelines'](compilation2 as unknown as Compilation); | ||
| plugin['hookIntoPipelines'](compilation2); | ||
| await promise2; | ||
|
|
||
| // Should still produce valid output (manifests were re-read from disk) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,7 +10,7 @@ import { | |
| Compiler, | ||
| WebpackPluginInstance, | ||
| } 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 +25,55 @@ function getWebpackInstance(config: Configuration) { | |
| return webpack(config); | ||
| } | ||
|
|
||
| async function withWatching<T>( | ||
| config: Configuration, | ||
| callback: ( | ||
| waitForBuild: (trigger?: () => void) => Promise<void>, | ||
| ) => Promise<T>, | ||
| ) { | ||
| const compiler = webpack(config); | ||
| // @ts-expect-error - Node types need to be updated. | ||
|
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. I have a PR i'm working on already for this. Since it'll be fixed at that time, the |
||
| let build = Promise.withResolvers<void>(); | ||
davidmurdoch marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| const watchHandle = compiler.watch({}, (error, stats) => { | ||
| if (error) { | ||
| build.reject(error); | ||
| return; | ||
| } | ||
| if (!stats) { | ||
| build.reject( | ||
| new Error('Webpack finished watch build without returning stats.'), | ||
| ); | ||
| return; | ||
| } | ||
| if (stats.hasErrors()) { | ||
| build.reject(new Error('Webpack watch build failed.')); | ||
| return; | ||
| } | ||
| build.resolve(); | ||
| }); | ||
cursor[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| assert(watchHandle, 'Webpack did not return a watch handle.'); | ||
| const watching = watchHandle; | ||
|
|
||
| const waitForBuild = (trigger?: () => void) => { | ||
| if (!trigger) { | ||
| return build.promise; | ||
| } | ||
| // @ts-expect-error - Node types need to be updated. | ||
| build = Promise.withResolvers<void>(); | ||
| trigger(); | ||
davidmurdoch marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| watching.invalidate(); | ||
| return build.promise; | ||
| }; | ||
|
|
||
| try { | ||
| return await callback(waitForBuild); | ||
| } finally { | ||
| await new Promise<void>((resolveClose, rejectClose) => | ||
| watching.close((error) => (error ? rejectClose(error) : 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 +215,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 +306,85 @@ ${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 :-( |
||
| using tempDirectory = fs.mkdtempDisposableSync( | ||
| join(tmpdir(), 'manifest-plugin-watch-test-'), | ||
| ); | ||
| const manifestDirectory = join(tempDirectory.path, 'manifest', 'v3'); | ||
| const sourceFilePath = join(tempDirectory.path, 'index.js'); | ||
| const outputPath = join(tempDirectory.path, 'dist'); | ||
| const manifestPath = join(outputPath, 'chrome', 'manifest.json'); | ||
|
|
||
| const readBuildId = () => | ||
| ( | ||
| JSON.parse(fs.readFileSync(manifestPath, 'utf8')) as Manifest & { | ||
| build_id?: string; | ||
| } | ||
| ).build_id; | ||
|
|
||
| const manifest = { manifest_version: 3, name: 'test', version: '1.0.0' }; | ||
| const baseManifestPath = join(manifestDirectory, '_base.json'); | ||
| const writeSource = (source: string | NodeJS.ArrayBufferView) => | ||
| fs.writeFileSync(sourceFilePath, source); | ||
| fs.mkdirSync(manifestDirectory, { recursive: true }); | ||
| fs.writeFileSync(baseManifestPath, JSON.stringify(manifest)); | ||
| writeSource('console.log("v1");\n'); | ||
|
|
||
| await withWatching( | ||
| { | ||
| mode: 'development', | ||
| context: tempDirectory.path, | ||
| entry: { app: sourceFilePath }, | ||
| output: { path: outputPath }, | ||
| 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, | ||
| }), | ||
| ], | ||
| }, | ||
| async (waitForBuild) => { | ||
| await waitForBuild(); | ||
| const firstBuildId = readBuildId(); | ||
| assert.ok(firstBuildId, 'expected initial build_id'); | ||
|
|
||
| await waitForBuild(() => writeSource(fs.readFileSync(sourceFilePath))); | ||
| const secondBuildId = readBuildId(); | ||
|
|
||
| await waitForBuild(() => writeSource('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', | ||
| ); | ||
| }, | ||
| ); | ||
| }); | ||
|
|
||
| it('keeps the MYX provider ignore only while the warning still exists', async () => { | ||
| const config: Configuration = getWebpackConfig(); | ||
| const myxWarningPattern = /MYXProvider\.mjs/u; | ||
|
|
||
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.
webpack uses a hash of the compilation, but that would be very too slow and complicated here in browserify, so I just went with a random ID for every non-manifest file change.