diff --git a/git_upstream/lib/rebaseeditor.py b/git_upstream/lib/rebaseeditor.py index ffef11e..44b0fab 100644 --- a/git_upstream/lib/rebaseeditor.py +++ b/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 diff --git a/git_upstream/tests/commands/import/test_import.py b/git_upstream/tests/commands/import/test_import.py index 6366414..f317c48 100644 --- a/git_upstream/tests/commands/import/test_import.py +++ b/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() diff --git a/git_upstream/tests/commands/import/test_interactive.py b/git_upstream/tests/commands/import/test_interactive.py index f044fde..d79994c 100644 --- a/git_upstream/tests/commands/import/test_interactive.py +++ b/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/"))