Tickets/DM-47619: Update implementation of the ignore feature in scripts. #256
Conversation
Use the new method `disable_checks_for_components` of the `RemoteGroup` class.
Also, refactor `attcs` attribute to `atcs`
MarcoRocchietti
force-pushed
the
tickets/DM-47619
branch
from
b8ef979 to
5392d21
Compare
- Removed the `ignore_components()` method as it is no longer needed. - Simplified the mechanism for the ignore feature, as ensuring that the checks are set up before calling `MTCS.start_task` is unnecessary.
… the ignore feature to the `configure` method
…yATCS In order to test the implementation of the ignore feature in the base scripts `EnableGroup`, `StandbyGroup` and `OfflineGroup`.
…fsetM2Hexapod, and CloseLoopLSSTCam
MarcoRocchietti
force-pushed
the
tickets/DM-47619
branch
from
5392d21 to
b54bdc8
Compare
There was a problem hiding this comment.
The changes in the scripts and most of the changes in the unit tests looks fine. However, some scripts were testing the ignore feature without any mocking, which I would expect to work with the updated version.
I made comments in the unit tests that I found, but I am not sure I caught all of them. Can you please, rollback the changes you've made on those? Unless there is really a reason for the changes, in which case, we should probably investigate what the problem is.
| @@ -40,24 +41,24 @@ async def basic_make_script(self, index): | |||
| self.script = DisableATAOSCorrections(index=index) | |||
| self.atcs_mock = ATCSMock() | |||
|
|
|||
| return (self.script, self.atcs_mock) | |||
| self.script.atcs = ATCS( | |||
There was a problem hiding this comment.
These unit tests were doing an actual test of the execution, see that they are using the ATCSMock above. Any reason for changing them? If not, I would rather leave them as they were.
| @@ -39,6 +40,14 @@ async def basic_make_script(self, index): | |||
| self.script = EnableATAOSCorrections(index=index) | |||
| self.atcs_mock = ATCSMock() | |||
|
|
|||
| self.script.atcs = ATCS( | |||
There was a problem hiding this comment.
Same as above, any reason for changing the unit tests?
| @@ -38,9 +38,19 @@ class TestEnableATTCS( | |||
| async def basic_make_script(self, index): | |||
| self.script = EnableATTCS(index=index) | |||
| self.atcs_mock = ATCSMock() | |||
| self.script._attcs.disable_checks_for_components = unittest.mock.Mock() | |||
There was a problem hiding this comment.
Same comment as above, any reason to change the unit test? they should work as is no?
| @@ -38,9 +38,19 @@ class TestStandbyATCS( | |||
| async def basic_make_script(self, index): | |||
| self.script = StandbyATCS(index=index) | |||
| self.atcs_mock = ATCSMock() | |||
| self.script._atcs.disable_checks_for_components = unittest.mock.Mock() | |||
There was a problem hiding this comment.
Same comment as above. This unit test is using ATCSMock any reason for changing it?
| @@ -47,18 +47,13 @@ async def test_executable(self): | |||
| await self.check_executable(script_path) | |||
|
|
|||
| async def test_configure_ignore(self): | |||
| async with self.make_script(): | |||
| async with self.make_dry_script(): | |||
Update scripts to use the
disable_checks_for_componentsmethod of theRemoteGroupclass.