Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,13 @@
"minimatch": "3.0.3",
"mz": "2.6.0",
"node-firefox-connect": "1.2.0",
"regenerator-runtime": "0.10.1",
"parse-json": "2.2.0",
"regenerator-runtime": "0.10.1",
"sign-addon": "0.2.0",
"source-map-support": "0.4.7",
"stream-to-promise": "2.2.0",
"tmp": "0.0.30",
"update-notifier": "1.0.3",
"watchpack": "1.1.0",
"yargs": "6.5.0",
"zip-dir": "1.0.2"
Expand Down
28 changes: 22 additions & 6 deletions src/program.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,14 @@ import defaultCommands from './cmd';
import {UsageError} from './errors';
import {createLogger, consoleStream as defaultLogStream} from './util/logger';
import {coerceCLICustomPreference} from './firefox/preferences';
import {checkForUpdates as defaultUpdateChecker} from './util/updates';

const log = createLogger(__filename);
const envPrefix = 'WEB_EXT';

type ProgramOptions = {|
absolutePackageDir?: string,
|}

/*
* The command line program.
Expand All @@ -24,7 +28,9 @@ export class Program {

constructor(
argv: ?Array<string>,
{absolutePackageDir = process.cwd()}: {absolutePackageDir?: string} = {}
{
absolutePackageDir = process.cwd(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

similarly, this module / class should be set up for dependency injection so that you can replace the update utility with a stub for testing:

import {checkForAutomaticUpdates as defaultUpdateChecker} from './util/updates';

class Program {
  constructor(
    argv: ?Array<string>,
    {
      absolutePackageDir?: string,
      checkForAutomaticUpdates = defaultUpdateChecker,
    }
  ) {
    // ...
    this.checkForUpdates = checkForAutomaticUpdates;
  }
}

The test can then use a stub:

const checkForAutomaticUpdates = sinon.stub();
const program = new Program(['run'], {checkForAutomaticUpdates});
await program.execute();

assert.equal(checkForAutomaticUpdates.firstCall.args[0].version, '1.0.0');

}: ProgramOptions = {}
) {
// This allows us to override the process argv which is useful for
// testing.
Expand Down Expand Up @@ -84,8 +90,9 @@ export class Program {
async execute(
absolutePackageDir: string,
{
systemProcess = process, logStream = defaultLogStream,
getVersion = defaultVersionGetter, shouldExitProgram = true,
checkForUpdates = defaultUpdateChecker, systemProcess = process,
logStream = defaultLogStream, getVersion = defaultVersionGetter,
shouldExitProgram = true, globalEnv = WEBEXT_BUILD_ENV,
}: Object = {}
): Promise<void> {

Expand All @@ -112,7 +119,14 @@ export class Program {
if (!runCommand) {
throw new UsageError(`Unknown command: ${cmd}`);
}
if (globalEnv === 'production') {
checkForUpdates ({
version: getVersion(absolutePackageDir),
});
}

await runCommand(argv);

} catch (error) {
const prefix = cmd ? `${cmd}: ` : '';
if (!(error instanceof UsageError) || argv.verbose) {
Expand All @@ -138,14 +152,14 @@ declare var WEBEXT_BUILD_ENV: string;

//A defintion of type of argument for defaultVersionGetter
type versionGetterOptions = {
localEnv?: string,
globalEnv?: string,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks :)

};

export function defaultVersionGetter(
absolutePackageDir: string,
{localEnv = WEBEXT_BUILD_ENV}: versionGetterOptions = {}
{globalEnv = WEBEXT_BUILD_ENV}: versionGetterOptions = {}
): string {
if (localEnv === 'production') {
if (globalEnv === 'production') {
log.debug('Getting the version from package.json');
const packageData: any = readFileSync(
path.join(absolutePackageDir, 'package.json'));
Expand All @@ -164,7 +178,9 @@ export function main(
runOptions = {},
}: Object = {}
): Promise<any> {

const program = new Program(argv, {absolutePackageDir});

// yargs uses magic camel case expansion to expose options on the
// final argv object. For example, the 'artifacts-dir' option is alternatively
// available as argv.artifactsDir.
Expand Down
21 changes: 21 additions & 0 deletions src/util/updates.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/* @flow */
import defaultUpdateNotifier from 'update-notifier';

type checkForUpdatesParams = {|
version: string,
updateNotifier?: typeof defaultUpdateNotifier,
|};

export function checkForUpdates(
{
version,
updateNotifier = defaultUpdateNotifier,
}: checkForUpdatesParams
) {
const pkg = {name: 'web-ext', version};

updateNotifier({
pkg,
updateCheckInterval: 1000 * 60 * 60 * 24 * 3, // 3 days,
}).notify();
}
29 changes: 29 additions & 0 deletions tests/unit/test-util/test.updates.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/* @flow */
import {it, describe} from 'mocha';
import {assert} from 'chai';
import sinon from 'sinon';

import {checkForUpdates} from '../../../src/util/updates';

describe('util/updates', () => {
describe('checkForUpdates()', () => {
it('calls the notifier with the correct parameters', () => {
const updateNotifierStub = sinon.spy(() => {
return {
notify: sinon.spy(),
};
});

checkForUpdates({
version: '1.0.0',
updateNotifier: updateNotifierStub,
});
assert.equal(updateNotifierStub.called, true);
assert.equal(updateNotifierStub.firstCall.args[0].pkg.name, 'web-ext');
assert.equal(updateNotifierStub.firstCall.args[0].pkg.version, '1.0.0');
assert.isNumber(updateNotifierStub.firstCall.args[0].updateCheckInterval);
assert.equal(updateNotifierStub.firstCall.args[0].updateCheckInterval,
1000 * 60 * 60 * 24 * 3);
});
});
});
50 changes: 46 additions & 4 deletions tests/unit/test.program.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ describe('program.Program', () => {
const absolutePackageDir = path.join(__dirname, '..', '..');
return program.execute(
absolutePackageDir, {
getVersion: () => spy(),
checkForUpdates: spy(),
systemProcess: fakeProcess,
shouldExitProgram: false,
...options,
Expand Down Expand Up @@ -176,7 +178,10 @@ describe('program.Program', () => {
});
program.command('thing', 'does a thing', () => {});

return execProgram(program, {getVersion: spy(), logStream})
return execProgram(program, {
getVersion: spy(),
logStream,
})
.then(() => {
assert.equal(logStream.makeVerbose.called, true);
});
Expand Down Expand Up @@ -241,13 +246,50 @@ describe('program.Program', () => {
});
});

it('checks for updates automatically', () => {
const handler = spy();
const getVersion = () => 'some-package-version';
const checkForUpdates = sinon.stub();
const program = new Program(['run'])
.command('run', 'some command', handler);
return execProgram(program, {
checkForUpdates,
getVersion,
globalEnv: 'production',
})
.then(() => {
assert.equal(checkForUpdates.firstCall.args[0].version,
'some-package-version');
});
});

it('does not check for updates during development', () => {
const handler = spy();
const getVersion = () => 'some-package-version';
const checkForUpdates = sinon.stub();
const program = new Program(['run'])
.command('run', 'some command', handler);
return execProgram(program, {
checkForUpdates,
getVersion,
globalEnv: 'development',
})
.then(() => {
assert.equal(checkForUpdates.called, false);
});
});
});


describe('program.main', () => {

function execProgram(argv, {projectRoot = '', ...mainOptions}: Object = {}) {
const runOptions = {shouldExitProgram: false, systemProcess: fake(process)};
const runOptions = {
getVersion: () => 'not-a-real-version',
checkForUpdates: spy(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since no assertions are made in these tests, you could just assign it as a sinon.stub() which does nothing. Doesn't really matter either way though.

shouldExitProgram: false,
systemProcess: fake(process),
};
return main(projectRoot, {argv, runOptions, ...mainOptions});
}

Expand Down Expand Up @@ -351,15 +393,15 @@ describe('program.defaultVersionGetter', () => {
const pkgFile = path.join(root, 'package.json');
return fs.readFile(pkgFile)
.then((pkgData) => {
const testBuildEnv = {localEnv: 'production'};
const testBuildEnv = {globalEnv: 'production'};
assert.equal(defaultVersionGetter(root, testBuildEnv),
JSON.parse(pkgData).version);
});
});

it('returns git commit information in development', () => {
const commit = `${git.branch()}-${git.long()}`;
const testBuildEnv = {localEnv: 'development'};
const testBuildEnv = {globalEnv: 'development'};
assert.equal(defaultVersionGetter(root, testBuildEnv),
commit);
});
Expand Down