Repository navigation
implemented retry_type checks used in the TestClass - #267
mskarayigit wants to merge 2 commits into
Conversation
… fails test and moves after attempts. RT_ABORT, aborts the test after attemts.
|
The This approach would enable the generation of a comprehensive test report at the end of the execution, identifying all failed circuits without requiring operator intervention or waiting for user input. As a result, any necessary rework can be performed on all failed circuits in a single batch, improving efficiency and avoiding the need to repeatedly repair and retest individual circuits one at a time. In some situations, test failures may be caused by transient issues such as instrument switching delays or measurement timing variations. In these cases, one or more retry attempts may successfully recover from the issue and result in a passing test outcome. Conversely, certain test cases may be associated with critical circuits or dependencies that are essential for downstream testing. For these scenarios, it should be possible to configure a maximum number of retry attempts and, if the test continues to fail after all retries, terminate the test sequence. This prevents subsequent tests from being executed when their results would no longer be valid due to the failure of a critical prerequisite circuit. Therefore, the test framework should support both behaviors: Non-critical tests (
This flexibility would improve test efficiency, reduce operator involvement, and provide more meaningful test reports and failure analysis. |
jcollins1983
left a comment
There was a problem hiding this comment.
There are a couple of things to address. Perhaps yourself, Daniel and I can sit down when you're back at CSM to discuss the logic a bit more before going through another cycle on the PR?
| Enum class for the Retry Modes. | ||
| """ | ||
|
|
||
| RT_ABORT = 1 # Abort the whole test sequence |
| """ | ||
| if not isinstance(retry_type, TestClass.RetryMode): | ||
| raise TypeError( | ||
| f"Invalid retry_type:{retry_type} retry_type must be valid{type(retry_type).__name__}" |
There was a problem hiding this comment.
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.
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: |
There was a problem hiding this comment.
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.
- Where
RT_PROMPTshould prompt regardless of the number of attempts that has been set andRT_RETRYis the only setting where a test will automatically retry, and potentially only allow at mostattemptsretries. This would require a deeper change to the logic. - We remove one of them, I vote for
RT_RETRYbecause the (automatic) retry attempts is set by theattemptsvariable 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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
|
I think we don't strictly need this to achieve the behaviour we want. Using "amptest_non-interactive" as the programming config value should work. The number of attempts will be limited to 1, but I would argue that tests should be reliable to pass first try. |



retry_type variables in the TestClass now check within the retry_prompt in the Sequencer.py
Also, added a test script under test.