Checkout now reports failures through exceptions.

Checkout sometimes throws an exception, and
other times it writes an error message to outw
and returns normally, even though the command
failed.  This commit now reports all failures
through a die() exception.

Change-Id: I038a5d976d95020fea3faac68e9178f923c25b28
Signed-off-by: Ned Twigg <ned.twigg@diffplug.com>

diff --git a/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/CheckoutTest.java b/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/CheckoutTest.java
index 4b86b6014ecfd31b397c53f7b567f43f57e2ded2..a98dd1cf98928dfdd2321bd899ea7c0e560c1c07 100644 (file)
--- a/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/CheckoutTest.java
+++ b/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/CheckoutTest.java
@@ -71,6 +71,23 @@ import org.junit.Assume;
 import org.junit.Test;
 
 public class CheckoutTest extends CLIRepositoryTestCase {
+       /**
+        * Executes specified git command (with arguments), captures exception and
+        * returns its message as an array of lines. Throws an AssertionError if no
+        * exception is thrown.
+        *
+        * @param command
+        *            a valid git command line, e.g. "git branch -h"
+        * @return message contained within the exception
+        */
+       private String[] executeExpectingException(String command) {
+               try {
+                       execute(command);
+                       throw new AssertionError("Expected Die");
+               } catch (Exception e) {
+                       return e.getMessage().split(System.lineSeparator());
+               }
+       }
 
        @Test
        public void testCheckoutSelf() throws Exception {
@@ -107,7 +124,7 @@ public class CheckoutTest extends CLIRepositoryTestCase {
        public void testCheckoutNonExistingBranch() throws Exception {
                assertStringArrayEquals(
                                "error: pathspec 'side' did not match any file(s) known to git.",
-                               execute("git checkout side"));
+                               executeExpectingException("git checkout side"));
        }
 
        @Test
@@ -131,7 +148,7 @@ public class CheckoutTest extends CLIRepositoryTestCase {
        public void testCheckoutUnresolvedHead() throws Exception {
                assertStringArrayEquals(
                                "error: pathspec 'HEAD' did not match any file(s) known to git.",
-                               execute("git checkout HEAD"));
+                               executeExpectingException("git checkout HEAD"));
        }
 
        @Test
@@ -159,7 +176,8 @@ public class CheckoutTest extends CLIRepositoryTestCase {
                        writeTrashFile("a", "New Hello world a");
                        git.add().addFilepattern(".").call();
 
-                       String[] execute = execute("git checkout branch_1");
+                       String[] execute = executeExpectingException(
+                                       "git checkout branch_1");
                        assertEquals(
                                        "error: Your local changes to the following files would be overwritten by checkout:",
                                        execute[0]);

diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Checkout.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Checkout.java
index b5cf56e9389edcfae629496bb7ec5d5b0a2cbcd7..2af1eca807e4c69e9742ab6437dd4df65e645fef 100644 (file)
--- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Checkout.java
+++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Checkout.java
@@ -122,17 +122,21 @@ class Checkout extends TextBuiltin {
                                                        CLIText.get().switchedToBranch,
                                                        Repository.shortenRefName(ref.getName())));
                        } catch (RefNotFoundException e) {
-                               outw.println(MessageFormat.format(
-                                               CLIText.get().pathspecDidNotMatch,
-                                               name));
+                               throw die(MessageFormat
+                                               .format(CLIText.get().pathspecDidNotMatch, name), e);
                        } catch (RefAlreadyExistsException e) {
-                               throw die(MessageFormat.format(CLIText.get().branchAlreadyExists,
-                                               name));
+                               throw die(MessageFormat
+                                               .format(CLIText.get().branchAlreadyExists, name));
                        } catch (CheckoutConflictException e) {
-                               outw.println(CLIText.get().checkoutConflict);
-                               for (String path : e.getConflictingPaths())
-                                       outw.println(MessageFormat.format(
+                               StringBuilder builder = new StringBuilder();
+                               builder.append(CLIText.get().checkoutConflict);
+                               builder.append(System.lineSeparator());
+                               for (String path : e.getConflictingPaths()) {
+                                       builder.append(MessageFormat.format(
                                                        CLIText.get().checkoutConflictPathLine, path));
+                                       builder.append(System.lineSeparator());
+                               }
+                               throw die(builder.toString(), e);
                        }
                }
        }