From ad74e00a7757f1bf686c8e93490c6c3ccdd48fa1 Mon Sep 17 00:00:00 2001 From: Jakub Kadlcik Date: Sun, 26 Apr 2020 12:26:28 +0200 Subject: [PATCH] Make yes or no input less aggressive Fix #364 Previously all non-yes input was considered to be "no". Which is not what we typically want. There should be expected "yes" inputs, expected "no" inputs and everything else should be considered to be an uknown input and should be treated this way. I think that printing an error for unknown input is too verbose and have no added value. Simply repeating the question is the way to go here: ##### Please review the above diff ##### Do you wish to proceed with commit? [y/n] foo Do you wish to proceed with commit? [y/n] y Proceeding with commit. --- src/tito/release/main.py | 16 ++++++++++------ test/unit/releaser_tests.py | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 6 deletions(-) create mode 100644 test/unit/releaser_tests.py diff --git a/src/tito/release/main.py b/src/tito/release/main.py index fdbcfb4..7668f08 100644 --- a/src/tito/release/main.py +++ b/src/tito/release/main.py @@ -102,12 +102,16 @@ class Releaser(ConfigObject): def _ask_yes_no(self, prompt="Y/N? ", default_auto_answer=True): if self.auto_accept: return default_auto_answer - else: - if PY2: - answer = raw_input(prompt) - else: - answer = input(prompt) - return answer.lower() in ['y', 'yes', 'ok', 'sure'] + + yes = ['y', 'yes', 'ok', 'sure'] + no = ['n', 'no', 'nah', 'nope'] + answers = yes + no + + while True: + input_function = raw_input if PY2 else input + answer = input_function(prompt).lower() + if answer in answers: + return answer in yes def _check_releaser_config(self): """ diff --git a/test/unit/releaser_tests.py b/test/unit/releaser_tests.py new file mode 100644 index 0000000..7dbbd64 --- /dev/null +++ b/test/unit/releaser_tests.py @@ -0,0 +1,36 @@ +import unittest +import mock +from tito.compat import PY2, RawConfigParser +from tito.release import Releaser +from unit import builtins_input + + +class ReleaserTests(unittest.TestCase): + + @mock.patch("tito.release.main.create_builder") + @mock.patch("tito.release.main.mkdtemp") + def setUp(self, mkdtemp, create_builder): + self.config = RawConfigParser() + + self.releaser_config = RawConfigParser() + self.releaser_config.add_section("test") + self.releaser_config.set('test', "releaser", + "tito.release.Releaser") + + self.releaser = Releaser("titotestpkg", None, "/tmp/tito/", + self.config, {}, "test", self.releaser_config, False, + False, False, **{"offline": True}) + + @mock.patch(builtins_input) + def test_ask_yes_or_no(self, input_mock): + input_mock.side_effect = "y" + assert self.releaser._ask_yes_no() + + input_mock.side_effect = "n" + assert not self.releaser._ask_yes_no() + + input_mock.side_effect = ["yy", "y"] + assert self.releaser._ask_yes_no() + + input_mock.side_effect = ["yy", "no"] + assert not self.releaser._ask_yes_no()