WHILE and BREAK/CONTINUE enhancements.

Go WHILE loop body through also if there are failurs. This makes loop
contents visible in the log file. #4482

Don't raise errors about invalid syntax with BREAK and CONTINUE when
they are just gone through to show in log, not really executed.
Fixes #4481.

atest/robot/running/while/break_and_continue.robot

@@ -1,7 +1,7 @@
 *** Settings ***
 Resource          while.resource
 Suite Setup       Run Tests    ${EMPTY}    running/while/break_and_continue.robot
-Test Template     Check while loop
+Test Template     Check WHILE loop
 
 *** Test Cases ***
 With CONTINUE
@@ -31,6 +31,18 @@ With BREAK inside EXCEPT
 With BREAK inside TRY-ELSE
     PASS    1
 
+Invalid BREAK
+    FAIL    1
+
+Invalid CONTINUE
+    FAIL    1
+
+Invalid BREAK not executed
+    PASS    1
+
+Invalid CONTINUE not executed
+    NOT RUN    1
+
 With CONTINUE in UK
     PASS    5    body[0].body[0]
 

atest/robot/running/while/invalid_while.robot

@@ -1,26 +1,30 @@
 *** Settings ***
 Resource          while.resource
-Suite Setup       Run Tests    ${EMPTY}    running/while/invalid_while.robot
+Suite Setup       Run Tests    --log test_result_model_as_well    running/while/invalid_while.robot
 
 *** Test Cases ***
 No condition
-    Check Test Case    ${TESTNAME}
+    ${tc} =    Check Invalid WHILE Test Case
+    Should Be Equal    ${tc.body[0].condition}    ${NONE}
 
 Multiple conditions
-    ${tc} =    Check Test Case    ${TESTNAME}
+    ${tc} =   Check Invalid WHILE Test Case
     Should Be Equal    ${tc.body[0].condition}    Too, many, !
 
 Invalid condition
-    Check Test Case    ${TESTNAME}
+    Check Invalid WHILE Test Case
+
+Invalid condition on second round
+    Check Test Case    ${TEST NAME}
 
 Non-existing variable in condition
-    Check Test Case    ${TESTNAME}
+    Check Invalid WHILE Test Case
 
 No body
-    Check Test Case    ${TESTNAME}
+    Check Invalid WHILE Test Case    body=False
 
 No END
-    Check Test Case    ${TESTNAME}
+    Check Invalid WHILE Test Case
 
 Invalid data causes syntax error
     Check Test Case    ${TEST NAME}
@@ -30,3 +34,17 @@ Invalid condition causes normal error
 
 Non-existing variable in condition causes normal error
     Check Test Case    ${TEST NAME}
+
+*** Keywords ***
+Check Invalid WHILE Test Case
+    [Arguments]    ${body}=True
+    ${tc} =    Check Test Case    ${TESTNAME}
+    Should Be Equal    ${tc.body[0].type}              WHILE
+    Should Be Equal    ${tc.body[0].status}            FAIL
+    Should Be Equal    ${tc.body[0].body[0].type}      ITERATION
+    Should Be Equal    ${tc.body[0].body[0].status}    NOT RUN
+    IF    ${body}
+        Should Be Equal    ${tc.body[0].body[0].body[0].name}      BuiltIn.Fail
+        Should Be Equal    ${tc.body[0].body[0].body[0].status}    NOT RUN
+    END
+    RETURN    ${tc}

atest/robot/running/while/while.resource

@@ -1,9 +1,8 @@
 *** Settings ***
-Resource    atest_resource.robot
-
+Resource        atest_resource.robot
 
 *** Keywords ***
-Check while loop
+Check WHILE loop
     [Arguments]    ${status}    ${iterations}    ${path}=body[0]
     ${tc}=    Check test case    ${TEST NAME}
     ${loop}=    Check loop attributes     ${tc.${path}}    ${status}    ${iterations}

atest/testdata/running/while/break_and_continue.robot

@@ -106,6 +106,32 @@ With BREAK inside TRY-ELSE
     END
     Should be equal    ${variable}    ${2}
 
+Invalid BREAK
+    [Documentation]    FAIL    BREAK does not accept arguments, got 'bad'.
+    WHILE   True
+        BREAK    bad
+    END
+
+Invalid CONTINUE
+    [Documentation]    FAIL    CONTINUE does not accept arguments, got 'bad'.
+    WHILE   True
+        CONTINUE    bad
+    END
+
+Invalid BREAK not executed
+    WHILE   True
+        IF    False
+            BREAK    bad
+        ELSE
+            BREAK
+        END
+    END
+
+Invalid CONTINUE not executed
+    WHILE   False
+        CONTINUE    bad
+    END
+
 With CONTINUE in UK
     With CONTINUE in UK
 

atest/testdata/running/while/invalid_while.robot

@@ -17,6 +17,17 @@ Invalid condition
         Fail    Not executed!
     END
 
+Invalid condition on second round
+    [Documentation]    FAIL Evaluating WHILE condition failed: Evaluating expression 'bad' failed: NameError: name 'bad' is not defined nor importable as module
+    ${condition} =    Set Variable    True
+    WHILE    ${condition}
+        IF    ${condition}
+            ${condition} =    Set Variable    bad
+        ELSE
+            Fail    Not executed!
+        END
+    END
+
 Non-existing variable in condition
     [Documentation]    FAIL Evaluating WHILE condition failed: Variable '\${ooops}' not found.
     WHILE    ${ooops}

atest/testdata/running/while/while_limit.robot

@@ -1,7 +1,7 @@
 *** Variables ***
 ${variable}    ${1}
 ${limit}       11
-${number}      ${0.7}
+${number}      ${0.2}
 
 *** Test Cases ***
 Default limit is 10000 iterations
@@ -29,8 +29,8 @@ Limit with iteration count with underscore
     END
 
 Limit as timestr
-    [Documentation]     FAIL WHILE loop was aborted because it did not finish within the limit of 0.5 seconds. Use the 'limit' argument to increase or remove the limit if needed.
-    WHILE    $variable < 2    limit=0.5s
+    [Documentation]     FAIL WHILE loop was aborted because it did not finish within the limit of 0.1 seconds. Use the 'limit' argument to increase or remove the limit if needed.
+    WHILE    $variable < 2    limit=0.1s
         Log     ${variable}
     END
 
@@ -41,7 +41,7 @@ Limit from variable
     END
 
 Part of limit from variable
-    [Documentation]     FAIL WHILE loop was aborted because it did not finish within the limit of 0.7 seconds. Use the 'limit' argument to increase or remove the limit if needed.
+    [Documentation]     FAIL WHILE loop was aborted because it did not finish within the limit of 0.2 seconds. Use the 'limit' argument to increase or remove the limit if needed.
     WHILE    $variable < 2    limit=${number} s
         Log     ${variable}
     END
@@ -59,7 +59,7 @@ Invalid limit invalid suffix
     END
 
 Invalid limit invalid value
-    [Documentation]     FAIL Invalid WHILE loop limit: Iteration limit must be a positive integer, got: '-100'.
+    [Documentation]     FAIL Invalid WHILE loop limit: Iteration count must be a positive integer, got '-100'.
     WHILE    $variable < 2    limit=-100
         Log     ${variable}
     END

src/robot/running/bodyrunner.py

@@ -319,43 +319,45 @@ def __init__(self, context, run=True, templated=False):
         self._templated = templated
 
     def run(self, data):
-        run = self._run
-        executed_once = False
-        result = WhileResult(data.condition, data.limit)
-        with StatusReporter(data, result, self._context, run) as status:
-            if self._context.dry_run or not run:
+        ctx = self._context
+        error = None
+        run = False
+        limit = None
+        if self._run:
+            if data.error:
+                error = DataError(data.error, syntax=True)
+            elif not ctx.dry_run:
                 try:
-                    self._run_iteration(data, result, run)
-                except (BreakLoop, ContinueLoop):
-                    pass
+                    limit = WhileLimit.create(data.limit, ctx.variables)
+                    run = self._should_run(data.condition, ctx.variables)
+                except DataError as err:
+                    error = err
+        result = WhileResult(data.condition, data.limit)
+        with StatusReporter(data, result, self._context, run):
+            if ctx.dry_run or not run:
+                self._run_iteration(data, result, run)
+                if error:
+                    raise error
                 return
-            if data.error:
-                raise DataError(data.error, syntax=True)
-            limit = WhileLimit.create(data.limit, self._context.variables)
             errors = []
-            while self._should_run(data.condition, self._context.variables) \
-                    and limit.is_valid:
-                executed_once = True
+            while True:
                 try:
                     with limit:
-                        self._run_iteration(data, result, run)
+                        self._run_iteration(data, result)
                 except BreakLoop:
                     break
                 except ContinueLoop:
-                    continue
+                    pass
                 except ExecutionFailed as err:
                     errors.extend(err.get_errors())
-                    if not err.can_continue(self._context, self._templated):
+                    if not err.can_continue(ctx, self._templated):
                         break
-            if not executed_once:
-                status.pass_status = result.NOT_RUN
-                self._run_iteration(data, result, run=False)
+                if not self._should_run(data.condition, ctx.variables):
+                    break
             if errors:
                 raise ExecutionFailures(errors)
-            if not limit.is_valid:
-                raise DataError(limit.reason)
 
-    def _run_iteration(self, data, result, run):
+    def _run_iteration(self, data, result, run=True):
         runner = BodyRunner(self._context, run, self._templated)
         with StatusReporter(data, result.body.create_iteration(), self._context, run):
             runner.run(data.body)
@@ -570,26 +572,29 @@ def _run_finally(self, data, run):
 
 
 class WhileLimit:
-    is_valid = True
 
     @classmethod
     def create(cls, limit, variables):
+        if not limit:
+            return IterationCountLimit(DEFAULT_WHILE_LIMIT)
+        value = variables.replace_string(limit)
+        if value.upper() == 'NONE':
+            return NoLimit()
         try:
-            if not limit:
-                return IterationCountLimit(DEFAULT_WHILE_LIMIT)
-            if limit.upper() == 'NONE':
-                return NoLimit()
-            value = variables.replace_string(limit)
-            try:
-                count = int(value.replace(' ', ''))
-                if count <= 0:
-                    return InvalidLimit(f"Iteration limit must be a positive integer, "
-                                        f"got: '{count}'.")
-                return IterationCountLimit(count)
-            except ValueError:
-                return DurationLimit(timestr_to_secs(value))
-        except Exception as error:
-            return InvalidLimit(error)
+            count = int(value.replace(' ', ''))
+        except ValueError:
+            pass
+        else:
+            if count <= 0:
+                raise DataError(f"Invalid WHILE loop limit: Iteration count must be "
+                                f"a positive integer, got '{count}'.")
+            return IterationCountLimit(count)
+        try:
+            secs = timestr_to_secs(value)
+        except ValueError as err:
+            raise DataError(f'Invalid WHILE loop limit: {err.args[0]}')
+        else:
+            return DurationLimit(secs)
 
     def limit_exceeded(self):
         raise ExecutionFailed(f"WHILE loop was aborted because it did not finish "
@@ -638,13 +643,3 @@ class NoLimit(WhileLimit):
 
     def __enter__(self):
         pass
-
-
-class InvalidLimit(WhileLimit):
-    is_valid = False
-
-    def __init__(self, reason):
-        self.reason = f'Invalid WHILE loop limit: {reason}'
-
-    def __enter__(self):
-        raise DataError(self.reason)

src/robot/running/model.py

@@ -194,7 +194,8 @@ def run(self, context, run=True, templated=False):
             if run:
                 if self.error:
                     raise DataError(self.error, syntax=True)
-                raise ReturnFromKeyword(self.values)
+                if not context.dry_run:
+                    raise ReturnFromKeyword(self.values)
 
 
 @Body.register
@@ -212,10 +213,11 @@ def source(self):
 
     def run(self, context, run=True, templated=False):
         with StatusReporter(self, ContinueResult(), context, run):
-            if self.error:
-                raise DataError(self.error, syntax=True)
             if run:
-                raise ContinueLoop()
+                if self.error:
+                    raise DataError(self.error, syntax=True)
+                if not context.dry_run:
+                    raise ContinueLoop()
 
 
 @Body.register
@@ -233,10 +235,11 @@ def source(self):
 
     def run(self, context, run=True, templated=False):
         with StatusReporter(self, BreakResult(), context, run):
-            if self.error:
-                raise DataError(self.error, syntax=True)
             if run:
-                raise BreakLoop()
+                if self.error:
+                    raise DataError(self.error, syntax=True)
+                if not context.dry_run:
+                    raise BreakLoop()
 
 
 class TestCase(model.TestCase):