Merge "Don't test rebasing with unstaged changes"

3 files changed, 45 insertions, 0 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/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.