Merge pull request #7 from KurtWagner/adding-comment-tests

Resolve #1: Added option --fail-on-severity for comments action

README.md

@@ -12,6 +12,8 @@ Creates comments on a Bitbucket pull request for given checkstyle result XML fil
 
 It's optimized for consumption within an existing project but can be used independently. The only configuration file required is a credentials file. See authentication below for more information.
 
+Comments will only be left for lines located in the returned diff. This may not only include added and changed lines but lines around such ones. Keep this in mind when using the `--fail-on-severity` option. It may not only reflect lines touched by the author but lines around the touches.
+
 ### Linter comments on Pull Request 
 
 ```
@@ -28,6 +30,26 @@ Parses and leaves comments on a pull request for given lint results.
 * [Checkstyle Result XML](http://checkstyle.sourceforge.net/) with `--checkstyle`
  * _Needs examples of tools that have checkstyle formatters. e.g, eslint_ 
 
+#### Options
+
+##### --pull-request
+
+The id of the pull request. This should match up with the user and repository slugs.
+
+##### --checkstyle
+
+One or more paths to checkstyle result XML files. This cannot be used in conjunction with `--android-lint`.
+
+##### --android-lint
+
+One or more paths to Android Lint result XML files. This cannot be used in conjunction with `--checkstyle`.
+
+##### --fail-on-severity
+
+Exit on a non-zero code to indicate that a given severity was seen in a comment. For example, fail if a comment is made for an "error". This is useful if you wish to fail a build process only if a certain severity was introduced.
+
+Severities are case insensitive.
+
 ## Reviewers
 
 _In progress_
@@ -116,7 +138,7 @@ bitbucket-toolbox comments
 
 The message identifier defaults to `.:.`.
 
-# Options
+# Global Options
 
 ##### --repo-slug / config.repoSlug
 
@@ -126,18 +148,6 @@ The name of the bitbucket repository.
 
 The username who owns the bitbucket repository containing the pull request.
 
-##### --pull-request
-
-The id of the pull request. This should match up with the user and repository slugs.
-
-##### --checkstyle
-
-One or more paths to checkstyle result XML files. This cannot be used in conjunction with `--android-lint`.
-
-##### --android-lint
-
-One or more paths to Android Lint result XML files. This cannot be used in conjunction with `--checkstyle`.
-
 ##### --credentials
 
 Path to the JSON file containing a `bitbucket.username` and `bitbucket.password`.

bin/bitbucket-toolbox.js

@@ -1,19 +1,31 @@
 #!/usr/bin/env node
 'use strict';
 
+const logger = require('../lib/logger');
 const {getConfig} = require('../lib/config');
 const {getArgs} = require('../lib/args');
 const {getCredentialsFromArgs} = require('../lib/credentials');
 const {dispatchAction} = require('../lib/actions');
 
+const {name, version} = require('../package.json');
+logger.title(`${name} ${version}`);
+
 try {
 	const config = getConfig();
 	const {action, args} = getArgs(process.argv);
 	const credentials = getCredentialsFromArgs(args);
 
-	dispatchAction(action, {config, args, credentials});
-
+	dispatchAction(action, {config, args, credentials})
+		.then(() => {
+			logger.success(`Running "${action}"`);
+			logger.log('Done.');
+		})
+		.catch(handleError);
 } catch (e) {
-	console.error(e.message);
+	handleError(e);
+}
+
+function handleError({message}) {
+	logger.error(message);
 	process.exit(1);
 }

lib/actions/comments.js

@@ -2,8 +2,10 @@
 
 const Q = require('q');
 
+const logger = require('../logger');
 const {BitbucketClient} = require('../bitbucket');
 const {loadAndroidLintResults, loadCheckstyleResults} = require('../loaders');
+const TOTAL_STEPS = 3;
 
 module.exports = {
 	executeCommentsAction,
@@ -21,24 +23,21 @@ function executeCommentsAction({args, config, credentials}) {
 		pullRequestID,
 		checkstyleFilePaths = [],
 		androidLintFilePaths = [],
+		failOnSeverity = [],
 	} = args;
 
 	if (!repoSlug || !repoUser || !pullRequestID) {
-		console.error('Required repo slug, repo user and pull request id');
-		process.exit(1);
+		throw new Error('Required repo slug, repo user and pull request id');
 	}
 
 	if (checkstyleFilePaths.length === 0 && androidLintFilePaths.length === 0) {
-		console.error('Please supply --checkstyle or --android-lint result files.');
-		process.exit(1);
+		throw new Error('Please supply --checkstyle or --android-lint result files.');
 	}
 
 	if (checkstyleFilePaths.length > 0 && androidLintFilePaths.length > 0) {
-		console.error('Please supply either --checkstyle or --android-lint result files not both.');
-		process.exit(1);
+		throw new Error('Please supply either --checkstyle or --android-lint result files not both.');
 	}
 
-
 	const client = new BitbucketClient({
 		username: credentials.bitbucket.username,
 		password: credentials.bitbucket.password,
@@ -48,8 +47,8 @@ function executeCommentsAction({args, config, credentials}) {
 	const loader = checkstyleFilePaths.length > 0 ? loadCheckstyleResults : loadAndroidLintResults;
 	const loaderFilePaths = checkstyleFilePaths.length > 0 ? checkstyleFilePaths : androidLintFilePaths;
 
-	console.log('Getting comments, diff and current user');
-	loader(loaderFilePaths)
+	logger.step(1, TOTAL_STEPS, 'Getting comments, diff and current user');
+	return loader(loaderFilePaths)
 		.then(checkstyle => {
 			return Q.all([
 				Q(checkstyle),
@@ -61,32 +60,36 @@ function executeCommentsAction({args, config, credentials}) {
 		.then(([linterResult, changedChunks, existingComments, currentUser]) => {
 			const comments = getComments({ changedChunks, linterResult, messageIdentifier });
 			const currentUserComments = getPreviousCommentIds({ currentUser, existingComments, messageIdentifier });
-			return {pullRequest, comments, currentUserComments};
+			return { pullRequest, comments, currentUserComments, failOnSeverity};
 		})
-		.then(startProcessing)
-		.catch(error => {
-			console.error(error);
-			process.exit();
-		});
+		.then(startProcessing);
 }
 
-function startProcessing({pullRequest, comments, currentUserComments}) {
-	console.log(`Deleting ${currentUserComments.length} comments ${currentUserComments.join(', ')}`);
-	pullRequest.deleteComments(...currentUserComments).then(({ errors }) => {
+function startProcessing({ pullRequest, comments, currentUserComments, failOnSeverity}) {
+	logger.step(2, TOTAL_STEPS, `Deleting ${currentUserComments.length} comments ${currentUserComments.join(', ')}`);
+	return pullRequest.deleteComments(...currentUserComments).then(({ errors }) => {
 		if (errors.length > 0) {
-			console.error('Failed to remove comments');
-			console.error(errors);
+			logger.error('Failed to remove comments');
+			throw new Error(errors);
 		} else {
-			console.log(`Adding ${comments.length} comments`);
+			logger.step(3, TOTAL_STEPS, `Adding ${comments.length} comments`);
 			const addCommentPromises = [];
+			const hitSeverities = new Set();
 			comments.forEach(comment => {
 				const addCommentPromise = pullRequest.addComment(comment);
+
+				if (failOnSeverity.includes(comment.severity)) {
+					hitSeverities.add(comment.severity);
+				}
+
 				addCommentPromises.push(addCommentPromise);
 			});
-			Q
+			return Q
 				.all(addCommentPromises)
-				.finally(() => {
-					console.log('Done.');
+				.then(() => {
+					if (hitSeverities.size > 0) {
+						throw new Error(`Severities hit: ${[...hitSeverities].sort().join(', ')}`);
+					}
 				});
 		}
 	});
@@ -105,6 +108,7 @@ function getComments({ linterResult, changedChunks, messageIdentifier }) {
 					changed: line.changed,
 					message: `${error.message} ${messageIdentifier}`,
 					column: error.column,
+					severity: error.severity,
 				});
 			}
 		});

lib/actions/comments.spec.js

@@ -1,11 +1,142 @@
 'use strict';
 
 const chai = require('chai');
-const {_test} = require('./comments');
+const nock = require('nock');
+const mock = require('mock-fs');
 
+const {executeCommentsAction, _test} = require('./comments');
 const {getPreviousCommentIds} = _test;
 
+const HTTP_OK = 200;
+
 describe('Action Comments', () => {
+
+	describe('executeCommentsAction', () => {
+		afterEach(mock.restore);
+
+		beforeEach(() => {
+			nock('https://api.bitbucket.org/2.0')
+				.get('/repositories/my-user/my-repo/pullrequests/10/diff')
+				.reply(HTTP_OK, getDiff());
+			nock('https://api.bitbucket.org/2.0')
+				.get('/repositories/my-user/my-repo/pullrequests/10/comments')
+				.reply(HTTP_OK, {});
+			nock('https://api.bitbucket.org/1.0')
+				.persist()
+				.post('/repositories/my-user/my-repo/pullrequests/10/comments')
+				.reply(HTTP_OK, {});
+			nock('https://api.bitbucket.org/2.0')
+				.get('/user')
+				.reply(HTTP_OK, {
+					username: 'my-current-username',
+				});
+		});
+
+		const config = {
+			bitbucket: {
+				repoUser: 'my-user',
+				repoSlug: 'my-repo',
+			},
+			messageIdentifier: '.:id:.',
+		};
+		const credentials = {
+			bitbucket: {
+				username: 'user1',
+				password: 'pass1',
+			},
+		};
+		const args = {
+			pullRequestID: 10,
+			checkstyleFilePaths: ['checkstyle.xml'],
+		};
+
+		it('returns success if no severities are configured', (done) => {
+			mock({
+				'checkstyle.xml': `
+					<?xml version="1.0" encoding="utf-8"?>
+					<checkstyle version="4.3">
+					<file name="eslint.js">
+						<error line="13" column="1" severity="critical" message=""/>
+						<error line="14" column="1" severity="fatal" message=""/>
+					</file>
+					<file name="package.json">
+						<error line="10" column="1" severity="info" message=""/>
+						<error line="11" column="1" severity="warning" message=""/>
+						<error line="12" column="1" severity="error" message=""/>
+					</file>
+					</checkstyle>
+				`
+			});
+			executeCommentsAction({
+				args: Object.assign({}, args, {
+					failOnSeverity: [],
+				}),
+				config,
+				credentials
+			})
+			.then(() => done())
+			.catch(({message}) => done(message));
+		});
+
+		it('returns error if single severity is hit', (done) => {
+			mock({
+				'checkstyle.xml': `
+					<?xml version="1.0" encoding="utf-8"?>
+					<checkstyle version="4.3">
+					<file name="eslint.js">
+						<error line="13" column="1" severity="critical" message=""/>
+						<error line="14" column="1" severity="fatal" message=""/>
+					</file>
+					<file name="package.json">
+						<error line="10" column="1" severity="info" message=""/>
+						<error line="11" column="1" severity="warning" message=""/>
+						<error line="12" column="1" severity="error" message=""/>
+					</file>
+					</checkstyle>
+				`
+			});
+			executeCommentsAction({
+				args: Object.assign({}, args, {
+					failOnSeverity: ['error'],
+				}),
+				config,
+				credentials
+			})
+			.then(() => done('expected error severity'))
+			.catch(({message}) => {
+				chai.expect(message).to.equal('Severities hit: error');
+				done();
+			});
+		});
+
+		it('returns error if multiple severities are hit', (done) => {
+			mock({
+				'checkstyle.xml': `
+					<?xml version="1.0" encoding="utf-8"?>
+					<checkstyle version="4.3">
+					<file name="package.json">
+						<error line="10" column="1" severity="info" message=""/>
+						<error line="11" column="1" severity="warning" message=""/>
+						<error line="12" column="1" severity="error" message=""/>
+					</file>
+					</checkstyle>
+				`
+			});
+
+			executeCommentsAction({
+				args: Object.assign({}, args, {
+					failOnSeverity: ['error', 'warning'],
+				}),
+				config,
+				credentials
+			})
+			.then(() => done('expected error severity'))
+			.catch(({ message }) => {
+				chai.expect(message).to.equal('Severities hit: error, warning');
+				done();
+			});
+		});
+	});
 	describe('when calling getPreviousCommentIds', () => {
 
 		const currentUser = { username: 'bob' };
@@ -75,3 +206,30 @@ describe('Action Comments', () => {
 		});
 	});
 });
+
+
+function getDiff() {
+	return `
+diff --git a/package.json b/package.json
+index a01fff5..c6020e4 100644
+--- a/package.json
++++ b/package.json
+@@ -10,14 +10,14 @@
+	 "postinstall": "do something",
+	 "preinstall": "do something else"
+   },
++  "license": "MIT",
+   "private": true,
+-  "version": "6.0.0",
+   "dependencies": {
+	 "dependency-100": "1.0.0",
+	 "dependency-101": "1.0.0"
+   },
+   "devDependencies": {
+-    "dependency-1": "1.0.0",
++    "dependency-1": "1.0.1",
+	 "dependency-2": "1.0.0",
+	 "dependency-3": "1.0.0",
+	 "dependency-4": "1.0.0",
+`;
+}