Ensure correct mode of git-rebase executed

When not using interactive mode, ensure that git rebase is called as a
subprocess to ensure git-upstream can switch the final branch to the
merged result after completion of the import.

Only where interactive mode is enabled should the branch remain on the
import branch after running to completion and leave it to the user to
switch to the target branch, as otherwise git-rebase will throw an
error.

Change-Id: I633a371775b5a177c76529a49b44fb841d9c1332
Related-Bug: #1625876

git_upstream/lib/rebaseeditor.py

@@ -205,11 +205,32 @@ class RebaseEditor(GitMixin, LogDedentMixin):
                 return subprocess.call(cmd), None, None
             finally:
                 self.cleanup()
-        elif mode == "1":
-            # run in test mode to avoid replacing the existing process
-            # to keep the majority of tests simple and only require special
-            # launching code for those tests written to check the rebase
-            # resume behaviour
+        elif not self._interactive:
+            # If in non-interactive mode use subprocess instead of exec
+            #
+            # This ensures that if no conflicts occur, that the calling
+            # git-upstream process will be able to switch the current
+            # branch after the git-rebase subprocess exits. This is not
+            # possible when using exec to have git-rebase replace the
+            # existing process. Since git-rebase performs checks once
+            # it is completed running the instructions (todo file),
+            # changing the current branch checked out in the git
+            # repository via the final instruction (calling
+            # `git-upstream import --finish ...`) results in git-rebase
+            # exiting with an exception.
+            #
+            # For interactive mode it is impossible to perform a rebase
+            # via subprocess and have it correctly attach an editor to
+            # the console for users to edit/reword commits. The
+            # consequence of using exec to support interactive usage
+            # prevents correctly switching final branch to anything other
+            # than the branch that git-rebase was started on (which will
+            # be the import branch).
+            #
+            # As interactive mode involves user intervention it seems a
+            # reasonable compromise to require manual switch of branches
+            # after being finished until such time that an alternative
+            # solution can be found.
             try:
                 return 0, subprocess.check_output(
                     cmd, stderr=subprocess.STDOUT, env=environ), None

git_upstream/tests/commands/import/test_import.py

@@ -18,11 +18,11 @@
 import inspect
 import os
 
-import mock
 from testscenarios import TestWithScenarios
 from testtools.content import text_content
 from testtools.matchers import Contains
 from testtools.matchers import Equals
+from testtools.matchers import Not
 
 from git_upstream.lib.pygitcompat import Commit
 from git_upstream import main
@@ -30,8 +30,6 @@ from git_upstream.tests.base import BaseTestCase
 from git_upstream.tests.base import get_scenarios
 
 
-@mock.patch.dict('os.environ',
-                 {'TEST_GIT_UPSTREAM_REBASE_EDITOR': '1'})
 class TestImportCommand(TestWithScenarios, BaseTestCase):
 
     scenarios = get_scenarios(os.path.join(os.path.dirname(__file__),
@@ -56,6 +54,11 @@ class TestImportCommand(TestWithScenarios, BaseTestCase):
         self.assertThat(args.cmd.run(args), Equals(True),
                         "import command failed to complete successfully")
 
+        # assuming non-interactive we should *NOT* see the following message
+        # appear in the logged output.
+        self.assertThat(self.logger.output,
+                        Not(Contains("Successfully rebased and updated")))
+
         # perform sanity checks on results
         self._check_tree_state()
 

git_upstream/tests/commands/import/test_interactive.py

@@ -55,13 +55,14 @@ class TestImportInteractiveCommand(TestWithScenarios, BaseTestCase):
 
         cmdline = self.parser.get_default('script_cmdline') + self.parser_args
         try:
-            output = subprocess.check_output(cmdline, stderr=subprocess.STDOUT)
+            self.output = subprocess.check_output(cmdline,
+                                                  stderr=subprocess.STDOUT)
         except subprocess.CalledProcessError as cpe:
             self.addDetail('subprocess-output',
                            text_content(cpe.output.decode('utf-8')))
             raise
         self.addDetail('subprocess-output',
-                       text_content(output.decode('utf-8')))
+                       text_content(self.output.decode('utf-8')))
 
         expected = getattr(self, 'expect_rebased', [])
         if expected:
@@ -107,3 +108,8 @@ class TestImportInteractiveCommand(TestWithScenarios, BaseTestCase):
         extra_test_func = getattr(self, '_verify_%s' % self.name, None)
         if extra_test_func:
             extra_test_func()
+
+    def _verify_basic(self):
+        self.assertThat(
+            self.output.decode('utf-8'),
+            Contains("Successfully rebased and updated refs/heads/import/"))