diff --git a/CHANGES.rst b/CHANGES.rst index 8f92614c..6d9871ea 100644 --- a/CHANGES.rst +++ b/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] diff --git a/zest/releaser/baserelease.py b/zest/releaser/baserelease.py index 82b4e88b..458e3ca0 100644 --- a/zest/releaser/baserelease.py +++ b/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, diff --git a/zest/releaser/tests/baserelease.txt b/zest/releaser/tests/baserelease.txt index fbe60853..c49eefb2 100644 --- a/zest/releaser/tests/baserelease.txt +++ b/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 diff --git a/zest/releaser/tests/release.txt b/zest/releaser/tests/release.txt index aa0d325f..afc66ce5 100644 --- a/zest/releaser/tests/release.txt +++ b/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. diff --git a/zest/releaser/tests/utils.txt b/zest/releaser/tests/utils.txt index d238a44d..f6b2ec7b 100644 --- a/zest/releaser/tests/utils.txt +++ b/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. diff --git a/zest/releaser/utils.py b/zest/releaser/utils.py index fa9bd769..66a65523 100644 --- a/zest/releaser/utils.py +++ b/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)