set finished and cleanup state properly for operations (#3214)

* set finished and cleanup state properly for operations

* remove unnecessary lines

* include operation ID in cleanup log msg

* simplify condition

* add unit tests for cleanup status

* Update tox.ini

* Update quality.yml

Author: uruwhy

app/objects/c_operation.py

@@ -265,10 +265,7 @@ async def wait_for_links_completion(self, link_ids):
                     break
 
     async def is_closeable(self):
-        if await self.is_finished() or self.auto_close:
-            self.state = self.states['FINISHED']
-            return True
-        return False
+        return await self.is_finished() or self.auto_close
 
     async def is_finished(self):
         if self.state in [self.states['FINISHED'], self.states['OUT_OF_TIME'], self.states['CLEANUP']] \
@@ -427,7 +424,10 @@ async def _cleanup_operation(self, services):
                 self.add_link(link)
                 cleanup_count += 1
         if cleanup_count:
+            self.state = self.states['CLEANUP']
+            logging.debug(f'Starting cleanup for operation {self.id}')
             await self._safely_handle_cleanup(cleanup_count)
+            logging.debug(f'Completed cleanup for operation {self.id}')
 
     async def _safely_handle_cleanup(self, cleanup_link_count):
         try:

tests/objects/test_operation.py

@@ -10,6 +10,7 @@
 from app.objects.c_operation import Operation
 from app.objects.secondclass.c_link import Link
 from app.service.interfaces.i_event_svc import EventServiceInterface
+from app.service.interfaces.i_planning_svc import PlanningServiceInterface
 from app.utility.base_service import BaseService
 from app.objects.c_source import Source
 from app.objects.c_planner import Planner
@@ -139,16 +140,43 @@ async def fire_event(self, exchange=None, queue=None, timestamp=True, **callback
     BaseService.remove_service('event_svc')
 
 
+@pytest.fixture
+def fake_planning_svc(event_loop, make_test_link, test_agent):
+    class FakePlanningService(BaseService, PlanningServiceInterface):
+        def __init__(self):
+            self.fired = {}
+
+        async def get_cleanup_links(self, operation, agent):
+            cleanup_link = make_test_link(135, link_paw=test_agent.paw, link_cleanup=1, link_status=0)
+            return [cleanup_link]
+
+        def get_links(self, operation, buckets, agent, trim):
+            pass
+
+        def generate_and_trim_links(self, agent, operation, abilities, trim):
+            pass
+
+        def sort_links(self, links):
+            pass
+
+    service = FakePlanningService()
+    service.add_service('planning_svc', service)
+
+    yield service
+
+    BaseService.remove_service('planning_svc')
+
+
 @pytest.fixture
 def test_ability(ability, executor):
     return ability(ability_id='123', executors=[executor(name='psh', platform='windows')])
 
 
 @pytest.fixture
 def make_test_link(test_ability):
-    def _make_link(link_id, link_paw='123456', link_status=-3):
+    def _make_link(link_id, link_paw='123456', link_status=-3, link_cleanup=0):
         return Link(command='', paw=link_paw, ability=test_ability, id=link_id, executor=next(test_ability.executors),
-                    status=link_status)
+                    status=link_status, cleanup=link_cleanup)
     return _make_link
 
 
@@ -597,3 +625,11 @@ async def test_add_ignored_link(self, make_test_link, operation_agent):
         assert op.ignored_links
         assert test_link.id in op.ignored_links
         assert len(op.ignored_links) == 1
+
+    async def test_operation_cleanup_status(self, fake_planning_svc, operation_agent):
+        services = {'planning_svc': fake_planning_svc}
+        op = Operation(name='test with cleanup', agents=[operation_agent], state='running')
+        assert op.state == 'running'
+        assert await op.is_closeable()
+        await op._cleanup_operation(services)
+        assert op.state == 'cleanup'