Skip to content

Commit

Permalink
Properly implement the canApprove method in executable reporter
Browse files Browse the repository at this point in the history
This uses 'where' on windows and 'type' on unix to check if the
executable exists. In the future we might implement this by scanning the
PATH variable on different platforms.

closes nikolavp#14
  • Loading branch information
nikolavp committed Sep 1, 2014
1 parent f4cb474 commit b9099a9
Show file tree
Hide file tree
Showing 10 changed files with 181 additions and 23 deletions.
9 changes: 9 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,12 @@ script: "mvn clean test verify"

after_success:
- mvn clean cobertura:cobertura coveralls:cobertura


notifications:
webhooks:
urls:
- https://webhooks.gitter.im/e/2aecec65e90b910aaefc
on_success: change # options: [always|never|change] default: always
on_failure: always # options: [always|never|change] default: always
on_start: false # default: false
5 changes: 4 additions & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
<coberturaCoverageRate>89</coberturaCoverageRate>
<individual.coverage>70</individual.coverage>
<guava.version>13.0.1</guava.version>
<cobertura.format>xml</cobertura.format>
</properties>
<build>
<plugins>
Expand Down Expand Up @@ -101,13 +102,15 @@
<artifactId>cobertura-maven-plugin</artifactId>
<version>2.6</version>
<configuration>
<format>xml</format>
<format>${cobertura.format}</format>
<instrumentation>
<ignores>
<ignore>com.nikolavp.approval.reporters.*Reporters*</ignore>
<ignore>com.nikolavp.approval.utils.*ExecutableExistsOnPath*</ignore>
</ignores>
<excludes>
<exclude>com/nikolavp/approval/reporters/*Reporters*.class</exclude>
<exclude>com/nikolavp/approval/utils/*ExecutableExistsOnPath*.class</exclude>
</excludes>
</instrumentation>
<check>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@
*/

import com.nikolavp.approval.Reporter;
import com.nikolavp.approval.utils.ExecutableExistsOnPath;

import javax.annotation.Nullable;
import java.io.File;
import java.io.IOException;
import java.util.ArrayList;
Expand All @@ -39,16 +41,19 @@
public class ExecutableDifferenceReporter implements Reporter {
private final String diffCommand;
private final String approvalCommand;
private final String executable;

/**
* Main constructor for the executable reporter.
*
* @param approvalCommand the approval command
* @param diffCommand the difference command
* @param executable the executable for which to check. If null we won't check for executable existance
*/
public ExecutableDifferenceReporter(String approvalCommand, String diffCommand) {
public ExecutableDifferenceReporter(String approvalCommand, String diffCommand, @Nullable String executable) {
this.approvalCommand = approvalCommand;
this.diffCommand = diffCommand;
this.executable = executable;
}

protected String getDiffCommand() {
Expand Down Expand Up @@ -91,6 +96,9 @@ protected String[] buildApproveNewCommand(File approvalDestination, File fileFor

@Override
public boolean canApprove(File fileForApproval) {
if (executable != null) {
return new ExecutableExistsOnPath(executable).execute();
}
return true;
}

Expand Down Expand Up @@ -127,4 +135,5 @@ static List<String> buildCommandline(String... cmdParts) {
cmd.addAll(Arrays.asList(cmdParts).subList(1, cmdParts.length));
return cmd;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,10 @@ private MacOSReporters() {

}

private static final ExecutableDifferenceReporter KSDIFF = new ExecutableDifferenceReporter("ksdiff", "ksdiff");
private static final ExecutableDifferenceReporter DIFF_MERGE = new ExecutableDifferenceReporter("DiffMerge", "DiffMerge");
private static final ExecutableDifferenceReporter P4_MERGE = new ExecutableDifferenceReporter("p4merge", "p4merge");
private static final ExecutableDifferenceReporter TK_DIFF = new ExecutableDifferenceReporter("tkdiff", "tkdiff");
private static final ExecutableDifferenceReporter KSDIFF = new ExecutableDifferenceReporter("ksdiff", "ksdiff", "ksdiff");
private static final ExecutableDifferenceReporter DIFF_MERGE = new ExecutableDifferenceReporter("DiffMerge", "DiffMerge", "DiffMerge");
private static final ExecutableDifferenceReporter P4_MERGE = new ExecutableDifferenceReporter("p4merge", "p4merge", "p4merge");
private static final ExecutableDifferenceReporter TK_DIFF = new ExecutableDifferenceReporter("tkdiff", "tkdiff", "tkdiff");

/**
* A reporter that calls <a href="https://sourcegear.com/diffmerge/">diffmerge</a> to show you the results.
Expand Down
14 changes: 10 additions & 4 deletions src/main/java/com/nikolavp/approval/reporters/Reporters.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

import com.nikolavp.approval.Reporter;
import com.nikolavp.approval.utils.CrossPlatformCommand;
import com.nikolavp.approval.utils.ExecutableExistsOnPath;

import java.io.File;
import java.io.IOException;
Expand All @@ -38,18 +39,23 @@ private Reporters() {

}

private static final Reporter VIM_INSTANCE = new ExecutableDifferenceReporter("gvimdiff -f", "gvimdiff -f");
private static final Reporter GEDIT = SwingInteractiveReporter.wrap(new ExecutableDifferenceReporter("gedit -w", "gedit -w") {
private static final Reporter VIM_INSTANCE = new ExecutableDifferenceReporter("gvimdiff -f", "gvimdiff -f", "gvimdiff");
private static final Reporter GEDIT = SwingInteractiveReporter.wrap(new ExecutableDifferenceReporter("gedit -w", "gedit -w", "gedit") {
@Override
protected String[] buildApproveNewCommand(File approvalDestination, File fileForVerification) {
return new String[] {getApprovalCommand(), approvalDestination.getAbsolutePath()};
}
});
private static final Reporter CONSOLE_REPORTER_INSTANCE = SwingInteractiveReporter.wrap(new ExecutableDifferenceReporter("cat", "diff -u") {
private static final Reporter CONSOLE_REPORTER_INSTANCE = SwingInteractiveReporter.wrap(new ExecutableDifferenceReporter("cat", "diff -u", "cat") {
@Override
protected String[] buildApproveNewCommand(File approvalDestination, File fileForVerification) {
return new String[] {getApprovalCommand(), approvalDestination.getAbsolutePath()};
}

@Override
public boolean canApprove(File fileForApproval) {
return super.canApprove(fileForApproval) && new ExecutableExistsOnPath("diff").execute();
}
});

/**
Expand Down Expand Up @@ -109,7 +115,7 @@ protected String onUnix() {

} .execute();

return SwingInteractiveReporter.wrap(new ExecutableDifferenceReporter(cmd, cmd) {
return SwingInteractiveReporter.wrap(new ExecutableDifferenceReporter(cmd, cmd, null) {
@Override
protected String[] buildApproveNewCommand(File approvalDestination, File fileForVerification) {
return new String[]{getApprovalCommand(), approvalDestination.getAbsolutePath()};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,10 @@ public static class WindowsExecutableReporter extends ExecutableDifferenceReport
*
* @param approvalCommand the approval command
* @param diffCommand the difference command
* @param executable the executable which will be executed
*/
public WindowsExecutableReporter(String approvalCommand, String diffCommand) {
super(approvalCommand, diffCommand);
public WindowsExecutableReporter(String approvalCommand, String diffCommand, String executable) {
super(approvalCommand, diffCommand, executable);
}

@Override
Expand All @@ -57,16 +58,16 @@ public boolean canApprove(File fileForApproval) {
}
}

private static final Reporter NOTEPAD_PLUS_PLUS = SwingInteractiveReporter.wrap(new WindowsExecutableReporter("cmd /C notepad++", "cmd /C notepad++") {
private static final Reporter NOTEPAD_PLUS_PLUS = SwingInteractiveReporter.wrap(new WindowsExecutableReporter("cmd /C notepad++", "cmd /C notepad++", "notepad++") {
@Override
protected String[] buildApproveNewCommand(File approvalDestination, File fileForVerification) {
return new String[] {getApprovalCommand(), approvalDestination.getAbsolutePath()};
}
});
private static final ExecutableDifferenceReporter BEYOND_COMPARE = new WindowsExecutableReporter("cmd /C BCompare", "cmd /C BCompare");
private static final ExecutableDifferenceReporter TORTOISE_IMAGE_DIFF = new WindowsExecutableReporter("cmd /C TortoiseIDiff", "cmd /C TortoiseIDiff");
private static final ExecutableDifferenceReporter TORTOISE_TEXT_DIFF = new WindowsExecutableReporter("cmd /C TortoiseMerge", "cmd /C TortoiseMerge");
private static final ExecutableDifferenceReporter WIN_MERGE = new WindowsExecutableReporter("cmd /C WinMergeU", "cmd /C WinMergeU");
private static final ExecutableDifferenceReporter BEYOND_COMPARE = new WindowsExecutableReporter("cmd /C BCompare", "cmd /C BCompare", "BCompare");
private static final ExecutableDifferenceReporter TORTOISE_IMAGE_DIFF = new WindowsExecutableReporter("cmd /C TortoiseIDiff", "cmd /C TortoiseIDiff", "TortoiseIDiff");
private static final ExecutableDifferenceReporter TORTOISE_TEXT_DIFF = new WindowsExecutableReporter("cmd /C TortoiseMerge", "cmd /C TortoiseMerge", "TortoiseMerge");
private static final ExecutableDifferenceReporter WIN_MERGE = new WindowsExecutableReporter("cmd /C WinMergeU", "cmd /C WinMergeU", "WinMergeU");

/**
* A reporter that calls <a href="http://notepad-plus-plus.org/">notepad++</a> to show you the results.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
package com.nikolavp.approval.utils;

/*
* #%L
* com.github.nikolavp:approval-core
* %%
* Copyright (C) 2014 Nikolavp
* %%
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
* #L%
*/

import com.nikolavp.approval.reporters.ExecutableDifferenceReporter;

import java.io.IOException;

/**
* An utility class that checks if a specified executable exists in your PATH environment variable.
*/
public class ExecutableExistsOnPath extends CrossPlatformCommand<Boolean> {
private final String executable;

/**
* Main constructor that acceptance the executable to check for.
*
* @param executable the executable name
*/
public ExecutableExistsOnPath(String executable) {
this.executable = executable;
}

@Override
protected Boolean onWindows() {
try {
return ExecutableDifferenceReporter.runProcess("where " + executable)
.waitFor() == 0;
} catch (InterruptedException e) {
return Boolean.FALSE;
} catch (IOException e) {
return Boolean.FALSE;
}
}

@Override
protected Boolean onUnix() {
try {
//Type is the mostl portable way to check this. Some shells that implement POSIX don't implement the where or which builtin
//http://stackoverflow.com/questions/4781772/how-to-test-if-an-executable-exists-in-the-path-from-a-windows-batch-file
return ExecutableDifferenceReporter.runProcess("/bin/sh", "-c", "type " + executable)
.waitFor() == 0;
} catch (InterruptedException e) {
return Boolean.FALSE;
} catch (IOException e) {
return Boolean.FALSE;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@

import static com.nikolavp.approval.TestUtils.RAW_VALUE;
import static com.nikolavp.approval.TestUtils.forApproval;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.*;

/**
Expand All @@ -55,7 +57,7 @@ public class ExecutableDifferenceReporterTest {

@Before
public void setupMocks() {
executableDifferenceReporter = Mockito.spy(new ExecutableDifferenceReporter(GVIM_EXECUTABLE, GDIFFVIM_EXECUTABLE));
executableDifferenceReporter = Mockito.spy(new ExecutableDifferenceReporter(GVIM_EXECUTABLE, GDIFFVIM_EXECUTABLE, "gvimdiff"));
}

@Test(expected = AssertionError.class)
Expand All @@ -75,7 +77,7 @@ public void shouldThrowAnExceptionIfExecutableReturdErrorCode() throws Exception
executableDifferenceReporter.approveNew("test content".getBytes(StandardCharsets.UTF_8), forApproval(testFile), testFile.file());
Assert.fail("Should throw an exception");
} catch (AssertionError error) {
Assert.assertTrue(true);
assertTrue(true);
}
}
}
Expand Down Expand Up @@ -110,8 +112,8 @@ public void shouldThrowAssertionError_IfThereIsErrorWhileExecutingNotSame() thro
@Test
public void shouldBuildTheCommandLineProperlyIfInitialCommandsHaveArguments() throws Exception {
List<String> cmd = ExecutableDifferenceReporter.buildCommandline(GVIM_EXECUTABLE, "test");
Assert.assertThat(cmd.size(), CoreMatchers.equalTo(3));
Assert.assertThat(cmd.get(1), CoreMatchers.equalTo("-f"));
assertThat(cmd.size(), CoreMatchers.equalTo(3));
assertThat(cmd.get(1), CoreMatchers.equalTo("-f"));
}

@Test
Expand All @@ -129,4 +131,19 @@ public void shouldThrowAnExceptionIfExecutableIsNotFound() throws Exception {
Process process = executableDifferenceReporter.startProcess("unexistingcommand", "-invalid-flag");
process.waitFor();
}

@Test
public void shouldReportProperDiffCommandAndApprovalCommandsWhenAsked() throws Exception {
final String approvalCommand = executableDifferenceReporter.getApprovalCommand();
assertThat(approvalCommand, CoreMatchers.equalTo(GVIM_EXECUTABLE));

final String diffCommand = executableDifferenceReporter.getDiffCommand();
assertThat(diffCommand, CoreMatchers.equalTo(GDIFFVIM_EXECUTABLE));
}

@Test
public void shouldNotCheckIfExecutableExistsIfItIsNull() throws Exception {
final boolean canApprove = new ExecutableDifferenceReporter(GVIM_EXECUTABLE, GDIFFVIM_EXECUTABLE, null).canApprove(testFile.file());
assertTrue(canApprove);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,8 @@ public void shouldReturnCanApproveFromOther() throws Exception {
assertThat(interactiveReporter.canApprove(testFile.file()), equalTo(true));
}

@Test
public void shouldNotCallSwingIfInHeadlessMode() throws Exception {
@Test(expected = AssertionError.class)
public void shouldThrowIfCalledInHeadlessMode() throws Exception {
//assign
doThrow(new AssertionError("error")).when(interactiveReporter).promptUser();
doReturn(true).when(interactiveReporter).isHeadless();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package com.nikolavp.approval.utils;

/*
* #%L
* com.github.nikolavp:approval-core
* %%
* Copyright (C) 2014 Nikolavp
* %%
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
* #L%
*/

import org.hamcrest.CoreMatchers;
import org.junit.Assert;
import org.junit.Assume;
import org.junit.Test;

/**
* User: nikolavp (Nikola Petrov) Date: 14-9-1 Time: 8:39
*/
public class ExecutableExistsOnPathTest {
@Test
public void shouldProperlyExecuteOnWindows() throws Exception {
CrossPlatformCommand.setOS("Windows NT");
Assert.assertThat(new ExecutableExistsOnPath("non-existing-executable").execute(), CoreMatchers.equalTo(false));
}

@Test
public void shouldProperlyExecuteOnUnix() throws Exception {
CrossPlatformCommand.setOS("Linux");
Assert.assertThat(new ExecutableExistsOnPath("non-existing-executable").execute(), CoreMatchers.equalTo(false));

Assume.assumeTrue(CrossPlatformCommand.isUnix());
Assert.assertThat(new ExecutableExistsOnPath("vim").execute(), CoreMatchers.equalTo(true));
}
}

0 comments on commit b9099a9

Please sign in to comment.