CLI: remove double call of normal callback for InteractiveOption (#…

…5064)

In the `InteractiveOption` providing a normal callback is wrapped
in the method `after_callback` while the actual callback is replaced
by `prompt_callback`. Now the following situation occurs when the
parameter should be prompted for:

   1. `prompt_loop` is entered via `prompt_callback` and the value
      is converted in `safely_convert`
   2. `safely_convert` also includes a call to
      `prompt_callback`, where a call to `after_callback` is
      performed
   3. After a value succeeds the code goes back into the place, where
      the `prompt_loop` was entered in `prompt_callback`
      and continues with an additional call to `after_callback`
      which already executed before

The fix here is to directly return after the `prompt_loop` finishes and
handle the case, where the value should not be prompted separately

aiida/cmdline/params/options/interactive.py

@@ -271,11 +271,11 @@ def prompt_callback(self, ctx, param, value):
 
             # If we are here, we are in interactive mode and the parameter is not specified
             # We enter the prompt loop
-            value = self.prompt_loop(ctx, param, value)
-        else:
-            # There is a prompt_fn function and returns False (i.e. should not ask for this value
-            # We then set the value to None
-            value = None
+            return self.prompt_loop(ctx, param, value)
+
+        # There is a prompt_fn function and returns False (i.e. should not ask for this value
+        # We then set the value to None
+        value = None
 
         # And then we call the callback
         return self.after_callback(ctx, param, value)

tests/cmdline/params/options/test_interactive.py

@@ -47,6 +47,21 @@ def validate_positive_number(ctx, param, value):  # pylint: disable=unused-argum
         raise BadParameter(f'{value} is not a valid positive number')
 
 
+def validate_positive_number_with_echo(ctx, param, value):  # pylint: disable=unused-argument
+    """Validate that the number passed to this parameter is a positive number.
+       Also echos a message to the terminal
+
+    :param ctx: the `click.Context`
+    :param param: the parameter
+    :param value: the value passed for the parameter
+    :raises `click.BadParameter`: if the value is not a positive number
+    """
+    click.echo(f'Validating {value}')
+    if not isinstance(value, (int, float)) or value < 0:
+        from click import BadParameter
+        raise BadParameter(f'{value} is not a valid positive number')
+
+
 class InteractiveOptionTest(unittest.TestCase):
     """Unit tests for InteractiveOption."""
 
@@ -95,6 +110,29 @@ def test_callback_prompt_twice(self):
         self.assertIn(expected_3, lines[9])
         self.assertIn(expected_4, lines[12])
 
+    def test_callback_prompt_only_once(self):
+        """
+        scenario: using InteractiveOption with type=float and callback that echos an additional message
+        behaviour: the callback should be called at most once per prompt
+        """
+        cmd = self.simple_command(type=float, callback=validate_positive_number_with_echo)
+        runner = CliRunner()
+        result = runner.invoke(cmd, [], input='string\n-1\n1\n')
+        self.assertIsNone(result.exception)
+        expected_1 = 'Error: string is not a valid floating point value'
+        expected_2 = 'Validating -1.0'
+        expected_3 = 'Error: -1.0 is not a valid positive number'
+        expected_4 = 'Validating 1.0'
+        expected_5 = '1.0'
+        lines = result.output.split('\n')
+        #The callback should be called once per prompt
+        #where type conversion was successful
+        self.assertEqual(expected_1, lines[3])
+        self.assertEqual(expected_2, lines[6])
+        self.assertEqual(expected_3, lines[7])
+        self.assertEqual(expected_4, lines[10])
+        self.assertEqual(expected_5, lines[11])
+
     def test_prompt_str(self):
         """
         scenario: using InteractiveOption with type=str