ELM migrations: treat 'completed' equivalent to 'succeeded' for terminal status handling

Author: bhuvanshah7

azure-devops/azext_devops/dev/migration/migration.py

@@ -31,7 +31,12 @@
     'targetrepositorydoesnotexist': 256,
     'all': 2147483647,
 }
+_SUCCESS_TERMINAL_STATES = {
+    'succeeded',
+    'completed'
+}
 _NON_ACTIVE_STATES = {
+    'completed',
     'succeeded',
     'failed',
     'suspended'
@@ -184,14 +189,14 @@ def resume_migration(repository_id=None, validate_only=False, migration=False, o
                        .format(state_text))
 
     if _is_migration_terminal(migration_data):
-        status = _normalize_state(migration_data.get('status'))
+        status = _get_effective_status(migration_data)
         is_val_only = migration_data.get('validateOnly') is True
-        if status == 'succeeded' and is_val_only:
-            raise CLIError('Validation already succeeded. Promote it with '
+        if _is_success_terminal_status(status) and is_val_only:
+            raise CLIError('Validation already completed. Promote it with '
                            '"az devops migrations resume --repository-id <guid> --migration", '
                            'or abandon and create a new migration.')
-        if status == 'succeeded':
-            raise CLIError('Migration already succeeded. Use '
+        if _is_success_terminal_status(status):
+            raise CLIError('Migration already completed. Use '
                            '"az devops migrations abandon --repository-id <guid>" to reset, '
                            'then create a new migration.')
 
@@ -257,6 +262,20 @@ def _normalize_state(value):
     return normalized.replace(' ', '').replace('-', '').replace('_', '')
 
 
+def _is_success_terminal_status(status):
+    return status in _SUCCESS_TERMINAL_STATES
+
+
+def _get_effective_status(migration):
+    if not isinstance(migration, dict):
+        return ''
+    # Prefer actual migration status over requested status when both are present.
+    status = _normalize_state(migration.get('status'))
+    if status:
+        return status
+    return _normalize_state(migration.get('statusRequested'))
+
+
 def _get_migration_state_text(migration):
     status_requested = migration.get('statusRequested')
     status = migration.get('status')
@@ -277,7 +296,7 @@ def _is_migration_active(migration):
     if not isinstance(migration, dict):
         return False
 
-    status = _normalize_state(migration.get('statusRequested') or migration.get('status'))
+    status = _get_effective_status(migration)
     if status:
         return status not in _NON_ACTIVE_STATES
 
@@ -291,15 +310,15 @@ def _is_migration_active(migration):
 def _is_migration_terminal(migration):
     if not isinstance(migration, dict):
         return False
-    status = _normalize_state(migration.get('status'))
-    return status in ('succeeded', 'failed')
+    status = _get_effective_status(migration)
+    return _is_success_terminal_status(status) or status == 'failed'
 
 
 def _is_validate_only_succeeded(migration):
     if not isinstance(migration, dict):
         return False
     return (migration.get('validateOnly') is True and
-            _normalize_state(migration.get('status')) == 'succeeded')
+            _is_success_terminal_status(_get_effective_status(migration)))
 
 
 def _promote_to_full_migration(migration_data, repository_id, organization):

azure-devops/azext_devops/tests/latest/migration/test_migration.py

@@ -479,6 +479,28 @@ def test_resume_migration_promotes_validate_only_succeeded(self):
             self.assertFalse(payload['validateOnly'])
             self.assertEqual(payload['statusRequested'], 'active')
 
+    def test_resume_migration_promotes_validate_only_completed(self):
+        with patch('azext_devops.dev.migration.migration.get_migration') as mock_get, \
+             patch('azext_devops.dev.migration.migration.resolve_instance') as mock_resolve, \
+             patch('azext_devops.dev.migration.migration._get_service_client') as mock_client, \
+             patch('azext_devops.dev.migration.migration._send_request') as mock_send:
+            mock_send.return_value = {}
+            mock_get.return_value = {
+                'status': 'completed',
+                'validateOnly': True,
+            }
+            mock_resolve.return_value = self._TEST_ORG
+
+            resume_migration(repository_id='00000000-0000-0000-0000-000000000000',
+                             migration=True,
+                             organization=self._TEST_ORG, detect=False)
+
+            args = mock_send.call_args[0]
+            self.assertEqual(args[1], 'PUT')
+            payload = args[3]
+            self.assertFalse(payload['validateOnly'])
+            self.assertEqual(payload['statusRequested'], 'active')
+
     def test_resume_migration_promote_uses_only_state_transition_fields(self):
         with patch('azext_devops.dev.migration.migration.get_migration') as mock_get, \
              patch('azext_devops.dev.migration.migration.resolve_instance') as mock_resolve, \
@@ -521,6 +543,17 @@ def test_resume_succeeded_without_migration_flag_errors(self):
                                  organization=self._TEST_ORG, detect=False)
             self.assertIn('--migration', str(ctx.exception))
 
+    def test_resume_completed_without_migration_flag_errors(self):
+        with patch('azext_devops.dev.migration.migration.get_migration') as mock_get, \
+             patch('azext_devops.dev.migration.migration.resolve_instance') as mock_resolve:
+            mock_get.return_value = {'status': 'completed', 'validateOnly': True}
+            mock_resolve.return_value = self._TEST_ORG
+
+            with self.assertRaises(CLIError) as ctx:
+                resume_migration(repository_id='00000000-0000-0000-0000-000000000000',
+                                 organization=self._TEST_ORG, detect=False)
+            self.assertIn('--migration', str(ctx.exception))
+
     def test_resume_succeeded_full_migration_errors(self):
         with patch('azext_devops.dev.migration.migration.get_migration') as mock_get, \
              patch('azext_devops.dev.migration.migration.resolve_instance') as mock_resolve:
@@ -532,6 +565,54 @@ def test_resume_succeeded_full_migration_errors(self):
                                  organization=self._TEST_ORG, detect=False)
             self.assertIn('abandon', str(ctx.exception))
 
+    def test_resume_completed_full_migration_errors(self):
+        with patch('azext_devops.dev.migration.migration.get_migration') as mock_get, \
+             patch('azext_devops.dev.migration.migration.resolve_instance') as mock_resolve:
+            mock_get.return_value = {'status': 'completed', 'validateOnly': False}
+            mock_resolve.return_value = self._TEST_ORG
+
+            with self.assertRaises(CLIError) as ctx:
+                resume_migration(repository_id='00000000-0000-0000-0000-000000000000',
+                                 organization=self._TEST_ORG, detect=False)
+            self.assertIn('abandon', str(ctx.exception))
+
+    def test_resume_completed_status_takes_precedence_over_active_status_requested(self):
+        with patch('azext_devops.dev.migration.migration.get_migration') as mock_get, \
+             patch('azext_devops.dev.migration.migration.resolve_instance') as mock_resolve:
+            mock_get.return_value = {
+                'status': 'completed',
+                'statusRequested': 'active',
+                'validateOnly': True,
+            }
+            mock_resolve.return_value = self._TEST_ORG
+
+            with self.assertRaises(CLIError) as ctx:
+                resume_migration(repository_id='00000000-0000-0000-0000-000000000000',
+                                 organization=self._TEST_ORG, detect=False)
+            self.assertIn('--migration', str(ctx.exception))
+
+    def test_resume_completed_status_requested_without_status_is_terminal(self):
+        with patch('azext_devops.dev.migration.migration.get_migration') as mock_get, \
+             patch('azext_devops.dev.migration.migration.resolve_instance') as mock_resolve:
+            mock_get.return_value = {'statusRequested': 'completed', 'validateOnly': False}
+            mock_resolve.return_value = self._TEST_ORG
+
+            with self.assertRaises(CLIError) as ctx:
+                resume_migration(repository_id='00000000-0000-0000-0000-000000000000',
+                                 organization=self._TEST_ORG, detect=False)
+            self.assertIn('abandon', str(ctx.exception))
+
+    def test_resume_completed_case_variants_are_treated_as_terminal(self):
+        with patch('azext_devops.dev.migration.migration.get_migration') as mock_get, \
+             patch('azext_devops.dev.migration.migration.resolve_instance') as mock_resolve:
+            mock_get.return_value = {'status': 'Com_PleTed', 'validateOnly': False}
+            mock_resolve.return_value = self._TEST_ORG
+
+            with self.assertRaises(CLIError) as ctx:
+                resume_migration(repository_id='00000000-0000-0000-0000-000000000000',
+                                 organization=self._TEST_ORG, detect=False)
+            self.assertIn('abandon', str(ctx.exception))
+
 
 if __name__ == '__main__':
     unittest.main()