Add tests for svn workflow (#2)

* Move svn-specific commands to namespace

* Add new loaders

* Pass shell_exec as a dependency

* Add tests for isNewSvnFile

* Use passed shell_exec for all functions

* Add tests for getSvnUnifiedDiff

* Fix tabs in tests

* Change workflows to return PhpcsMessages

This refactors the workflow functions so that they return a
PhpcsMessages object which makes them easier to test. The object is then
formatted and output by a separate function.

This change also modifies the `is_readable()` guard in
`runSvnWorkflow()` to be injected so it can be mocked for testing.

* Add happy path test for runSvnWorkflow

* Allow PhpcsMessages::fromPhpcsMessages to have null filename

* Add svn workflow test for new file

* Inject validateExecutableExists into runSvnWorkflow

* Group shell operations into UnixShell

* Remove vars injected into mock ShellOperator

* Add --version option

PhpcsChanged/Cli.php

@@ -3,12 +3,15 @@
 
 namespace PhpcsChanged\Cli;
 
+use PhpcsChanged\NonFatalException;
 use PhpcsChanged\Reporter;
 use PhpcsChanged\JsonReporter;
 use PhpcsChanged\FullReporter;
 use PhpcsChanged\PhpcsMessages;
 use PhpcsChanged\DiffLineMap;
-use function PhpcsChanged\{getNewPhpcsMessages, getNewPhpcsMessagesFromFiles};
+use PhpcsChanged\ShellOperator;
+use function PhpcsChanged\{getNewPhpcsMessages, getNewPhpcsMessagesFromFiles, getVersion};
+use function PhpcsChanged\SvnWorkflow\{getSvnUnifiedDiff, isNewSvnFile, getSvnBasePhpcsOutput, getSvnNewPhpcsOutput, validateSvnFileExists};
 
 function getDebug($debugEnabled) {
 	return function(...$outputs) use ($debugEnabled) {
@@ -42,6 +45,15 @@ function printTwoColumns(array $columns) {
 	echo PHP_EOL;
 }
 
+function printVersion() {
+	$version = getVersion();
+	echo <<<EOF
+phpcs-changed version {$version}
+
+EOF;
+	die(0);
+}
+
 function printHelp() {
 	echo <<<EOF
 A tool to run phpcs on changed sections of files.
@@ -82,6 +94,8 @@ function printHelp() {
 		'--standard <STANDARD>' => 'The phpcs standard to use.',
 		'--report <REPORTER>' => 'The phpcs reporter to use. One of "full" (default) or "json".',
 		'--debug' => 'Enable debug output.',
+		'--help' => 'Print this help.',
+		'--version' => 'Print the current version.',
 	]);
 	echo <<<EOF
 
@@ -104,7 +118,7 @@ function getReporter(string $reportType): Reporter {
 	printErrorAndExit("Unknown Reporter '{$reportType}'");
 }
 
-function runManualWorkflow($reportType, $diffFile, $phpcsOldFile, $phpcsNewFile): void {
+function runManualWorkflow($diffFile, $phpcsOldFile, $phpcsNewFile): PhpcsMessages {
 	try {
 		$messages = getNewPhpcsMessagesFromFiles(
 			$diffFile,
@@ -114,77 +128,41 @@ function runManualWorkflow($reportType, $diffFile, $phpcsOldFile, $phpcsNewFile)
 	} catch (\Exception $err) {
 		printErrorAndExit($err->getMessage());
 	}
-	$reporter = getReporter($reportType);
-	echo $reporter->getFormattedMessages($messages);
-	exit($reporter->getExitCode($messages));
-}
-
-function validateExecutableExists($name, $command) {
-	exec(sprintf("type %s > /dev/null 2>&1", escapeshellarg($command)), $ignore, $returnVal);
-	if ($returnVal != 0) {
-		throw new \Exception("Cannot find executable for {$name}, currently set to '{$command}'.");
-	}
+	return $messages;
 }
 
-function runSvnWorkflow($svnFile, $reportType, $options, $debug): void {
+function runSvnWorkflow($svnFile, $options, ShellOperator $shell, callable $debug): PhpcsMessages {
 	$svn = getenv('SVN') ?: 'svn';
 	$phpcs = getenv('PHPCS') ?: 'phpcs';
 	$cat = getenv('CAT') ?: 'cat';
+
 	try {
 		$debug('validating executables');
-		validateExecutableExists('svn', $svn);
-		validateExecutableExists('phpcs', $phpcs);
-		validateExecutableExists('cat', $cat);
-	} catch (\Exception $err) {
-		printErrorAndExit($err->getMessage());
-	}
-	$debug('executables are valid');
-	$phpcsStandard = $options['standard'] ?? null;
-	$phpcsStandardOption = $phpcsStandard ? ' --standard=' . escapeshellarg($phpcsStandard) : '';
-	if (! is_readable($svnFile)) {
-		printErrorAndExit("Cannot read file '{$svnFile}'");
-	}
-
-	$unifiedDiffCommand = "{$svn} diff " . escapeshellarg($svnFile);
-	$debug('running diff command:', $unifiedDiffCommand);
-	$unifiedDiff = shell_exec($unifiedDiffCommand);
-	if (! $unifiedDiff) {
-		$debug("Cannot get svn diff for file '{$svnFile}'; skipping");
+		$shell->validateExecutableExists('svn', $svn);
+		$shell->validateExecutableExists('phpcs', $phpcs);
+		$shell->validateExecutableExists('cat', $cat);
+		$debug('executables are valid');
+
+		$phpcsStandard = $options['standard'] ?? null;
+		$phpcsStandardOption = $phpcsStandard ? ' --standard=' . escapeshellarg($phpcsStandard) : '';
+		validateSvnFileExists($svnFile, [$shell, 'isReadable']);
+		$unifiedDiff = getSvnUnifiedDiff($svnFile, $svn, [$shell, 'executeCommand'], $debug);
+		$isNewFile = isNewSvnFile($svnFile, $svn, [$shell, 'executeCommand'], $debug);
+		$oldFilePhpcsOutput = $isNewFile ? '' : getSvnBasePhpcsOutput($svnFile, $svn, $phpcs, $phpcsStandardOption, [$shell, 'executeCommand'], $debug);
+		$newFilePhpcsOutput = getSvnNewPhpcsOutput($svnFile, $phpcs, $cat, $phpcsStandardOption, [$shell, 'executeCommand'], $debug);
+	} catch( NonFatalException $err ) {
+		$debug($err->getMessage());
 		exit(0);
+	} catch( \Exception $err ) {
+		printErrorAndExit($err->getMessage());
 	}
-	$debug('diff command output:', $unifiedDiff);
-
-	$svnStatusCommand = "${svn} info " . escapeshellarg($svnFile);
-	$debug('checking svn status of file with command:', $svnStatusCommand);
-	$svnStatusOutput = shell_exec($svnStatusCommand);
-	$debug('svn status output:', $svnStatusOutput);
-	if (! $svnStatusOutput || false === strpos($svnStatusOutput, 'Schedule:')) {
-		printErrorAndExit("Cannot get svn info for file '{$svnFile}'");
-	}
-	$isNewFile = (false !== strpos($svnStatusOutput, 'Schedule: add'));
-
-	$oldFilePhpcsOutput = '';
-	if (! $isNewFile) {
-		$oldFilePhpcsOutputCommand = "${svn} cat " . escapeshellarg($svnFile) . " | {$phpcs} --report=json" . $phpcsStandardOption;
-		$debug('running orig phpcs command:', $oldFilePhpcsOutputCommand);
-		$oldFilePhpcsOutput = shell_exec($oldFilePhpcsOutputCommand);
-		if (! $oldFilePhpcsOutput) {
-			printErrorAndExit("Cannot get old phpcs output for file '{$svnFile}'");
-		}
-		$debug('orig phpcs command output:', $oldFilePhpcsOutput);
-	}
-
-	$newFilePhpcsOutputCommand = "{$cat} " . escapeshellarg($svnFile) . " | {$phpcs} --report=json" . $phpcsStandardOption;
-	$debug('running new phpcs command:', $newFilePhpcsOutputCommand);
-	$newFilePhpcsOutput = shell_exec($newFilePhpcsOutputCommand);
-	if (! $newFilePhpcsOutput) {
-		printErrorAndExit("Cannot get new phpcs output for file '{$svnFile}'");
-	}
-	$debug('new phpcs command output:', $newFilePhpcsOutput);
 
 	$debug('processing data...');
 	$fileName = DiffLineMap::getFileNameFromDiff($unifiedDiff);
-	$messages = getNewPhpcsMessages($unifiedDiff, PhpcsMessages::fromPhpcsJson($oldFilePhpcsOutput, $fileName), PhpcsMessages::fromPhpcsJson($newFilePhpcsOutput, $fileName));
+	return getNewPhpcsMessages($unifiedDiff, PhpcsMessages::fromPhpcsJson($oldFilePhpcsOutput, $fileName), PhpcsMessages::fromPhpcsJson($newFilePhpcsOutput, $fileName));
+}
+
+function reportMessagesAndExit(PhpcsMessages $messages, string $reportType): void {
 	$reporter = getReporter($reportType);
 	echo $reporter->getFormattedMessages($messages);
 	exit($reporter->getExitCode($messages));

PhpcsChanged/NonFatalException.php

@@ -0,0 +1,7 @@
+<?php
+declare(strict_types=1);
+
+namespace PhpcsChanged;
+
+class NonFatalException extends \Exception {
+}

PhpcsChanged/PhpcsMessages.php

@@ -20,7 +20,9 @@ public function __construct(array $messages) {
 
 	public static function fromPhpcsMessages(array $messages, string $fileName = null): self {
 		return new self(array_map(function(PhpcsMessage $message) use ($fileName) {
-			$message->setFile($fileName);
+			if ($fileName) {
+				$message->setFile($fileName);
+			}
 			return $message;
 		}, $messages));
 	}

PhpcsChanged/ShellOperator.php

@@ -0,0 +1,15 @@
+<?php
+declare(strict_types=1);
+
+namespace PhpcsChanged;
+
+/**
+ * Interface to perform file and shell operations
+ */
+interface ShellOperator {
+	public function validateExecutableExists(string $name, string $command): void;
+
+	public function executeCommand(string $command): string;
+
+	public function isReadable(string $fileName): bool;
+}

PhpcsChanged/SvnWorkflow.php

@@ -0,0 +1,56 @@
+<?php
+declare(strict_types=1);
+
+namespace PhpcsChanged\SvnWorkflow;
+
+use PhpcsChanged\NonFatalException;
+
+function validateSvnFileExists(string $svnFile, callable $isReadable): void {
+	if (! $isReadable($svnFile)) {
+		throw new \Exception("Cannot read file '{$svnFile}'");
+	}
+}
+
+function getSvnUnifiedDiff(string $svnFile, string $svn, callable $executeCommand, callable $debug): string {
+	$unifiedDiffCommand = "{$svn} diff " . escapeshellarg($svnFile);
+	$debug('running diff command:', $unifiedDiffCommand);
+	$unifiedDiff = $executeCommand($unifiedDiffCommand);
+	if (! $unifiedDiff) {
+		throw new NonFatalException("Cannot get svn diff for file '{$svnFile}'; skipping");
+	}
+	$debug('diff command output:', $unifiedDiff);
+	return $unifiedDiff;
+}
+
+function isNewSvnFile(string $svnFile, string $svn, callable $executeCommand, callable $debug): bool {
+	$svnStatusCommand = "${svn} info " . escapeshellarg($svnFile);
+	$debug('checking svn status of file with command:', $svnStatusCommand);
+	$svnStatusOutput = $executeCommand($svnStatusCommand);
+	$debug('svn status output:', $svnStatusOutput);
+	if (! $svnStatusOutput || false === strpos($svnStatusOutput, 'Schedule:')) {
+		throw new \Exception("Cannot get svn info for file '{$svnFile}'");
+	}
+	return (false !== strpos($svnStatusOutput, 'Schedule: add'));
+}
+
+function getSvnBasePhpcsOutput(string $svnFile, string $svn, string $phpcs, string $phpcsStandardOption, callable $executeCommand, callable $debug): string {
+	$oldFilePhpcsOutputCommand = "${svn} cat " . escapeshellarg($svnFile) . " | {$phpcs} --report=json" . $phpcsStandardOption;
+	$debug('running orig phpcs command:', $oldFilePhpcsOutputCommand);
+	$oldFilePhpcsOutput = $executeCommand($oldFilePhpcsOutputCommand);
+	if (! $oldFilePhpcsOutput) {
+		throw new \Exception("Cannot get old phpcs output for file '{$svnFile}'");
+	}
+	$debug('orig phpcs command output:', $oldFilePhpcsOutput);
+	return $oldFilePhpcsOutput;
+}
+
+function getSvnNewPhpcsOutput(string $svnFile, string $phpcs, string $cat, string $phpcsStandardOption, callable $executeCommand, callable $debug): string {
+	$newFilePhpcsOutputCommand = "{$cat} " . escapeshellarg($svnFile) . " | {$phpcs} --report=json" . $phpcsStandardOption;
+	$debug('running new phpcs command:', $newFilePhpcsOutputCommand);
+	$newFilePhpcsOutput = $executeCommand($newFilePhpcsOutputCommand);
+	if (! $newFilePhpcsOutput) {
+		throw new \Exception("Cannot get new phpcs output for file '{$svnFile}'");
+	}
+	$debug('new phpcs command output:', $newFilePhpcsOutput);
+	return $newFilePhpcsOutput;
+}

PhpcsChanged/UnixShell.php

@@ -0,0 +1,26 @@
+<?php
+declare(strict_types=1);
+
+namespace PhpcsChanged;
+
+use PhpcsChanged\ShellOperator;
+
+/**
+ * Module to perform file and shell operations
+ */
+class UnixShell implements ShellOperator {
+	public function validateExecutableExists(string $name, string $command): void {
+		exec(sprintf("type %s > /dev/null 2>&1", escapeshellarg($command)), $ignore, $returnVal);
+		if ($returnVal != 0) {
+			throw new \Exception("Cannot find executable for {$name}, currently set to '{$command}'.");
+		}
+	}
+
+	public function executeCommand(string $command): string {
+		return shell_exec($command);
+	}
+
+	public function isReadable(string $fileName): bool {
+		return is_readable($fileName);
+	}
+}

PhpcsChanged/Version.php

@@ -0,0 +1,8 @@
+<?php
+declare(strict_types=1);
+
+namespace PhpcsChanged;
+
+function getVersion(): string {
+	return '2.1.0';
+}

bin/phpcs-changed

@@ -9,17 +9,19 @@ require_once __DIR__ . '/../index.php';
 
 use function PhpcsChanged\Cli\{
 	printHelp,
-	printErrorAndExit,
+	printVersion,
 	getDebug,
-	getReporter,
 	runManualWorkflow,
-	runSvnWorkflow
+	runSvnWorkflow,
+	reportMessagesAndExit
 };
+use PhpcsChanged\UnixShell;
 
 $options = getopt(
 	'h',
 	[
 		'help',
+		'version',
 		'diff:',
 		'phpcs-orig:',
 		'phpcs-new:',
@@ -34,6 +36,10 @@ if (isset($options['h']) || isset($options['help'])) {
 	printHelp();
 }
 
+if (isset($options['version'])) {
+	printVersion();
+}
+
 $debug = getDebug(isset($options['debug']));
 $debug('Running...');
 
@@ -43,12 +49,13 @@ $phpcsOldFile = $options['phpcs-orig'] ?? null;
 $phpcsNewFile = $options['phpcs-new'] ?? null;
 
 if ($diffFile && $phpcsOldFile && $phpcsNewFile) {
-	runManualWorkflow($reportType, $diffFile, $phpcsOldFile, $phpcsNewFile);
+	reportMessagesAndExit(runManualWorkflow($diffFile, $phpcsOldFile, $phpcsNewFile), $reportType);
 }
 
 $svnFile = $options['svn'] ?? null;
 if ($svnFile) {
-	runSvnWorkflow($svnFile, $reportType, $options, $debug);
+	$shell = new UnixShell();
+	reportMessagesAndExit(runSvnWorkflow($svnFile, $options, $shell, $debug), $reportType);
 }
 
 printHelp();

index.php

@@ -6,6 +6,7 @@
 use PhpcsChanged\DiffLineMap;
 use PhpcsChanged\PhpcsMessages;
 
+require_once __DIR__ . '/PhpcsChanged/Version.php';
 require_once __DIR__ . '/PhpcsChanged/DiffLine.php';
 require_once __DIR__ . '/PhpcsChanged/DiffLineType.php';
 require_once __DIR__ . '/PhpcsChanged/DiffLineMap.php';
@@ -15,6 +16,10 @@
 require_once __DIR__ . '/PhpcsChanged/Reporter.php';
 require_once __DIR__ . '/PhpcsChanged/JsonReporter.php';
 require_once __DIR__ . '/PhpcsChanged/FullReporter.php';
+require_once __DIR__ . '/PhpcsChanged/NonFatalException.php';
+require_once __DIR__ . '/PhpcsChanged/SvnWorkflow.php';
+require_once __DIR__ . '/PhpcsChanged/ShellOperator.php';
+require_once __DIR__ . '/PhpcsChanged/UnixShell.php';
 
 function getNewPhpcsMessages(string $unifiedDiff, PhpcsMessages $oldPhpcsMessages, PhpcsMessages $newPhpcsMessages): PhpcsMessages {
 	$map = DiffLineMap::fromUnifiedDiff($unifiedDiff);