Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,5 @@ venv37
.vscode/settings.json
docs/_build/
.DS_Store
.env
.env
*.lock
17 changes: 12 additions & 5 deletions src/fixate/core/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import ctypes
import logging
import warnings
from enum import Enum, unique
from functools import wraps
from collections import namedtuple
from fixate.core.exceptions import ParameterError, InvalidScalarQuantityError
Expand Down Expand Up @@ -407,16 +408,22 @@ class TestClass:
as the name of the test with the remaining lines stored as self.test_desc_long which will show in the test logs
"""

RT_ABORT = 1 # Abort the whole test sequence
RT_RETRY = 2 # Automatically retry up to "attempts"
RT_PROMPT = 3 # Prompt the user; Options are Abort the sequence, retry, or fail and continue
RT_FAIL = 4 # Automatically fail and move on
@unique
class RetryMode(Enum):
"""
Enum class for the Retry Modes.
"""

RT_ABORT = 1 # Abort the whole test sequence

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you move these comments into the doc string? That way instead of

image.png

We get
image.png

RT_RETRY = 2 # Automatically retry up to "attempts"
RT_PROMPT = 3 # Prompt the user; Options are Abort the sequence, retry, or fail and continue
RT_FAIL = 4 # Automatically fail and move on

test_desc = None
test_desc_long = None
attempts = 1
tests = []
retry_type = RT_PROMPT
retry_type: RetryMode = RetryMode.RT_PROMPT
retry_exceptions = [BaseException] # Depreciated
skip_exceptions = []
abort_exceptions = [KeyboardInterrupt, AttributeError, NameError]
Expand Down
25 changes: 22 additions & 3 deletions src/fixate/sequencer.py
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,9 @@ def run_once(self):
if self.run_test():
top.index += 1
else:
if not self.retry_prompt():
if not self.retry_prompt(
retry_type=top.current().retry_type
):
# mark the test as failed and continue. else will loop and try again
self.tests_failed += 1
top.index += 1
Expand Down Expand Up @@ -371,7 +373,7 @@ def run_test(self):
)
attempts = 0
active_test_status = "ERROR"
if not self.retry_prompt():
if not self.retry_prompt(retry_type=active_test.retry_type):
self.tests_errored += 1
break
# Retry logic for exceptions
Expand All @@ -395,15 +397,32 @@ def run_test(self):
)
return active_test_status == "PASS"

def retry_prompt(self):
def retry_prompt(self, *, retry_type: TestClass.RetryMode):
"""Prompt the user when something goes wrong.

For retry return True, to fail return False and to abort raise and abort exception. Respect the
non_interactive flag, which can be set by the command line option --non-interactive
"""
if not isinstance(retry_type, TestClass.RetryMode):
raise TypeError(
f"Invalid retry_type:{retry_type} retry_type must be valid{type(retry_type).__name__}"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error message is not really all that helpful.
If the person writing the tests ignores the hints that the IDE will be giving them because of the type hinting, and they pass an int, they will be told that what they passed in must be a valid int.

image.png

I think something like the below will be clearer.

            raise TypeError(
                f"Invalid retry_type type: {type(retry_type).__name__}. retry_type must be a valid {TestClass.RetryMode.__name__} member"
            )

)

if self.non_interactive:
return False
if retry_type == TestClass.RetryMode.RT_PROMPT:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These 2 cases are currently identical. If attempts in the test class is > 1, the test will be attempted attempts times regardless of whether we have set RT_PROMPT or RT_RETRY.

I think we have 2 options.

  1. Where RT_PROMPT should prompt regardless of the number of attempts that has been set and RT_RETRY is the only setting where a test will automatically retry, and potentially only allow at most attempts retries. This would require a deeper change to the logic.
  2. We remove one of them, I vote for RT_RETRY because the (automatic) retry attempts is set by the attempts variable for the test class, and either way we will end up with a prompt for further retries or moving on.

My personal vote is for option 2.

@daniel-montanari thoughts?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the logic should be moved out a level.
Have retry_prompt be the thing that purely handles the prompt. If we have a prompt AND we are non-interactive then the default behaviour is to fail.

Then outside of this function implement the logic for the other RT types and switch off either the prompt or non-interactive RT type

RT_RETRY should return True (though I am not a fan of auto retrying tests, it is probably still a valid thing to do)
RT_FAIL should return False
RT_ABORT should raise SequenceAbort

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah having another look at this the whole loop is broken.
If you use RT_RETRY as I want it to behave above then this would cause an infinite loop that ignores the number of attempts.

I don't understand why abort_exceptions allows retries. self.ABORT inside of run_test also doesn't seem to get used as it only gets set after the test has exited.

I think the attempts and retry logic should be moved out of run_test so the only place to deal with it is in run_once. Also those names seem backwards to me.

# RT_PROMPT is the default behaviour and will prompt the user below.
pass
if retry_type == TestClass.RetryMode.RT_RETRY:
# RT_RETRY behaviour is determined by active_test.attempt,
# By default it is set to 1. For example if the value is set to 3 it will
# retry the test 3 times if all fails then it prompts the user.
pass
if retry_type == TestClass.RetryMode.RT_FAIL:
return False
if retry_type == TestClass.RetryMode.RT_ABORT:
raise SequenceAbort("Sequence Aborted By The Test")

resp = user_retry_abort_fail(msg="")
if resp == "ABORT":
raise SequenceAbort("Sequence Aborted By User")
Expand Down
175 changes: 175 additions & 0 deletions test/test_retry_type.py

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You get bonus points for writing tests without being prompted to. But these are manual tests, so should be in the manual folder.
Having said that though, I think that can be incorporated into the sequencer tests test/core/test_sequencer.py and they can all be automated.

Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
"""
Testing the RetryMode implementation.
"""

from fixate.core.checks import chk_true
from fixate.core.common import TestClass, TestList
from fixate.core.ui import user_info, user_ok


class SimpleTestToPass(TestClass):
"""Simple test case that always passes"""

test_desc = __qualname__

def set_up(self):
user_info("Setting up to pass")

def test(self):
user_info("Testing to Pass the test")
chk_true(True, "Passed the test")

def tear_down(self):
user_info("Well done, you wouldn't fail anyway!")


class SimpleTestToFail(TestClass):
"""Simple test case that always fails"""

test_desc = __qualname__
attempts = 2
retry_type = TestClass.RetryMode.RT_PROMPT

def set_up(self):
user_info("Setting up to fail")

def test(self):
user_info("Testing to Fail the test")
chk_true(False, "Failed the test")

def tear_down(self):
user_info("Well, you wouldn't pass anyway!")


class RetryTestPass(TestClass):
"""Simple test case that always passes but setup to retry if fails to test passing cases."""

test_desc = __qualname__
retry_type = TestClass.RetryMode.RT_RETRY
attempts = 3

def set_up(self):
user_info("Setting up to pass")

def test(self):
user_info("Testing to Pass the test")
chk_true(True, "Passed the test")

def tear_down(self):
user_info("Well done, you wouldn't fail anyway!")


class RetryTestFail(TestClass):
"""Simple test case that always fails and retries for the number of
attempts and prompts the user after all the attempts have failed..
"""

test_desc = __qualname__

retry_type = TestClass.RetryMode.RT_RETRY
attempts = 3

def set_up(self):
user_info("Setting up to fail")

def test(self):
user_info("Testing to Fail the test")
chk_true(False, "Failed the test")

def tear_down(self):
user_info("Well, you wouldn't pass anyway!")


# RT_FAIL = 4 # Automatically fail and move on
class RetryTestPassRTFAIL(TestClass):
"""Simple test case that always passes but setup to retry if fails to test passing cases."""

test_desc = __qualname__

retry_type = TestClass.RetryMode.RT_FAIL
attempts = 3

def set_up(self):
user_info("Setting up to pass")

def test(self):
user_info("Testing to Pass the test")
chk_true(True, "Passed the test")

def tear_down(self):
user_info("Well done, you wouldn't fail anyway!")


class RetryTestFailRTFAIL(TestClass):
"""Simple test case that always fails and retries for the number of
attempts. Then automatically failes the test and moves onto the next one."""

test_desc = __qualname__

retry_type = TestClass.RetryMode.RT_FAIL
attempts = 3

def set_up(self):
user_info("Setting up to fail")

def test(self):
user_info("Testing to Fail the test")
chk_true(False, "Failed the test")

def tear_down(self):
user_info("Well, you wouldn't pass anyway!")


class TestListRTFAIL(TestList):
"""Test list to test the RT_FAIL within the test list"""

def enter(self):
user_info("Entering the test list")

def exit(self):
user_info("Leaving the test list")


# RT_ABORT = 1 # Abort the whole test sequence
class RetryTestFailRTABORT(TestClass):
"""Simple test case that always fails and retries the number of attempts when final attempt has failed
It aborts the all the test. It may be useful if there is critical test that may damage Device Under Test if fails.
"""

test_desc = __qualname__

retry_type = TestClass.RetryMode.RT_ABORT
attempts = 3

def set_up(self):
user_ok("Setting up to abort, Press Enter to proceed")

def test(self):
user_info("Testing to Fail the test")
chk_true(False, "Failed the test")

def tear_down(self):
user_info("ABORTING!")


TEST_SEQUENCE = [
SimpleTestToPass(),
SimpleTestToFail(),
RetryTestPass(),
RetryTestFail(),
RetryTestPassRTFAIL(),
RetryTestFailRTFAIL(),
TestListRTFAIL(
[
RetryTestPassRTFAIL(),
RetryTestFailRTFAIL(),
]
),
RetryTestFailRTABORT(),
]

if __name__ == "__main__":
import fixate

argv = ["-p", __file__, "--serial_number", "1234567890"]
fixate.run(__file__, argv)
Loading