When a command fails, always ask the user to continue or not.

Interestingly, this needed some fixes in the tests, where commands had gone wrong for years but we never noticed.

* At the end of `fullrelease` we try to push, and this always failed because there was no push target.
  We now ignore the push commands and instead print them.
* One test renames `CHANGES.rst` to `CHANGES.md`, and then calling `python setup.py --version` actually fails:
  `setup.py` still tries to read the no longer existing `CHANGES.rst`.

I hope we now strike a good balance between showing error output, exit codes, and asking if the user wants to continue.
With the previous alpha release, the balance was not good for me.  The final `git push` used `stderr` to report that everything was fine,
and then `postrelease` treated this as an error, and asked me if I wanted to continue.  Now we check the exit code.

CHANGES.rst

@@ -5,10 +5,10 @@ Changelog for zest.releaser
 --------------------
 
 - When a command we call exits with a non-zero exit code, clearly state this in the output.
+  Ask the user if she wants to continue or not.
   Note that this is tricky to do right.  Some commands like ``git`` seem to print everything to stderr,
   leading us to think there are errors, but the exit code is zero, so it should be fine.
-  We do not want to ask the user to often if she wants to continue or not.
-  But we do not want to silently swallow important errors either.
+  We do not want to ask too many questions, but we do not want to silently swallow important errors either.
   [maurits]
 
 

zest/releaser/baserelease.py

@@ -408,6 +408,9 @@ def _push(self):
         default_anwer = self.zest_releaser_config.push_changes()
         if utils.ask("OK to push commits to the server?", default=default_anwer):
             for push_cmd in push_cmds:
+                if utils.TESTMODE:
+                    logger.info("MOCK push command: %s", push_cmd)
+                    continue
                 output = execute_command(
                     push_cmd,
                     allow_retry=True,

zest/releaser/tests/baserelease.txt

@@ -155,6 +155,10 @@ history file ends with ``.md``, we consider it Markdown::
     >>> with open('setup.cfg', 'w') as f:
     ...     _ = f.write('\n'.join(lines))
     >>> rename_changelog("CHANGES.txt", "CHANGES.md")
+    >>> with open('setup.py') as f:
+    ...     setup_py_contents = f.read()
+    >>> with open('setup.py', 'w') as f:
+    ...     _ = f.write(setup_py_contents.replace("CHANGES.txt", "CHANGES.md"))
     >>> base = baserelease.Basereleaser()
     >>> base._grab_history()
     >>> base.history_format

zest/releaser/tests/release.txt

@@ -83,12 +83,9 @@ Mock that the tag exists and we get a question:
     Our reply: n
     >>> utils.test_answer_book.set_answers(['y'])
     >>> releaser._info_if_tag_already_exists()
-    Question: There is already a tag 0.1, show if there are differences? (Y/n)?
-    Our reply: y
-    git diff 0.1
-    RED ERROR: exit code 128.
-    RED fatal: ambiguous argument '0.1': unknown revision or path not in the working tree.
-    ...
+    Traceback (most recent call last):
+    ...diff_command...
+    RuntimeError: SYSTEM EXIT (code=1)
 
 Note: the diff itself fails as we mocked its existence.
 

zest/releaser/tests/utils.txt

@@ -489,9 +489,10 @@ We want to discover errors and show them in red.
     '\x1b[31m'
     >>> Fore.MAGENTA
     '\x1b[35m'
-    >>> output = utils.execute_command(['ls', 'some-non-existing-file'])
-    >>> output.startswith(Fore.RED)
-    True
+    >>> utils.execute_command(['ls', 'some-non-existing-file'])
+    Traceback (most recent call last):
+    ...
+    SystemExit: 1
 
 Warnings may also end up in the error output.  That may be unwanted.
 We have a script to test this, which passes all input to std error.

zest/releaser/utils.py

@@ -678,9 +678,9 @@ def _execute_command(command, cwd=None, extra_environ=None):
         if process.returncode:
             return (
                 Fore.RED
-                 + f"{ERROR_EXIT_CODE} {process.returncode}.\n"
-                 + process.stdout
-                 + get_errors(process.stderr)
+                + f"{ERROR_EXIT_CODE} {process.returncode}.\n"
+                + process.stdout
+                + get_errors(process.stderr)
             )
         return process.stdout + get_errors(process.stderr)
     # Only return the stdout. Stderr only contains possible
@@ -750,9 +750,24 @@ def execute_command(
     """
     result = _execute_command(command, cwd=cwd, extra_environ=extra_environ)
     if not allow_retry:
+        if ERROR_EXIT_CODE in result:
+            print(result)
+            if not ask(
+                "There were errors or warnings. Are you sure you want to continue?",
+                default=False,
+            ):
+                sys.exit(1)
+        # Note: whoever calls us could print the result.  This would be double
+        # in case there was an error code but the user continues.  So be it.
         return result
+
+    # At this point, a retry is allowed.  We only do this for very few commands.
     if AUTO_RESPONSE:
-        # Also don't ask for retry, just return the result.
+        # Retry is not possible with auto response, so just return the result.
+        if ERROR_EXIT_CODE in result:
+            # This is a real error, and the user cannot react.  We quit.
+            print(result)
+            sys.exit(1)
         return result
     if Fore.RED not in result or ERROR_EXIT_CODE not in result:
         show_interesting_lines(result)