diff options
-rw-r--r-- | git_review/cmd.py | 17 | ||||
-rw-r--r-- | git_review/tests/__init__.py | 20 | ||||
-rw-r--r-- | git_review/tests/test_git_review.py | 20 | ||||
-rw-r--r-- | releasenotes/notes/refuse-unstaged-changes-61600bd33adf3a45.yaml | 8 |
4 files changed, 61 insertions, 4 deletions
diff --git a/git_review/cmd.py b/git_review/cmd.py index b86cfdf..202c3b6 100644 --- a/git_review/cmd.py +++ b/git_review/cmd.py @@ -862,6 +862,23 @@ def rebase_changes(branch, remote, interactive=True): return False _orig_head = output + # Avoid trying to do a test rebase if there are uncommitted changes. + # Either the rebase will fail with a similar message, or if the user + # has turned on rebase.autostash then the subsequent reset will + # silently discard those changes. + cmd = "git diff --quiet" + (status, output) = run_command_status(cmd) + if status != 0: + printwrap("You have unstaged changes. Please commit or stash them " + "first, and then try again.") + sys.exit(1) + cmd = "git diff --cached --quiet" + (status, output) = run_command_status(cmd) + if status != 0: + printwrap("You have uncommitted changes. Please commit or stash them " + "first, and then try again.") + sys.exit(1) + cmd = "git show-ref --quiet --verify refs/%s" % remote_branch (status, output) = run_command_status(cmd) if status != 0: diff --git a/git_review/tests/__init__.py b/git_review/tests/__init__.py index 8652d42..379ac86 100644 --- a/git_review/tests/__init__.py +++ b/git_review/tests/__init__.py @@ -197,7 +197,7 @@ class BaseGitReviewTestCase(testtools.TestCase, GerritHelpers): Require Gerrit war file in the .gerrit directory to run Gerrit local. """ super(BaseGitReviewTestCase, self).setUp() - self.useFixture(fixtures.Timeout(2 * 60, True)) + self.useFixture(fixtures.Timeout(5 * 60, True)) # ensures git-review command runs in local mode (for functional tests) self.useFixture( @@ -313,13 +313,25 @@ class BaseGitReviewTestCase(testtools.TestCase, GerritHelpers): utils.run_cmd(gerrit_sh, 'start') self.addCleanup(utils.run_cmd, gerrit_sh, 'stop') - def _simple_change(self, change_text, commit_message, - file_=None): - """Helper method to create small changes and commit them.""" + def _unstaged_change(self, change_text, file_=None): + """Helper method to create small changes and not stage them.""" if file_ is None: file_ = self._dir('test', 'test_file.txt') + utils.write_to_file(file_, ''.encode()) + self._run_git('add', file_) utils.write_to_file(file_, change_text.encode()) + + def _uncommitted_change(self, change_text, file_=None): + """Helper method to create small changes and not commit them.""" + if file_ is None: + file_ = self._dir('test', 'test_file.txt') + self._unstaged_change(change_text, file_) self._run_git('add', file_) + + def _simple_change(self, change_text, commit_message, + file_=None): + """Helper method to create small changes and commit them.""" + self._uncommitted_change(change_text, file_) self._run_git('commit', '-m', commit_message) def _simple_amend(self, change_text, file_=None): diff --git a/git_review/tests/test_git_review.py b/git_review/tests/test_git_review.py index 6662299..66e4ced 100644 --- a/git_review/tests/test_git_review.py +++ b/git_review/tests/test_git_review.py @@ -222,6 +222,26 @@ class GitReviewTestCase(tests.BaseGitReviewTestCase): self.assertEqual(set(['reviewer1', 'reviewer2']), reviewers) + def test_rebase_unstaged_changes_msg(self): + """Test message displayed when unstaged changes are present.""" + self._run_git_review('-s') + self._run_git('checkout', '-b', 'test_branch') + # By not passing a filename, we rely on an edit to a preprovisioned + # tracked default file, which will be unstaged + self._unstaged_change(change_text='simple message') + exc = self.assertRaises(Exception, self._run_git_review) + self.assertIn("You have unstaged changes. Please", exc.args[0]) + + def test_rebase_uncommitted_changes_msg(self): + """Test message displayed when staged changes are present.""" + self._run_git_review('-s') + self._run_git('checkout', '-b', 'test_branch') + # By passing a filename, we rely on an edit to a new tracked + # file, which will be staged + self._uncommitted_change(change_text='simple message') + exc = self.assertRaises(Exception, self._run_git_review) + self.assertIn("You have uncommitted changes. Please", exc.args[0]) + def test_rebase_no_remote_branch_msg(self): """Test message displayed where no remote branch exists.""" self._run_git_review('-s') diff --git a/releasenotes/notes/refuse-unstaged-changes-61600bd33adf3a45.yaml b/releasenotes/notes/refuse-unstaged-changes-61600bd33adf3a45.yaml new file mode 100644 index 0000000..8e19e81 --- /dev/null +++ b/releasenotes/notes/refuse-unstaged-changes-61600bd33adf3a45.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + For safety, attempts to push a commit with unstaged or uncommitted changes + in the worktree will be caught and an error reported, rather than leaving + it up to ``git rebase`` to spot them. This addresses a situation where + users enabling *rebase.autostash* would otherwise experience data loss when + the test rebase is subsequently reset. |