Skip to content
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

T/43: Added tests #44

Merged
merged 13 commits into from
Feb 13, 2017
32 changes: 17 additions & 15 deletions lib/commands/bootstrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,9 @@ const exec = require( '../utils/exec' );
module.exports = {
/**
* @param {Object} data
* @param {Object} data.parameters Additional arguments provided by the user.
* @param {String} data.packageName Name of current package to process.
* @param {Options} data.options The options object.
* @param {Repository|null} data.repository
* @param {Repository} data.repository
* @returns {Promise}
*/
execute( data ) {
Expand All @@ -25,19 +24,22 @@ module.exports = {
return new Promise( ( resolve, reject ) => {
const destinationPath = path.join( data.options.packages, data.repository.directory );

// Package is already cloned.
if ( fs.existsSync( destinationPath ) ) {
log.info( `Package "${ data.packageName }" is already cloned. Skipping.` );
let promise = Promise.resolve();

return resolve( { logs: log.all() } );
// Package is not cloned.
Copy link
Member

Choose a reason for hiding this comment

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

Wrong comment.

if ( fs.existsSync( destinationPath ) ) {
log.info( `Package "${ data.packageName }" is already cloned.` );
} else {
const command = [
Copy link
Member

Choose a reason for hiding this comment

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

Since this condition sets the promise to exec( cmd ), the previous condition could set it to Promise.resolve(). This would be clearer.

`git clone --progress ${ data.repository.url } ${ destinationPath }`,
`cd ${ destinationPath }`,
`git checkout --quiet ${ data.repository.branch }`
].join( ' && ' );

promise = exec( command );
}

const command =
`git clone --progress ${ data.repository.url } ${ destinationPath } && ` +
`cd ${ destinationPath } && ` +
`git checkout --quiet ${ data.repository.branch }`;

exec( command )
promise
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing return statement (?)

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

I see now where resolve and reject parts are. But it looks a little bit strange for me to return a promise which has another promise inside and which is even not returned ;) Maybe it could be written step by step with .then() method and get 27-42 lines before the return statement?

Copy link
Member Author

Choose a reason for hiding this comment

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

A promise inside another promise can not be returned because you need to call resolve or reject function.

OTOH maybe the "wrapper" Promise is not necessary. Who knows? :D

.then( ( output ) => {
log.info( output );

Expand All @@ -47,14 +49,14 @@ module.exports = {

if ( data.options.recursive ) {
const packageJson = require( path.join( destinationPath, 'package.json' ) );
let packages = [];
const packages = [];

if ( packageJson.dependencies ) {
packages = packages.concat( Object.keys( packageJson.dependencies ) );
packages.push( ...Object.keys( packageJson.dependencies ) );
}

if ( packageJson.devDependencies ) {
packages = packages.concat( Object.keys( packageJson.devDependencies ) );
packages.push( ...Object.keys( packageJson.devDependencies ) );
}

commandOutput.packages = packages;
Expand Down
12 changes: 6 additions & 6 deletions lib/commands/exec.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,20 @@ const exec = require( '../utils/exec' );

module.exports = {
/**
* @param {Array.<String>} parameters Arguments that user provided calling the mgit.
* @param {Array.<String>} args Arguments that user provided calling the mgit.
*/
beforeExecute( parameters ) {
if ( parameters.length === 1 ) {
beforeExecute( args ) {
if ( args.length === 1 ) {
throw new Error( 'Missing command to execute. Use: mgit exec [command-to-execute].' );
}
},

/**
* @param {Object} data
* @param {Object} data.parameters Additional arguments provided by the user.
* @param {Object} data.arguments Arguments that user provided calling the mgit.
* @param {String} data.packageName Name of current package to process.
* @param {Options} data.options The options object.
* @param {Repository|null} data.repository
* @param {Repository} data.repository
* @returns {Promise}
*/
execute( data ) {
Expand All @@ -42,7 +42,7 @@ module.exports = {

process.chdir( newCwd );

exec( data.parameters[ 0 ] )
exec( data.arguments[ 0 ] )
.then( ( stdout ) => {
process.chdir( data.options.cwd );

Expand Down
6 changes: 3 additions & 3 deletions lib/commands/savehashes.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ module.exports = {
commit: commitHash
};

log.info( `Commit: ${ commitHash }.` );
log.info( `Commit: "${ commitHash }".` );

resolve( {
response: commandResponse,
Expand All @@ -44,7 +44,7 @@ module.exports = {

function getExecData( command ) {
return Object.assign( {}, data, {
parameters: [ command ]
arguments: [ command ]
} );
}
},
Expand All @@ -56,7 +56,7 @@ module.exports = {
* @param {Set} commandResponses Results of executed command for each package.
*/
afterExecute( processedPackages, commandResponses ) {
const cwd = require( '../utils/getcwd.js' )();
const cwd = require( '../utils/getcwd' )();
const mgitJsonPath = path.join( cwd, 'mgit.json' );

updateJsonFile( mgitJsonPath, ( json ) => {
Expand Down
17 changes: 11 additions & 6 deletions lib/commands/update.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,9 @@ const chalk = require( 'chalk' );
module.exports = {
/**
* @param {Object} data
* @param {Object} data.parameters Additional arguments provided by the user.
* @param {String} data.packageName Name of current package to process.
* @param {Options} data.options The options object.
* @param {Repository|null} data.repository
* @param {Repository} data.repository
* @returns {Promise}
*/
execute( data ) {
Expand All @@ -33,7 +32,6 @@ module.exports = {
const bootstrapOptions = {
options: data.options,
packageName: data.packageName,
mgit: data.mgit,
repository: data.repository
};

Expand Down Expand Up @@ -68,11 +66,11 @@ module.exports = {
log.concat( response.logs );
} )
.then( () => {
return execCommand.execute( getExecData( 'git branch' ) );
return execCommand.execute( getExecData( 'git branch -a' ) );
} )
.then( ( response ) => {
const stdout = response.logs.info.join( '\n' ).trim();
const isOnBranchRegexp = /HEAD detached at [\w\d]+/;
const isOnBranchRegexp = /HEAD detached at+/;

// If on a detached commit, mgit must not pull the changes.
if ( isOnBranchRegexp.test( stdout ) ) {
Expand All @@ -81,6 +79,13 @@ module.exports = {
return resolve( { logs: log.all() } );
}

const isRemoteBranchAvailableRegexp = new RegExp( `remotes\\\/origin\\\/${ data.repository.branch }` );

// Check whether the remote branch is available.
if ( !stdout.match( isRemoteBranchAvailableRegexp ) ) {
throw new Error( `Branch "${ data.repository.branch }" is not available on server.` );
}

return execCommand.execute( getExecData( `git pull origin ${ data.repository.branch }` ) );
} )
.then( ( response ) => {
Expand All @@ -97,7 +102,7 @@ module.exports = {

function getExecData( command ) {
return Object.assign( {}, data, {
parameters: [ command ]
arguments: [ command ]
} );
}
},
Expand Down
12 changes: 6 additions & 6 deletions lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ const logDisplay = require( './utils/displaylog' );
const getOptions = require( './utils/getoptions' );

/**
* @param {Array.<String>} parameters Arguments that the user provided.
* @param {Array.<String>} args Arguments that the user provided.
* @param {Options} options The options object. It will be extended with the default options.
*/
module.exports = function( parameters, options ) {
module.exports = function( args, options ) {
const startTime = process.hrtime();
const forkPool = createForkPool( path.join( __dirname, 'utils', 'child-process.js' ) );

Expand All @@ -23,13 +23,13 @@ module.exports = function( parameters, options ) {
const resolver = require( options.resolverPath );

// Remove all dashes from command name.
parameters[ 0 ] = parameters[ 0 ].replace( /-/g, '' );
args[ 0 ] = args[ 0 ].replace( /-/g, '' );

const commandPath = path.join( __dirname, 'commands', parameters[ 0 ] );
const commandPath = path.join( __dirname, 'commands', args[ 0 ] );
const command = require( commandPath );

if ( typeof command.beforeExecute == 'function' ) {
command.beforeExecute( parameters );
command.beforeExecute( args );
}

const processedPackages = new Set();
Expand All @@ -52,7 +52,7 @@ module.exports = function( parameters, options ) {

const data = {
command: commandPath,
parameters: parameters.slice( 1 ),
arguments: args.slice( 1 ),
options,
packageName: packageName,
repository: resolver( packageName, options )
Expand Down
4 changes: 4 additions & 0 deletions lib/utils/log.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ module.exports = function log() {
},

log( type, msg ) {
if ( !msg ) {
return;
}

msg = msg.trim();

if ( !msg ) {
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
"guppy-pre-commit": "^0.4.0",
"istanbul": "^0.4.5",
"mocha": "^3.2.0",
"mockery": "^2.0.0",
"sinon": "^1.17.7"
},
"repository": {
Expand Down
156 changes: 156 additions & 0 deletions tests/commands/bootstrap.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
/**
* @license Copyright (c) 2003-2017, CKSource - Frederico Knabben. All rights reserved.
* For licensing, see LICENSE.md.
*/

/* jshint mocha:true */

'use strict';

const fs = require( 'fs' );
const path = require( 'path' );
const sinon = require( 'sinon' );
const mockery = require( 'mockery' );
const expect = require( 'chai' ).expect;

describe( 'commands/bootstrap', () => {
let bootstrapCommand, sandbox, stubs, data;

beforeEach( () => {
sandbox = sinon.sandbox.create();

mockery.enable( {
useCleanCache: true,
warnOnReplace: false,
warnOnUnregistered: false
} );

stubs = {
exec: sandbox.stub(),
fs: {
existsSync: sandbox.stub( fs, 'existsSync' )
},
path: {
join: sandbox.stub( path, 'join', ( ...chunks ) => chunks.join( '/' ) )
}
};

data = {
packageName: 'test-package',
options: {
cwd: __dirname,
packages: 'packages'
},
repository: {
directory: 'test-package',
url: 'git@github.com/organization/test-package.git',
branch: 'master'
}
};

mockery.registerMock( '../utils/exec', stubs.exec );

bootstrapCommand = require( '../../lib/commands/bootstrap' );
} );

afterEach( () => {
sandbox.restore();
mockery.disable();
} );

describe( 'execute()', () => {
it( 'rejects promise if something went wrong', () => {
const error = new Error( 'Unexpected error.' );

stubs.fs.existsSync.returns( false );
stubs.exec.returns( Promise.reject( error ) );

return bootstrapCommand.execute( data )
.then(
() => {
throw new Error( 'Supposed to be rejected.' );
},
( response ) => {
expect( response.logs.error[ 0 ].split( '\n' )[ 0 ] ).to.equal( `Error: ${ error.message }` );
}
);
} );

it( 'clones a repository if is not available', () => {
stubs.fs.existsSync.returns( false );
stubs.exec.returns( Promise.resolve( 'Git clone log.' ) );

return bootstrapCommand.execute( data )
.then( ( response ) => {
expect( stubs.exec.calledOnce ).to.equal( true );

const cloneCommand = stubs.exec.firstCall.args[ 0 ].split( ' && ' );

// Clone the repository.
expect( cloneCommand[ 0 ] ).to.equal( 'git clone --progress git@github.com/organization/test-package.git packages/test-package' );
// Change the directory to cloned package.
expect( cloneCommand[ 1 ] ).to.equal( 'cd packages/test-package' );
// And check out to proper branch.
expect( cloneCommand[ 2 ] ).to.equal( 'git checkout --quiet master' );

expect( response.logs.info[ 0 ] ).to.equal( 'Git clone log.' );
} );
} );

it( 'does not clone a repository if is available', () => {
stubs.fs.existsSync.returns( true );

return bootstrapCommand.execute( data )
.then( ( response ) => {
expect( stubs.exec.called ).to.equal( false );

expect( response.logs.info[ 0 ] ).to.equal( 'Package "test-package" is already cloned.' );
} );
} );

it( 'installs dependencies of cloned package', () => {
Copy link
Member

Choose a reason for hiding this comment

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

Duplicated test name.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry. The other one is devDeps.

Copy link
Member Author

@pomek pomek Feb 13, 2017

Choose a reason for hiding this comment

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

Fixed in cc8e24f.

data.options.recursive = true;
data.options.packages = __dirname + '/../fixtures';
data.repository.directory = 'project-a';

stubs.fs.existsSync.returns( true );

return bootstrapCommand.execute( data )
.then( ( response ) => {
expect( response.packages ).is.an( 'array' );
expect( response.packages ).to.deep.equal( [ 'test-foo' ] );
} );
} );

it( 'installs devDependencies of cloned package', () => {
data.options.recursive = true;
data.options.packages = __dirname + '/../fixtures';
data.repository.directory = 'project-with-options-in-mgitjson';

stubs.fs.existsSync.returns( true );

return bootstrapCommand.execute( data )
.then( ( response ) => {
expect( response.packages ).is.an( 'array' );
expect( response.packages ).to.deep.equal( [ 'test-bar' ] );
} );
} );
} );

describe( 'afterExecute()', () => {
it( 'informs about number of processed packages', () => {
const consoleLog = sandbox.stub( console, 'log' );

const processedPackages = new Set();
processedPackages.add( 'package-1' );
processedPackages.add( 'package-2' );

bootstrapCommand.afterExecute( processedPackages );

expect( consoleLog.calledOnce ).to.equal( true );
expect( consoleLog.firstCall.args[ 0 ] ).to.match( /2 packages have been processed\./ );

consoleLog.restore();
} );
} );
} );
Loading