diff --git a/src/cmd/run.js b/src/cmd/run.js index dcfd52d682..e8b2754d5c 100644 --- a/src/cmd/run.js +++ b/src/cmd/run.js @@ -62,8 +62,13 @@ export default async function run( getValidatedManifest = defaultGetValidatedManifest, } = {}, ) { - sourceDir = path.resolve(sourceDir); - log.info(`Running web extension from ${sourceDir}`); + // Most sourceDir values across the codebase accept only one string, here we + // may accept multiple. + if (!Array.isArray(sourceDir)) { + sourceDir = [sourceDir]; + } + sourceDir = sourceDir.map((s) => path.resolve(s)); + log.info(`Running web extension from ${sourceDir.join(', ')}`); if (preInstall) { log.info( "Disabled auto-reloading because it's not possible with " + @@ -88,8 +93,6 @@ export default async function run( // object containing one or more preferences. const customChromiumPrefs = chromiumPref; - const manifestData = await getValidatedManifest(sourceDir); - const profileDir = firefoxProfile || chromiumProfile; if (profileCreateIfMissing) { @@ -112,11 +115,17 @@ export default async function run( const commonRunnerParams = { // Common options. - extensions: [{ sourceDir, manifestData }], + extensions: [], // Populated below from sourceDir (--source-dir) keepProfileChanges, startUrl, args, }; + for (const sourceDirPath of sourceDir) { + commonRunnerParams.extensions.push({ + sourceDir: sourceDirPath, + manifestData: await getValidatedManifest(sourceDirPath), + }); + } if (!target || target.length === 0 || target.includes('firefox-desktop')) { const firefoxDesktopRunnerParams = { diff --git a/src/extension-runners/chromium.js b/src/extension-runners/chromium.js index f04faff69a..d3887035a0 100644 --- a/src/extension-runners/chromium.js +++ b/src/extension-runners/chromium.js @@ -550,13 +550,42 @@ export class ChromiumExtensionRunner { * Reloads a single extension, collect any reload error and resolves to * an array composed by a single ExtensionRunnerReloadResult object. */ - async reloadExtensionBySourceDir( - extensionSourceDir, // eslint-disable-line no-unused-vars - ) { - // TODO(rpl): detect the extension ids assigned to the - // target extensions and map it to the extensions source dir - // (https://github.com/mozilla/web-ext/issues/1687). - return this.reloadAllExtensions(); + async reloadExtensionBySourceDir(extensionSourceDir) { + if ( + // For a long time we simply reloaded all extensions without checking the + // directory. Perhaps we can drop this check? + this.params.extensions.length === 1 || + // Support for old Chrome (<125) is on a best effort basis, and we simply + // reload them all instead of a specific one. If anyone ever wants to + // implement this, see https://github.com/mozilla/web-ext/issues/1687. + this.forceUseDeprecatedLoadExtension + ) { + // Common simple case, only one extension. For a long time this logic + // only worked with one (https://github.com/mozilla/web-ext/issues/1687). + return this.reloadAllExtensions(); + } + if ( + this.params.extensions.some((x) => x.sourceDir === extensionSourceDir) + ) { + try { + await this.cdp.sendCommand('Extensions.loadUnpacked', { + path: extensionSourceDir, + }); + } catch (e) { + log.error( + `Failed to load extension at ${extensionSourceDir}: ${e.message}`, + ); + } + } else { + log.error(`Unrecognized extensionSourceDir: ${extensionSourceDir}`); + } + + process.stdout.write( + `\rLast extension reload: ${new Date().toTimeString()}`, + ); + log.debug('\n'); + + return [{ runnerName: this.getName() }]; } /** diff --git a/src/extension-runners/index.js b/src/extension-runners/index.js index ca81b68c58..9ca6a94b91 100644 --- a/src/extension-runners/index.js +++ b/src/extension-runners/index.js @@ -230,19 +230,36 @@ export function defaultReloadStrategy( log.debug('Input has been disabled because of noInput==true'); } - const watcher = createWatcher({ - reloadExtension: (watchedSourceDir) => { - extensionRunner.reloadExtensionBySourceDir(watchedSourceDir); - }, - sourceDir, - watchFile, - watchIgnored, - artifactsDir, - ignoreFiles, - }); + const watchers = []; + const doCreateWatcher = (sourceDirPath) => { + const watcher = createWatcher({ + reloadExtension: (watchedSourceDir) => { + extensionRunner.reloadExtensionBySourceDir(watchedSourceDir); + }, + sourceDir: sourceDirPath, + watchFile, + watchIgnored, + artifactsDir, + ignoreFiles, + }); + watchers.push(watcher); + }; + doCreateWatcher(Array.isArray(sourceDir) ? sourceDir[0] : sourceDir); + if (Array.isArray(sourceDir) && !watchFile) { + // Need a watcher for each individual source directory, unless --watch-file + // is specified, in which case we do not monitor any source directory. + for (const sourceDirPath of sourceDir.slice(1)) { + // TODO: It does not make sense to use the same artifactsDir for all + // extensions. Should we use none or the same for each instead? + // For now a user can ignore them anyway with the --ignore-files flag. + doCreateWatcher(sourceDirPath); + } + } extensionRunner.registerCleanup(() => { - watcher.close(); + for (const watcher of watchers) { + watcher.close(); + } if (allowInput) { if (isTTY(stdin)) { setRawMode(stdin, false); diff --git a/src/program.js b/src/program.js index d7c1414c3e..da6e11d022 100644 --- a/src/program.js +++ b/src/program.js @@ -416,11 +416,19 @@ Example: $0 --help run. program.setGlobalOptions({ 'source-dir': { alias: 's', - describe: 'Web extension source directory.', + describe: + 'Web extension source directory. The "run" command accepts multiple ' + + 'directories, the "build" and "sign" commands accept only one.', default: process.cwd(), requiresArg: true, type: 'string', - coerce: (arg) => arg ?? undefined, + coerce: (arg) => { + arg = arg ?? undefined; + if (Array.isArray(arg) && !program.programArgv.includes('run')) { + throw new UsageError('Multiple --source-dir are not allowed.'); + } + return arg; + }, }, 'artifacts-dir': { alias: 'a', diff --git a/tests/unit/test-cmd/test.run.js b/tests/unit/test-cmd/test.run.js index 140251ca59..6f5344f71d 100644 --- a/tests/unit/test-cmd/test.run.js +++ b/tests/unit/test-cmd/test.run.js @@ -463,4 +463,30 @@ describe('run', () => { ); }); }); + + describe('multiple sourceDir parameters', () => { + it('passes multiple --source-dir parameters to runner', async () => { + const cmd = await prepareRun(); + const { reloadStrategy } = cmd.options; + + const sourceDir1 = fixturePath('minimal-web-ext'); + const sourceDir2 = fixturePath('minimal-localizable-web-ext'); + await cmd.run({ sourceDir: [sourceDir1, sourceDir2], noReload: false }); + + sinon.assert.calledOnce(desktopRunnerStub); + const { extensions } = desktopRunnerStub.firstCall.args[0]; + assert.equal(extensions.length, 2); + assert.equal(extensions[0].sourceDir, sourceDir1); + assert.equal(extensions[0].manifestData.name, 'Minimal Extension'); + assert.equal(extensions[1].sourceDir, sourceDir2); + assert.equal(extensions[1].manifestData.name, '__MSG_extensionName__'); + + sinon.assert.calledOnce(reloadStrategy); + sinon.assert.calledWithMatch(reloadStrategy, { + sourceDir: [sourceDir1, sourceDir2], + }); + // tests/unit/test-extension-runners/test.extension-runners.js verifies + // that multiple watchers are created for each sourceDir. + }); + }); }); diff --git a/tests/unit/test-extension-runners/test.chromium.js b/tests/unit/test-extension-runners/test.chromium.js index 3f0cc7b37f..970462f5e5 100644 --- a/tests/unit/test-extension-runners/test.chromium.js +++ b/tests/unit/test-extension-runners/test.chromium.js @@ -2,7 +2,7 @@ import path from 'path'; import EventEmitter from 'events'; import { assert } from 'chai'; -import { describe, it, beforeEach } from 'mocha'; +import { describe, it, beforeEach, afterEach } from 'mocha'; import deepcopy from 'deepcopy'; import fs from 'fs-extra'; import * as sinon from 'sinon'; @@ -659,17 +659,61 @@ describe('util/extension-runners/chromium', async () => { }); }); - describe('reloadAllExtensions', () => { + describe('running multiple extensions', () => { let runnerInstance; + let sendCommandSpy; beforeEach(async () => { - const { params } = prepareExtensionRunnerParams(); + const { params } = prepareExtensionRunnerParams({ + params: { + extensions: [ + { + sourceDir: '/fake/sourceDir1', + manifestData: deepcopy(basicManifest), + }, + { + sourceDir: '/fake/sourceDir2', + manifestData: deepcopy(basicManifest), + }, + { + sourceDir: '/fake/sourceDir3', + manifestData: deepcopy(basicManifest), + }, + ], + }, + }); runnerInstance = new ChromiumExtensionRunner(params); await runnerInstance.run(); + sendCommandSpy = sinon.spy(runnerInstance.cdp, 'sendCommand'); + }); + + afterEach(async () => { + sendCommandSpy.restore(); + sendCommandSpy = null; + await runnerInstance.exit(); + runnerInstance = null; }); - it('resolve when not client connected', async () => { + it('reloadAllExtensions() should reload them all', async () => { await runnerInstance.reloadAllExtensions(); + sinon.assert.calledThrice(sendCommandSpy); + sinon.assert.calledWithMatch(sendCommandSpy, 'Extensions.loadUnpacked', { + path: '/fake/sourceDir1', + }); + sinon.assert.calledWithMatch(sendCommandSpy, 'Extensions.loadUnpacked', { + path: '/fake/sourceDir2', + }); + sinon.assert.calledWithMatch(sendCommandSpy, 'Extensions.loadUnpacked', { + path: '/fake/sourceDir3', + }); + }); + + it('reloadExtensionBySourceDir() should reload one extension', async () => { + await runnerInstance.reloadExtensionBySourceDir('/fake/sourceDir2'); + sinon.assert.calledOnce(sendCommandSpy); + sinon.assert.calledWithMatch(sendCommandSpy, 'Extensions.loadUnpacked', { + path: '/fake/sourceDir2', + }); }); }); }); diff --git a/tests/unit/test-extension-runners/test.extension-runners.js b/tests/unit/test-extension-runners/test.extension-runners.js index 5ca7d34ab8..2a19ab2748 100644 --- a/tests/unit/test-extension-runners/test.extension-runners.js +++ b/tests/unit/test-extension-runners/test.extension-runners.js @@ -602,5 +602,83 @@ describe('util/extension-runners', () => { exitKeypressLoop(fakeStdin); } }); + + describe('running multiple extensions', () => { + const sourceDir1 = '/fake/sourceDir1'; + const sourceDir2 = '/fake/sourceDir2'; + const sourceDir3 = '/fake/sourceDir3'; + + it('creates multiple watchers for each sourceDir', async () => { + const { + // createWatcher mock returns same watcher for each test. We only + // validate the parameters so it does not matter that the watchers + // are shared. + watcher, + createWatcher, + extensionRunner, + reloadStrategy, + } = prepare({ + stubExtensionRunner: { + registerCleanup() {}, + }, + }); + reloadStrategy({ + sourceDir: [sourceDir1, sourceDir2, sourceDir3], + watchFile: null, // sourceDir ignored if watchFile is set. + }); + sinon.assert.calledThrice(createWatcher); + sinon.assert.calledWithMatch(createWatcher, { sourceDir: sourceDir1 }); + sinon.assert.calledWithMatch(createWatcher, { sourceDir: sourceDir2 }); + sinon.assert.calledWithMatch(createWatcher, { sourceDir: sourceDir3 }); + + const { registerCleanup } = extensionRunner; + + sinon.assert.calledOnce(registerCleanup); + const registeredCb = registerCleanup.firstCall.args[0]; + registeredCb(); + + sinon.assert.calledThrice(watcher.close); + }); + + it('creates one watcher when --watch-file is specified', async () => { + const { createWatcher, reloadStrategy } = prepare(); + // watchFile is specified by prepare(). + reloadStrategy({ sourceDir: [sourceDir1, sourceDir2, sourceDir3] }); + sinon.assert.calledOnce(createWatcher); + sinon.assert.calledWithMatch(createWatcher, { sourceDir: sourceDir1 }); + }); + + it('reloads one extension when a change is detected', async () => { + const { extensionRunner, reloadStrategy } = prepare({ + stubExtensionRunner: { + reloadExtensionBySourceDir() {}, + }, + }); + const onSourceChange = sinon.spy(() => {}); + const createWatcher = sinon.spy((opts) => + defaultWatcherCreator({ ...opts, onSourceChange }), + ); + + reloadStrategy( + { + sourceDir: [sourceDir1, sourceDir2, sourceDir3], + watchFile: null, // sourceDir ignored if watchFile is set. + }, + { createWatcher }, + ); + + sinon.assert.calledThrice(createWatcher); + sinon.assert.calledThrice(onSourceChange); + onSourceChange.secondCall.args[0].onChange(); + + const { reloadExtensionBySourceDir } = extensionRunner; + + sinon.assert.calledOnce(reloadExtensionBySourceDir); + sinon.assert.calledWithMatch( + reloadExtensionBySourceDir, + sinon.match(sourceDir2), + ); + }); + }); }); }); diff --git a/tests/unit/test-extension-runners/test.firefox-android.js b/tests/unit/test-extension-runners/test.firefox-android.js index 7f168646c2..33b80ea6b0 100644 --- a/tests/unit/test-extension-runners/test.firefox-android.js +++ b/tests/unit/test-extension-runners/test.firefox-android.js @@ -1,7 +1,7 @@ import EventEmitter from 'events'; import { assert } from 'chai'; -import { describe, it } from 'mocha'; +import { describe, it, beforeEach, afterEach } from 'mocha'; import deepcopy from 'deepcopy'; import * as sinon from 'sinon'; @@ -881,4 +881,70 @@ describe('util/extension-runners/firefox-android', () => { consoleStream.stopCapturing(); }); }); + + describe('running multiple extensions', () => { + let runnerInstance; + + beforeEach(async () => { + const { params } = prepareSelectedDeviceAndAPKParams({ + fakeRemoteFirefox: { + installTemporaryAddon: sinon.spy(async (adbExtensionPath) => { + // Transforms /fake/built/fake/sourceDir1.xpi to 'id@sourceDir1'. + const id = `id@${adbExtensionPath.split('/').pop().split('.')[0]}`; + return { addon: { id } }; + }), + }, + }); + params.extensions = [ + { + sourceDir: '/fake/sourceDir1', + manifestData: deepcopy(basicManifest), + }, + { + sourceDir: '/fake/sourceDir2', + manifestData: deepcopy(basicManifest), + }, + { + sourceDir: '/fake/sourceDir3', + manifestData: deepcopy(basicManifest), + }, + ]; + params.buildSourceDir = sinon.spy(async (sourceDir) => { + return { extensionPath: `/fake/built/${sourceDir}.zip` }; + }); + runnerInstance = new FirefoxAndroidExtensionRunner(params); + await runnerInstance.run(); + }); + + afterEach(async () => { + await runnerInstance.exit(); + runnerInstance = null; + }); + + it('reloadAllExtensions() should reload them all', async () => { + await runnerInstance.reloadAllExtensions(); + sinon.assert.calledThrice(runnerInstance.remoteFirefox.reloadAddon); + sinon.assert.calledWithMatch( + runnerInstance.remoteFirefox.reloadAddon, + 'id@sourceDir1', + ); + sinon.assert.calledWithMatch( + runnerInstance.remoteFirefox.reloadAddon, + 'id@sourceDir2', + ); + sinon.assert.calledWithMatch( + runnerInstance.remoteFirefox.reloadAddon, + 'id@sourceDir3', + ); + }); + + it('reloadExtensionBySourceDir() should reload one extension', async () => { + await runnerInstance.reloadExtensionBySourceDir('/fake/sourceDir2'); + sinon.assert.calledOnce(runnerInstance.remoteFirefox.reloadAddon); + sinon.assert.calledWithMatch( + runnerInstance.remoteFirefox.reloadAddon, + 'id@sourceDir2', + ); + }); + }); }); diff --git a/tests/unit/test-extension-runners/test.firefox-desktop.js b/tests/unit/test-extension-runners/test.firefox-desktop.js index c8cabf5fb9..a8db145e14 100644 --- a/tests/unit/test-extension-runners/test.firefox-desktop.js +++ b/tests/unit/test-extension-runners/test.firefox-desktop.js @@ -1,5 +1,5 @@ import { assert } from 'chai'; -import { describe, it } from 'mocha'; +import { describe, it, beforeEach, afterEach } from 'mocha'; import deepcopy from 'deepcopy'; import * as sinon from 'sinon'; @@ -436,4 +436,69 @@ describe('util/extension-runners/firefox-desktop', () => { sinon.assert.called(remoteFirefox.reloadAddon); }); + + describe('running multiple extensions', () => { + let runnerInstance; + + beforeEach(async () => { + const { params } = prepareExtensionRunnerParams({ + fakeRemoteFirefox: { + installTemporaryAddon: sinon.spy(async (sourceDir) => { + // Transforms /fake/sourceDir1 to 'id@sourceDir1'. + const id = `id@${sourceDir.split('/').pop()}`; + return { addon: { id } }; + }), + }, + params: { + extensions: [ + { + sourceDir: '/fake/sourceDir1', + manifestData: deepcopy(basicManifest), + }, + { + sourceDir: '/fake/sourceDir2', + manifestData: deepcopy(basicManifest), + }, + { + sourceDir: '/fake/sourceDir3', + manifestData: deepcopy(basicManifest), + }, + ], + }, + }); + runnerInstance = new FirefoxDesktopExtensionRunner(params); + await runnerInstance.run(); + }); + + afterEach(async () => { + await runnerInstance.exit(); + runnerInstance = null; + }); + + it('reloadAllExtensions() should reload them all', async () => { + await runnerInstance.reloadAllExtensions(); + sinon.assert.calledThrice(runnerInstance.remoteFirefox.reloadAddon); + sinon.assert.calledWithMatch( + runnerInstance.remoteFirefox.reloadAddon, + 'id@sourceDir1', + ); + sinon.assert.calledWithMatch( + runnerInstance.remoteFirefox.reloadAddon, + 'id@sourceDir2', + ); + sinon.assert.calledWithMatch( + runnerInstance.remoteFirefox.reloadAddon, + 'id@sourceDir3', + ); + }); + + it('reloadExtensionBySourceDir() should reload one extension', async () => { + await runnerInstance.reloadExtensionBySourceDir('/fake/sourceDir2'); + sinon.assert.calledOnce(runnerInstance.remoteFirefox.reloadAddon); + sinon.assert.calledWithMatch( + runnerInstance.remoteFirefox.reloadAddon, + 'id@sourceDir2', + ); + }); + }); }); diff --git a/tests/unit/test.program.js b/tests/unit/test.program.js index e159aec3e8..b55abf6c3f 100644 --- a/tests/unit/test.program.js +++ b/tests/unit/test.program.js @@ -898,6 +898,46 @@ describe('program.main', () => { }); } }); + + describe('multiple --source-dir parameters', () => { + it('accepts multiple --source-dir in web-ext run', async () => { + const fakeCommands = fake(commands, { + run: () => Promise.resolve(), + }); + const opts = { commands: fakeCommands }; + + await execProgram(['run', '-s', 'ext1', '-s', 'ext2'], opts); + sinon.assert.calledWithMatch(fakeCommands.run, { + sourceDir: ['ext1', 'ext2'], + }); + }); + + it('rejects multiple --source-dir in web-ext build', () => { + const fakeCommands = fake(commands, { + build: () => Promise.resolve(), + }); + const opts = { commands: fakeCommands }; + + return execProgram(['build', '-s', 'ext1', '-s', 'ext2'], opts) + .then(makeSureItFails()) + .catch((error) => { + assert.match(error.message, /Multiple --source-dir are not allowed/); + }); + }); + + it('rejects multiple --source-dir in web-ext sign', () => { + const fakeCommands = fake(commands, { + build: () => Promise.resolve(), + }); + const opts = { commands: fakeCommands }; + + return execProgram(['sign', '-s', 'ext1', '-s', 'ext2'], opts) + .then(makeSureItFails()) + .catch((error) => { + assert.match(error.message, /Multiple --source-dir are not allowed/); + }); + }); + }); }); describe('program.defaultVersionGetter', () => {