diff --git a/package.json b/package.json index d6dad7aa9f..fa550e0dd4 100644 --- a/package.json +++ b/package.json @@ -61,7 +61,7 @@ "minimatch": "3.0.3", "mkdirp": "0.5.1", "mz": "2.6.0", - "node-firefox-connect": "1.2.0", + "node-firefox-connect": "1.2.0", "open": "0.0.5", "node-notifier": "5.0.2", "parse-json": "2.2.0", diff --git a/src/cmd/run.js b/src/cmd/run.js index 918ce7aab2..74510e51f9 100644 --- a/src/cmd/run.js +++ b/src/cmd/run.js @@ -1,4 +1,7 @@ /* @flow */ +import readline from 'readline'; +import tty from 'tty'; + import type FirefoxProfile from 'firefox-profile'; import type Watchpack from 'watchpack'; @@ -41,18 +44,17 @@ export type WatcherCreatorParams = {| sourceDir: string, artifactsDir: string, onSourceChange?: OnSourceChangeFn, - desktopNotifications?: typeof defaultDesktopNotifications, ignoreFiles?: Array, createFileFilter?: FileFilterCreatorFn, + addonReload: typeof defaultAddonReload, |}; export type WatcherCreatorFn = (params: WatcherCreatorParams) => Watchpack; export function defaultWatcherCreator( { - addonId, client, sourceDir, artifactsDir, ignoreFiles, + addonId, addonReload, client, sourceDir, artifactsDir, ignoreFiles, onSourceChange = defaultSourceWatcher, - desktopNotifications = defaultDesktopNotifications, createFileFilter = defaultFileFilterCreator, }: WatcherCreatorParams ): Watchpack { @@ -62,24 +64,37 @@ export function defaultWatcherCreator( return onSourceChange({ sourceDir, artifactsDir, - onChange: () => { - log.debug(`Reloading add-on ID ${addonId}`); - - return client.reloadAddon(addonId) - .catch((error) => { - log.error('\n'); - log.error(error.stack); - desktopNotifications({ - title: 'web-ext run: error occurred', - message: error.message, - }); - throw error; - }); - }, + onChange: () => addonReload({addonId, client}), shouldWatchFile: (file) => fileFilter.wantFile(file), }); } +export type ReloadParams = {| + addonId: string, + client: RemoteFirefox, + desktopNotifications?: typeof defaultDesktopNotifications, +|}; + +export async function defaultAddonReload( + { + addonId, client, + desktopNotifications = defaultDesktopNotifications, + }: ReloadParams +): Promise { + log.debug(`Reloading add-on ID ${addonId}`); + try { + await client.reloadAddon(addonId); + } catch (reloadError) { + log.error('\n'); + log.error(reloadError.stack); + desktopNotifications({ + title: 'web-ext run: error occurred', + message: reloadError.message, + }); + throw reloadError; + } +} + // defaultReloadStrategy types and implementation. @@ -94,8 +109,10 @@ export type ReloadStrategyParams = {| |}; export type ReloadStrategyOptions = {| + addonReload?: typeof defaultAddonReload, createWatcher?: WatcherCreatorFn, createFileFilter?: FileFilterCreatorFn, + stdin: stream$Readable, |}; export function defaultReloadStrategy( @@ -104,18 +121,50 @@ export function defaultReloadStrategy( sourceDir, artifactsDir, ignoreFiles, }: ReloadStrategyParams, { + addonReload = defaultAddonReload, createWatcher = defaultWatcherCreator, + stdin = process.stdin, }: ReloadStrategyOptions = {} ): void { - const watcher: Watchpack = ( - createWatcher({addonId, client, sourceDir, artifactsDir, ignoreFiles}) - ); + const watcher: Watchpack = createWatcher({ + addonId, addonReload, client, sourceDir, artifactsDir, ignoreFiles, + }); firefoxProcess.on('close', () => { client.disconnect(); watcher.close(); + stdin.pause(); }); + if (stdin.isTTY && stdin instanceof tty.ReadStream) { + readline.emitKeypressEvents(stdin); + stdin.setRawMode(true); + + // NOTE: this `Promise.resolve().then(...)` is basically used to spawn a "co-routine" that is executed + // before the callback attached to the Promise returned by this function (and it allows the `run` function + // to not be stuck in the while loop). + Promise.resolve().then(async function() { + log.info('Press R to reload (and Ctrl-C to quit)'); + + let userExit = false; + + while (!userExit) { + const keyPressed = await new Promise((resolve) => { + stdin.once('keypress', (str, key) => resolve(key)); + }); + + if (keyPressed.ctrl && keyPressed.name === 'c') { + userExit = true; + } else if (keyPressed.name === 'r' && addonId) { + log.debug('Reloading extension on user request'); + await addonReload({addonId, client}); + } + } + + log.info('\nExiting web-ext on user request'); + firefoxProcess.kill(); + }); + } } @@ -186,6 +235,7 @@ export type CmdRunOptions = {| firefoxApp: typeof defaultFirefoxApp, firefoxClient: typeof defaultFirefoxClient, reloadStrategy: typeof defaultReloadStrategy, + ExtensionRunnerClass?: typeof ExtensionRunner, |}; export default async function run( @@ -198,6 +248,7 @@ export default async function run( firefoxApp = defaultFirefoxApp, firefoxClient = defaultFirefoxClient, reloadStrategy = defaultReloadStrategy, + ExtensionRunnerClass = ExtensionRunner, }: CmdRunOptions = {}): Promise { log.info(`Running web extension from ${sourceDir}`); @@ -216,7 +267,7 @@ export default async function run( const manifestData = await getValidatedManifest(sourceDir); - const runner = new ExtensionRunner({ + const runner = new ExtensionRunnerClass({ sourceDir, firefoxApp, firefox, diff --git a/src/firefox/index.js b/src/firefox/index.js index 3f8a063a4b..d49700d9e2 100644 --- a/src/firefox/index.js +++ b/src/firefox/index.js @@ -96,6 +96,7 @@ export type FirefoxRunnerParams = {| export interface FirefoxProcess extends events$EventEmitter { stderr: events$EventEmitter; stdout: events$EventEmitter; + kill: Function; } export type FirefoxRunnerResults = {| diff --git a/tests/unit/helpers.js b/tests/unit/helpers.js index ae49bded80..fa2962bc88 100644 --- a/tests/unit/helpers.js +++ b/tests/unit/helpers.js @@ -149,7 +149,9 @@ export function makeSureItFails(): Function { * */ // $FLOW_IGNORE: fake can return any kind of object and fake a defined set of methods for testing. -export function fake(original: Object, methods: Object = {}): T { +export function fake( + original: Object, methods: Object = {}, skipProperties: Array = [] +): T { var stub = {}; // Provide stubs for all original members: @@ -162,7 +164,8 @@ export function fake(original: Object, methods: Object = {}): T { var proto = Object.getPrototypeOf(original); for (const key of props) { - if (!original.hasOwnProperty(key) && !proto.hasOwnProperty(key)) { + if (skipProperties.indexOf(key) >= 0 || + (!original.hasOwnProperty(key) && !proto.hasOwnProperty(key))) { continue; } const definition = original[key] || proto[key]; @@ -190,6 +193,10 @@ export function fake(original: Object, methods: Object = {}): T { return stub; } +export function createFakeProcess() { + return fake(process, {}, ['EventEmitter', 'stdin']); +} + /* * Returns a fake Firefox client as would be returned by diff --git a/tests/unit/test-cmd/test.run.js b/tests/unit/test-cmd/test.run.js index 6217f21be7..a1670a464e 100644 --- a/tests/unit/test-cmd/test.run.js +++ b/tests/unit/test-cmd/test.run.js @@ -1,6 +1,8 @@ /* @flow */ import path from 'path'; import EventEmitter from 'events'; +import tty from 'tty'; +import stream from 'stream'; import {describe, it} from 'mocha'; import {assert} from 'chai'; @@ -10,6 +12,7 @@ import {onlyInstancesOf, WebExtError, RemoteTempInstallNotSupported} from '../../../src/errors'; import run, { defaultFirefoxClient, defaultWatcherCreator, defaultReloadStrategy, + defaultAddonReload, } from '../../../src/cmd/run'; import * as defaultFirefoxApp from '../../../src/firefox'; import {RemoteFirefox} from '../../../src/firefox/remote'; @@ -32,6 +35,7 @@ describe('run', () => { function prepareRun(fakeInstallResult) { const sourceDir = fixturePath('minimal-web-ext'); + const argv = { artifactsDir: path.join(sourceDir, 'web-ext-artifacts'), sourceDir, @@ -293,8 +297,8 @@ describe('run', () => { sourceDir: '/path/to/extension/source/', artifactsDir: '/path/to/web-ext-artifacts', onSourceChange: sinon.spy(() => {}), - desktopNotifications: sinon.spy(() => Promise.resolve()), ignoreFiles: ['path/to/file', 'path/to/file2'], + addonReload: sinon.spy(() => Promise.resolve()), }; return { config, @@ -347,42 +351,11 @@ describe('run', () => { // Simulate executing the handler when a source file changes. return callArgs.onChange() .then(() => { - assert.equal(config.client.reloadAddon.called, true); - const reloadArgs = config.client.reloadAddon.firstCall.args; + assert.equal(config.addonReload.called, true); + const reloadArgs = config.addonReload.firstCall.args; assert.ok(config.addonId); - assert.equal(reloadArgs[0], config.addonId); - }); - }); - - it('notifies user on error from source change handler', () => { - const {config, createWatcher} = prepare(); - config.client.reloadAddon = () => Promise.reject(new Error('an error')); - createWatcher(); - - assert.equal(config.onSourceChange.called, true); - // Simulate executing the handler when a source file changes. - return config.onSourceChange.firstCall.args[0].onChange() - .then(makeSureItFails()) - .catch((error) => { - assert.equal(config.desktopNotifications.called, true); - assert.equal( - config.desktopNotifications.lastCall.args[0].message, - error.message - ); - }); - }); - - it('throws errors from source change handler', () => { - const {createWatcher, config} = prepare(); - config.client.reloadAddon = () => Promise.reject(new Error('an error')); - createWatcher(); - - assert.equal(config.onSourceChange.called, true); - // Simulate executing the handler when a source file changes. - return config.onSourceChange.firstCall.args[0].onChange() - .then(makeSureItFails()) - .catch((error) => { - assert.equal(error.message, 'an error'); + assert.equal(reloadArgs[0].addonId, config.addonId); + assert.equal(reloadArgs[0].client, config.client); }); }); @@ -393,6 +366,7 @@ describe('run', () => { class StubChildProcess extends EventEmitter { stderr = new EventEmitter(); stdout = new EventEmitter(); + kill = sinon.spy(() => {}); } function prepare() { @@ -410,14 +384,16 @@ describe('run', () => { ignoreFiles: ['first/file', 'second/file'], }; const options = { + addonReload: sinon.spy(() => Promise.resolve()), createWatcher: sinon.spy(() => watcher), + stdin: new stream.Readable(), }; return { ...args, ...options, client, watcher, - reloadStrategy: (argOverride = {}, optOverride = {}) => { + reloadStrategy: async (argOverride = {}, optOverride = {}) => { return defaultReloadStrategy( {...args, ...argOverride}, {...options, ...optOverride}); @@ -426,11 +402,17 @@ describe('run', () => { } it('cleans up connections when firefox closes', () => { - const {firefoxProcess, client, watcher, reloadStrategy} = prepare(); + const { + firefoxProcess, client, watcher, reloadStrategy, stdin, + } = prepare(); + + sinon.spy(stdin, 'pause'); + reloadStrategy(); firefoxProcess.emit('close'); assert.equal(client.client.disconnect.called, true); assert.equal(watcher.close.called, true); + assert.ok(stdin.pause.called); }); it('configures a watcher', () => { @@ -449,6 +431,93 @@ describe('run', () => { assert.deepEqual(receivedArgs.ignoreFiles, sentArgs.ignoreFiles); }); + it('can reload when user presses R in shell console', () => { + const {addonReload, reloadStrategy} = prepare(); + + const fakeStdin = new tty.ReadStream(); + sinon.spy(fakeStdin, 'setRawMode'); + + return reloadStrategy({}, {stdin: fakeStdin}) + .then(() => { + fakeStdin.emit('keypress', 'r', {name: 'r', ctrl: false}); + }) + .then(() => { + assert.ok(fakeStdin.setRawMode.called); + assert.ok(addonReload.called); + assert.equal(addonReload.firstCall.args[0].addonId, + tempInstallResult.addon.id); + }); + }); + + it('shuts down firefox on user request (CTRL+C in shell console)', () => { + const {firefoxProcess, reloadStrategy} = prepare(); + const fakeStdin = new tty.ReadStream(); + + return reloadStrategy({}, {stdin: fakeStdin}) + .then(() => { + fakeStdin.emit('keypress', 'c', {name: 'c', ctrl: true}); + }).then(() => { + assert.ok(firefoxProcess.kill.called); + }); + }); + + }); + + describe('defaultAddonReload', () => { + function createFakeRemoteFirefox(firefoxClient, {reloadAddon}) { + class FakeRemoteFirefox extends RemoteFirefox { + reloadAddon = sinon.spy(reloadAddon) + } + const client = new FakeRemoteFirefox(firefoxClient); + return client; + } + + const desktopNotifications = sinon.spy(() => Promise.resolve()); + const args = { + addonId: 'some-addon@test-suite', + desktopNotifications, + }; + + it('reloads addon', () => { + const client = createFakeRemoteFirefox(fakeFirefoxClient(), { + reloadAddon: () => Promise.resolve(), + }); + return defaultAddonReload({client, ...args}) + .then(() => { + assert.ok(client.reloadAddon.called, true); + const reloadArgs = client.reloadAddon.firstCall.args; + assert.equal(reloadArgs[0], args.addonId); + }); + }); + + it('notifies user on error from source change handler', () => { + const client = createFakeRemoteFirefox(fakeFirefoxClient(), { + reloadAddon: () => Promise.reject(new Error('an error')), + }); + return defaultAddonReload({client, ...args}) + .then(makeSureItFails()) + .catch((error) => { + assert.equal( + desktopNotifications.called, true + ); + assert.equal( + desktopNotifications.firstCall.args[0].message, + error.message + ); + }); + }); + + it('throws errors from source change handler', () => { + const client = createFakeRemoteFirefox(fakeFirefoxClient(), { + reloadAddon: () => Promise.reject(new Error('an error')), + }); + return defaultAddonReload({client, ...args}) + .then(makeSureItFails()) + .catch((error) => { + assert.equal(error.message, 'an error'); + }); + }); + }); describe('firefoxClient', () => { diff --git a/tests/unit/test.program.js b/tests/unit/test.program.js index 6d432be607..bfb8d16570 100644 --- a/tests/unit/test.program.js +++ b/tests/unit/test.program.js @@ -10,14 +10,19 @@ import {assert} from 'chai'; import {defaultVersionGetter, main, Program} from '../../src/program'; import commands from '../../src/cmd'; import {onlyInstancesOf, UsageError} from '../../src/errors'; -import {fake, makeSureItFails, ErrorWithCode} from './helpers'; +import { + createFakeProcess, + fake, + makeSureItFails, + ErrorWithCode, +} from './helpers'; import {ConsoleStream} from '../../src/util/logger'; describe('program.Program', () => { function execProgram(program, options = {}) { - const fakeProcess = fake(process); + const fakeProcess = createFakeProcess(); const absolutePackageDir = path.join(__dirname, '..', '..'); return program.execute( absolutePackageDir, { @@ -58,7 +63,7 @@ describe('program.Program', () => { }); it('exits 1 on a thrown error', () => { - const fakeProcess = fake(process); + const fakeProcess = createFakeProcess(); const program = new Program(['cmd']) .command('cmd', 'some command', () => { throw new Error('this is an error from a command handler'); @@ -280,7 +285,7 @@ describe('program.main', () => { getVersion: () => 'not-a-real-version', checkForUpdates: spy(), shouldExitProgram: false, - systemProcess: fake(process), + systemProcess: createFakeProcess(), }; return main(projectRoot, {argv, runOptions, ...mainOptions}); }