Skip to content
This repository has been archived by the owner on Feb 12, 2022. It is now read-only.

Commit

Permalink
review feedback
Browse files Browse the repository at this point in the history
use Path.join
some formatting changes
  • Loading branch information
KarateCowboy committed May 5, 2017
1 parent b4c8b85 commit 0eedbe1
Show file tree
Hide file tree
Showing 7 changed files with 80 additions and 56 deletions.
48 changes: 24 additions & 24 deletions commands/login.js
Original file line number Diff line number Diff line change
@@ -1,49 +1,49 @@
'use strict';
'use strict'

const cli = require('heroku-cli-util');
const co = require('co');
const child = require('child_process');
const log = require('../lib/log');
const cli = require('heroku-cli-util')
const co = require('co')
const child = require('child_process')
const log = require('../lib/log')

module.exports = function(topic) {
module.exports = function (topic) {
return {
topic: topic,
command: 'login',
flags: [{ name: 'verbose', char: 'v', hasValue: false }],
flags: [{name: 'verbose', char: 'v', hasValue: false}],
description: 'Logs in to the Heroku Docker registry',
needsApp: false,
needsAuth: true,
run: cli.command(co.wrap(login))
};
};
}
}

function* login(context, heroku) {
let herokuHost = process.env.HEROKU_HOST || 'heroku.com';
let registry = `registry.${ herokuHost }`;
let password = context.auth.password;
function* login (context, heroku) {
let herokuHost = process.env.HEROKU_HOST || 'heroku.com'
let registry = `registry.${ herokuHost }`
let password = context.auth.password

try {
let user = yield dockerLogin(registry, password, context.flags.verbose);
let user = yield dockerLogin(registry, password, context.flags.verbose)
}
catch (err) {
cli.error(`Error: docker login exited with ${ err }`);
cli.hush(err.stack || err);
cli.error(`Error: docker login exited with ${ err }`)
cli.hush(err.stack || err)
}
}

function dockerLogin(registry, password, verbose) {
function dockerLogin (registry, password, verbose) {
return new Promise((resolve, reject) => {
let args = [
'login',
'--username=_',
`--password=${ password }`,
registry
];
log(verbose, args);
child.spawn('docker', args, { stdio: 'inherit' })
]
log(verbose, args)
child.spawn('docker', args, {stdio: 'inherit'})
.on('exit', (code, signal) => {
if (signal || code) reject(signal || code);
else resolve();
});
});
if (signal || code) reject(signal || code)
else resolve()
})
})
}
4 changes: 2 additions & 2 deletions commands/logout.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@ module.exports = function(topic) {
};
};

function* logout(context, heroku) {
async function logout(context, heroku) {
let herokuHost = process.env.HEROKU_HOST || 'heroku.com';
let registry = `registry.${ herokuHost }`;

try {
let user = yield dockerLogout(registry, context.flags.verbose);
let user = await dockerLogout(registry, context.flags.verbose);
}
catch (err) {
cli.error(`Error: docker logout exited with ${ err }`);
Expand Down
20 changes: 13 additions & 7 deletions commands/push.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
const cli = require('heroku-cli-util')
const co = require('co')
let Sanbashi = require('../lib/sanbashi')
const Sanbashi = require('../lib/sanbashi')
const Chalk = require('chalk')

module.exports = function (topic) {
return {
topic: topic,
command: 'push',
description: 'Builds, then pushes a Docker image to deploy your Heroku app',
description: 'Builds, then pushes Docker images to deploy your Heroku app',
needsApp: true,
needsAuth: true,
variableArgs: true,
Expand All @@ -28,8 +29,13 @@ module.exports = function (topic) {

function* push (context, heroku) {
const recurse = !!context.flags.recursive
if(context.args.length === 0 && !recurse){
cli.error('Error: Requires either --recursive or one or more process types')
if (context.args.length === 0 && !recurse) {
cli.error(`Error: Requires either --recursive or one or more process types
${Chalk.bold.underline.magenta('Usage:')}
${ Chalk.white(' heroku container:push web')} # Pushes Dockerfile in current directory
${ Chalk.white(' heroku container:push web worker')} # Pushes Dockerfile.web and Dockerfile.worker found in the current directory
${ Chalk.white(' heroku container:push web worker --recursive')} # Pushes Dockerfile.web and Dockerfile.worker found in the current directory or subdirectories
${ Chalk.white(' heroku container:push --recursive')} # Pushes Dockerfile.* found in current directory or subdirectories`)
process.exit(1)
}
let herokuHost = process.env.HEROKU_HOST || 'heroku.com'
Expand All @@ -39,12 +45,12 @@ function* push (context, heroku) {
let jobs = yield Sanbashi.chooseJobs(possibleJobs)
if (!jobs.length) {
cli.warn('No images to push')
process.exit()
process.exit(1)
}

try {
for (let job of jobs) {
cli.log(`\n=== Building ${ job.name } (${ job.dockerfile })`)
cli.log(Chalk.bold.white.bgMagenta(`\n=== Building ${job.name} (${job.dockerfile})`))
yield Sanbashi.buildImage(job.dockerfile, job.resource, context.flags.verbose)
}
}
Expand All @@ -56,7 +62,7 @@ function* push (context, heroku) {

try {
for (let job of jobs) {
cli.log(`\n=== Pushing ${ job.name } (${ job.dockerfile })`)
cli.log(Chalk.bold.white.bgMagenta(`\n=== Pushing ${job.name} (${job.dockerfile })`))
yield Sanbashi.pushImage(job.resource, context.flags.verbose)
}
}
Expand Down
5 changes: 2 additions & 3 deletions lib/sanbashi.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,8 @@ let Inquirer = require('inquirer')
const Child = require('child_process')
const log = require('./log')

const DOCKERFILE_REGEX = /\/Dockerfile(.\w*)?$/
const DOCKERFILE_REGEX = /\bDockerfile(.\w*)?$/
class Sanbashi {
constructor () {}

static getDockerfiles (rootdir, recursive) {
let match = recursive ? './**/Dockerfile?(.)*' : 'Dockerfile*'
Expand All @@ -17,7 +16,7 @@ class Sanbashi {
nonull: false,
nodir: true
})
if(recursive){
if (recursive) {
dockerfiles = dockerfiles.filter(df => df.match(/Dockerfile\.[\w]+/))
}
return dockerfiles.map(file => Path.join(rootdir, file))
Expand Down
13 changes: 7 additions & 6 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,28 +19,29 @@
"license": "ISC",
"dependencies": {
"camelcase": "1.0.2",
"chalk": "1.1.3",
"co": "4.6.0",
"dotenv": "^1.1.0",
"glob": "7.1.0",
"heroku-cli-util": "^6.1.17",
"heroku-client": "^1.9.1",
"inquirer": "^3.0.6",
"heroku-cli-util": "6.1.17",
"heroku-client": "1.9.1",
"inquirer": "3.0.6",
"is-there": "4.0.0",
"lodash": "3.3.1",
"mkdirp": "^0.5.0",
"node-uuid": "^1.4.3",
"node-uuid": "1.4.3",
"progress": "1.1.8",
"request": "2.53.0",
"superagent": "^0.21.0",
"superagent": "0.21.0",
"yamljs": "0.2.4"
},
"devDependencies": {
"chai": "^3.2.0",
"sinon": "^2.1.0",
"depcheck": "^0.4.7",
"dir-compare": "^0.0.2",
"fs-extra": "^0.23.1",
"mocha": "^2.2.4",
"sinon": "^2.1.0",
"wrench": "^1.5.8"
}
}
44 changes: 31 additions & 13 deletions test/sanbashi.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,51 +24,69 @@ describe('Sanbashi', () => {
})
describe('.getJobs', () => {
it('returns objects representing jobs per Dockerfile', () => {
const dockerfiles = ['./Dockerfile.web', './Nested/Dockerfile.web']
const dockerfiles = [
Path.join('.', 'Dockerfile.web'),
Path.join('.', 'Nested', 'Dockerfile.web')
]
const resourceRoot = 'rootfulroot'
const results = Sanbashi.getJobs(resourceRoot, ['web'], dockerfiles)
expect(results.web).to.have.property('length', 2)
expect(results.web[0]).to.have.property('depth', 1, 'dockerfile', './Dockerfile.web', 'postfix', 1)
expect(results.web[1]).to.have.property('depth', 2, 'dockerfile', './Nested/Dockerfile.web', 'postfix', 1)
})
it('filters out by process type', () => {
const dockerfiles = ['./Dockerfile.web', './Nested/Dockerfile.worker']
const dockerfiles = [
Path.join('.', 'Dockerfile.web'),
Path.join('.', 'Nested', 'Dockerfile.worker')
]
const resourceRoot = 'rootfulroot'
const results = Sanbashi.getJobs(resourceRoot, ['web'], dockerfiles)
expect(results.web).to.have.property('length', 1)
expect(results.web[0]).to.have.property('name', 'web')
expect(results).to.not.have.property('worker')
})
it('sorts dockerfiles by directory depth, then proc type', () => {
const dockerfiles = ['./Nested/Dockerfile.worker', './Dockerfile.web', './Nested/Dockerfile']
const dockerfiles = [
Path.join('.', 'Nested', 'Dockerfile.worker'),
Path.join('.', 'Dockerfile.web'),
Path.join('.', 'Nested', 'Dockerfile')
]
const resourceRoot = 'rootfulroot'
const results = Sanbashi.getJobs(resourceRoot, [], dockerfiles)
expect(results.web).to.have.property('length', 2)
expect(results.web[0]).to.have.property('dockerfile', './Dockerfile.web')
expect(results.web[1]).to.have.property('dockerfile', './Nested/Dockerfile')
expect(results.worker[0]).to.have.property('dockerfile', './Nested/Dockerfile.worker')
expect(results.web[0]).to.have.property('dockerfile', 'Dockerfile.web')
expect(results.web[1]).to.have.property('dockerfile', 'Nested/Dockerfile')
expect(results.worker[0]).to.have.property('dockerfile', 'Nested/Dockerfile.worker')
})
it('groups the jobs by process type', () => {
const dockerfiles = ['./Nested/Dockerfile.worker', './Dockerfile.web', './Nested/Dockerfile']
const dockerfiles = [
Path.join('.', 'Nested', 'Dockerfile.worker'),
Path.join('.', 'Dockerfile.web'),
Path.join('.', 'Nested', 'Dockerfile')
]
const resourceRoot = 'rootfulroot'
const results = Sanbashi.getJobs(resourceRoot, [], dockerfiles)
expect(results).to.have.keys('worker', 'web')
expect(results['worker'].map(j => j.dockerfile)).to.have.members(['./Nested/Dockerfile.worker'])
expect(results['web'].map(j => j.dockerfile)).to.have.members(['./Dockerfile.web', './Nested/Dockerfile'])
expect(results['worker'].map(j => j.dockerfile)).to.have.members([Path.join('.', 'Nested', 'Dockerfile.worker')])
expect(results['web'].map(j => j.dockerfile)).to.have.members([Path.join('.', 'Dockerfile.web'), Path.join('.', 'Nested', 'Dockerfile')])
})
})
describe('.chooseJobs', () => {
it('returns the entry when ony one exist', () => {
const dockerfiles = ['./Nested/Dockerfile.web']
const dockerfiles = [Path.join('.', 'Nested', 'Dockerfile.web')]
const jobs = Sanbashi.getJobs('rootfulroot', [], dockerfiles)
let chosenJob = Sanbashi.chooseJobs(jobs)
expect(chosenJob[0]).to.have.property('dockerfile', dockerfiles[0])
expect(chosenJob).to.have.property('length',1)
expect(chosenJob).to.have.property('length', 1)
})
it.skip('queries the user when more than one job of a type exists', () => {
//really no way to simulate user input for Inquirer
Sinon.stub(Inquirer, 'prompt').resolves('something')
const dockerfiles = ['./Nested/Dockerfile.web', './OtherNested/Dockerfile.web','./AnotherNested/Dockerfile.web', './Dockerfile.web']
const dockerfiles = [
Path.join('.', 'Nested', 'Dockerfile.web'),
Path.join('.', 'OtherNested', 'Dockerfile.web'),
Path.join('.', 'AnotherNested', 'Dockerfile.web', './Dockerfile.web')
]
let jobs = Sanbashi.getJobs('rootfulroot', ['web'], dockerfiles)
let res = Sanbashi.chooseJobs(jobs)
// .then((chosen) => {
Expand All @@ -80,7 +98,7 @@ describe('Sanbashi', () => {
console.dir(res)
})
afterEach(() => {
if(Inquirer.prompt.restore)
if (Inquirer.prompt.restore)
Inquirer.prompt.restore()
})
})
Expand Down
2 changes: 1 addition & 1 deletion yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ cli-width@^2.0.0:
version "2.1.0"
resolved "https://registry.yarnpkg.com/cli-width/-/cli-width-2.1.0.tgz#b234ca209b29ef66fc518d9b98d5847b00edf00a"

[email protected]:
[email protected], co@^4.6.0:
version "4.6.0"
resolved "https://registry.yarnpkg.com/co/-/co-4.6.0.tgz#6ea6bdf3d853ae54ccb8e47bfa0bf3f9031fb184"

Expand Down

0 comments on commit 0eedbe1

Please sign in to comment.