Conditional skip of stopping resources in interactive mode #1018
Conversation
| @@ -709,6 +709,8 @@ def stop_test_resources(self): | |||
| interactive mode where a test environment may be stopped on-demand. | |||
| Handles running before/after_stop callables if required. | |||
| """ | |||
| if self.report.env_status == entity.ResourceStatus.STOPPED: | |||
| return | |||
There was a problem hiding this comment.
Abort explicitly calls stopping of test resources, we need the skip here.
| @@ -513,6 +513,9 @@ def stop_test_resources(self, test_uid, await_results=True): | |||
| if not await_results: | |||
| return self._run_async(self.stop_test_resources, test_uid) | |||
|
|
|||
| if self.report[test_uid].env_status == entity.ResourceStatus.STOPPED: | |||
| return | |||
There was a problem hiding this comment.
This is a performance boost in general, if something is stopped we simply skip the entire process. Not sure if there is a way to trigger another stop of the environment resources than usual (in which case the button status protects us), but just to be sure.
There was a problem hiding this comment.
It's better that we check env_status not in [STOPPING, STOPPED]
| @@ -513,6 +513,9 @@ def stop_test_resources(self, test_uid, await_results=True): | |||
| if not await_results: | |||
| return self._run_async(self.stop_test_resources, test_uid) | |||
|
|
|||
| if self.report[test_uid].env_status == entity.ResourceStatus.STOPPED: | |||
| return | |||
There was a problem hiding this comment.
It's better that we check env_status not in [STOPPING, STOPPED]
testplan/testing/multitest/base.py
Outdated
| @@ -709,6 +709,8 @@ def stop_test_resources(self): | |||
| interactive mode where a test environment may be stopped on-demand. | |||
| Handles running before/after_stop callables if required. | |||
| """ | |||
| if self.report.env_status == entity.ResourceStatus.STOPPED: | |||
There was a problem hiding this comment.
it seems report.env_status is not something that we handle at Test or Multitest level. It is currently handled by TestRunnerIHandler. Or maybe we can move this state transition logic to Test. Anyway, it's better we set and check in the same class.
There was a problem hiding this comment.
For now I addressed the other two comments. I think the Environment itself should know about the overall status of its drivers and derive from that. It does not seem to me a Test or MultiTest level thing, only if it derives from the actual environment.
There was a problem hiding this comment.
Also, there are now 5 failures across various testcases all relying on the fact that STOPPING is also considered as a skip condition. Let us discuss offline.
Bug / Requirement Description
If an environment is started then stopped in interactive mode, then the abort would once again initiate the full stop for the environment, resulting in duplicated work and a UID clash for before and after stop hooks.
Solution description
Added a simple conditional skip of this step based on environment status.
Checklist: