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.

Sign up for GitHub

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

Don't fail on stderr output, but print the output to stderr #110

Merged
merged 4 commits into from
Sep 25, 2016
Merged

Don't fail on stderr output, but print the output to stderr #110

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8f0623b
test: add test for not exiting on stderr content and match stderr output
Tapppi Sep 22, 2016
82cabab
feat: don't fail fast on stderr content, and print stderr, closes #91
Tapppi Sep 22, 2016
458da35
fix: print warnings in yellow instead of red
Sep 23, 2016
6338f32
style: simplify handleExecError if logic
Tapppi Sep 24, 2016
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
31 changes: 14 additions & 17 deletions index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,17 @@ function outputChangelog (argv, cb) {
})
}

function handleExecError (err, stderr) {
Copy link
Member

Choose a reason for hiding this comment

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

I thought we should handle err and stderr differently?
the problem is with stderr shouldn't terminate the program. But for err we keep the same behaviour. No?

Copy link
Member

Choose a reason for hiding this comment

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

I think I misread your code. It LGTM now.

// If exec returns content in stderr, but no error, print it as a warning
// If exec returns an error, print it and exit with return code 1
if (err) {
console.error(chalk.red(stderr || err.message))
process.exit(1)
} else if (stderr) {
console.warn(chalk.yellow(stderr))
}
}

function commit (argv, newVersion, cb) {
var msg = 'committing %s'
var args = [argv.infile]
Expand All @@ -118,14 +129,6 @@ function commit (argv, newVersion, cb) {
}
checkpoint(msg, args)

function handleExecError (err, stderr) {
// If exec returns an error or content in stderr, log it and exit with return code 1
var errMessage = stderr || (err && err.message)
if (errMessage) {
console.log(chalk.red(errMessage))
process.exit(1)
}
}
exec('git add package.json ' + argv.infile, function (err, stdout, stderr) {
handleExecError(err, stderr)
exec('git commit ' + verify + (argv.sign ? '-S ' : '') + 'package.json ' + argv.infile + ' -m "' + formatCommitMessage(argv.message, newVersion) + '"', function (err, stdout, stderr) {
Expand All @@ -148,17 +151,11 @@ function tag (newVersion, argv) {
}
checkpoint('tagging release %s', [newVersion])
exec('git tag ' + tagOption + 'v' + newVersion + ' -m "' + formatCommitMessage(argv.message, newVersion) + '"', function (err, stdout, stderr) {
handleExecError(err, stderr)
var message = 'git push --follow-tags origin master'
var errMessage = null
if (err) errMessage = err.message
if (stderr) errMessage = stderr
if (pkg.private !== true) message += '; npm publish'
if (errMessage) {
console.log(chalk.red(errMessage))
process.exit(1)
} else {
checkpoint('Run `%s` to publish', [message], chalk.blue(figures.info))
}

checkpoint('Run `%s` to publish', [message], chalk.blue(figures.info))
})
}

Expand Down
20 changes: 17 additions & 3 deletions test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ describe('cli', function () {

var result = execCli()
result.code.should.equal(1)
result.stdout.should.match(/commit yourself/)
result.stderr.should.match(/commit yourself/)

unmock()
})
Expand All @@ -131,7 +131,7 @@ describe('cli', function () {

var result = execCli()
result.code.should.equal(1)
result.stdout.should.match(/addition is hard/)
result.stderr.should.match(/addition is hard/)

unmock()
})
Expand All @@ -145,7 +145,21 @@ describe('cli', function () {

var result = execCli()
result.code.should.equal(1)
result.stdout.should.match(/tag, you're it/)
result.stderr.should.match(/tag, you're it/)

unmock()
})
})

it('doesn\'t fail fast on stderr output from git', function () {
// mock git by throwing on attempt to commit
return mockGit('console.error("haha, kidding, this is just a warning"); process.exit(0);', 'add')
.then(function (unmock) {
writePackageJson('1.0.0')

var result = execCli()
result.code.should.equal(0)
result.stderr.should.match(/haha, kidding, this is just a warning/)

unmock()
})
Expand Down