Improve code coverage by adding comprehensive tests for PR #150 functionality

Copilot · alvarolopez · alvarolopez · commit 7cf341217692 · 2025-09-26T13:48:06.000+02:00
Co-authored-by: alvarolopez &lt;468751+alvarolopez@users.noreply.github.com&gt;
diff --git a/caso/tests/base.py b/caso/tests/base.py
@@ -31,7 +31,7 @@
 _TRUE_VALUES = ("True", "true", "1", "yes")
 
 
-class TestCase(unittest.TestCase):
+class TestCase(fixtures.TestWithFixtures, unittest.TestCase):
     """Test case base class for all unit tests."""
 
     REQUIRES_LOCKING = False
diff --git a/caso/tests/extract/test_manager.py b/caso/tests/extract/test_manager.py
@@ -25,6 +25,7 @@
 from unittest import mock
 
 from caso.extract import manager
+import caso.manager  # Import to register the spooldir option
 
 CONF = cfg.CONF
 
@@ -36,6 +37,7 @@ def setUp(self):
         """Run before each test method to initialize test environment."""
         super(TestCasoManager, self).setUp()
         self.flags(extractor="mock")
+        self.flags(spooldir="/tmp/caso_test")
         self.p_extractors = mock.patch("caso.loading.get_available_extractors")
         patched = self.p_extractors.start()
         self.records = [{uuid.uuid4().hex: None}]
@@ -147,6 +149,86 @@ def test_lastrun_is_invalid(self):
                 path.return_value = True
                 self.assertRaises(ValueError, self.manager.get_lastrun, "foo")
 
+    def test_get_records_with_invalid_lastrun_continues(self):
+        """Test that get_records continues when lastrun file is invalid."""
+        self.flags(projects=["project1", "project2"])
+        
+        # Mock the file system operations to test exception handling path
+        with unittest.mock.patch.object(self.manager, "get_project_vo", return_value="test-vo") as m_vo:
+            with unittest.mock.patch('os.path.exists', return_value=True):
+                # One project gets invalid date, one gets valid date
+                file_contents = ["invalid-date-format", "2020-01-01 00:00:00"]
+                mock_file = unittest.mock.mock_open()
+                mock_file.return_value.read.side_effect = file_contents
+                
+                with unittest.mock.patch('builtins.open', mock_file):
+                    ret = self.manager.get_records()
+                    
+                    # Should call get_project_vo for both projects since VO lookup happens before lastrun
+                    self.assertEqual(m_vo.call_count, 2)
+                    m_vo.assert_any_call("project1")
+                    m_vo.assert_any_call("project2")
+                    
+                    # One project should succeed - doesn't matter which one for this test
+                    # The key is that get_records continues processing despite one project failing
+                    self.m_extractor.assert_called_once()
+                    args, kwargs = self.m_extractor.call_args
+                    self.assertEqual(args[1], "test-vo")  # VO should be correct
+                    self.assertIn(args[0], ["project1", "project2"])  # Should be one of the projects
+
+    def test_get_records_with_future_extract_from_continues(self):
+        """Test that get_records continues when extract_from is in the future."""
+        self.flags(projects=["project1", "project2"])
+        future_date = datetime.datetime.now(tz.tzutc()) + datetime.timedelta(days=1)
+        past_date = datetime.datetime.now(tz.tzutc()) - datetime.timedelta(days=1)
+        
+        # Mock file system operations and return dates
+        with unittest.mock.patch.object(self.manager, "get_project_vo", return_value="test-vo") as m_vo:
+            with unittest.mock.patch('os.path.exists', return_value=True):
+                # One project gets future date, one gets past date
+                file_contents = [str(future_date), str(past_date)]
+                mock_file = unittest.mock.mock_open()
+                mock_file.return_value.read.side_effect = file_contents
+                
+                with unittest.mock.patch('builtins.open', mock_file):
+                    ret = self.manager.get_records()
+                    
+                    # Should call get_project_vo for both projects since VO lookup happens before lastrun
+                    self.assertEqual(m_vo.call_count, 2)
+                    m_vo.assert_any_call("project1")
+                    m_vo.assert_any_call("project2")
+                    
+                    # One project should succeed - doesn't matter which one for this test
+                    # The key is that get_records continues processing despite one project failing
+                    self.m_extractor.assert_called_once()
+                    args, kwargs = self.m_extractor.call_args
+                    self.assertEqual(args[1], "test-vo")  # VO should be correct
+                    self.assertIn(args[0], ["project1", "project2"])  # Should be one of the projects
+
+    def test_get_lastrun_invalid_date_logs_exception(self):
+        """Test that get_lastrun properly logs exceptions when date parsing fails."""
+        if six.PY3:
+            builtins_open = "builtins.open"
+        else:
+            builtins_open = "__builtin__.open"
+        fopen = unittest.mock.mock_open(read_data="invalid-date-format")
+        
+        with unittest.mock.patch("os.path.exists") as path:
+            with unittest.mock.patch(builtins_open, fopen):
+                with unittest.mock.patch("caso.extract.manager.LOG") as mock_log:
+                    path.return_value = True
+                    
+                    with self.assertRaises(ValueError):
+                        self.manager.get_lastrun("test-project")
+                    
+                    # Verify that both error and exception were logged
+                    mock_log.error.assert_called_once()
+                    mock_log.exception.assert_called_once()
+                    
+                    # Check the error message contains the expected text
+                    error_call = mock_log.error.call_args[0][0]
+                    self.assertIn("Cannot read date from lastrun file", error_call)
+
     def test_write_lastrun_dry_run(self):
         """Test that we do not write lastrun file on dry run."""
         self.flags(dry_run=True)
@@ -166,7 +248,7 @@ def test_write_lastrun(self):
 
         with unittest.mock.patch(builtins_open, unittest.mock.mock_open()) as m:
             self.manager.get_records()
-            m.assert_called_once_with("/var/spool/caso/lastrun.bazonk", "w")
+            m.assert_called_once_with("/tmp/caso_test/lastrun.bazonk", "w")
 
     def flags(self, **kw):
         """Override flag variables for a test."""
diff --git a/caso/tests/test_cmd_projects.py b/caso/tests/test_cmd_projects.py
@@ -0,0 +1,214 @@
+# -*- coding: utf-8 -*-
+
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+"""Tests for `caso._cmd.projects` module."""
+
+import sys
+import unittest
+from unittest import mock
+
+from oslo_config import cfg
+
+from caso._cmd import projects
+from caso.tests import base
+
+CONF = cfg.CONF
+
+
+class TestProjectsCommand(base.TestCase):
+    """Test case for the projects command module."""
+
+    def setUp(self):
+        """Set up test fixtures."""
+        super(TestProjectsCommand, self).setUp()
+
+    @mock.patch('caso.config.parse_args')
+    @mock.patch('oslo_log.log.setup')
+    @mock.patch('caso.manager.Manager')
+    def test_main_success(self, mock_manager_cls, mock_log_setup, mock_parse_args):
+        """Test main function with successful project retrieval."""
+        # Setup mocks
+        mock_manager = mock.MagicMock()
+        mock_manager_cls.return_value = mock_manager
+        mock_manager.projects_and_vos.return_value = [('project1', 'vo1'), ('project2', 'vo2')]
+        
+        # Mock keystone project retrieval
+        mock_project = mock.MagicMock()
+        mock_project.name = 'Project One'
+        mock_manager.extractor_manager.keystone.projects.get.return_value = mock_project
+        
+        with mock.patch('builtins.print') as mock_print:
+            projects.main()
+            
+            # Verify setup calls
+            mock_parse_args.assert_called_once_with(sys.argv)
+            mock_log_setup.assert_called_once_with(cfg.CONF, "caso")
+            mock_manager_cls.assert_called_once()
+            
+            # Verify manager methods called
+            mock_manager.projects_and_vos.assert_called_once()
+            
+            # Verify print was called with expected output format
+            self.assertTrue(mock_print.called)
+            print_calls = [call[0][0] for call in mock_print.call_args_list]
+            self.assertIn("'project1 (Project One) mapped to VO 'vo1'", print_calls)
+
+    @mock.patch('caso.config.parse_args')
+    @mock.patch('oslo_log.log.setup')
+    @mock.patch('caso.manager.Manager')
+    def test_main_with_keystone_error(self, mock_manager_cls, mock_log_setup, mock_parse_args):
+        """Test main function when keystone project retrieval fails."""
+        # Setup mocks
+        mock_manager = mock.MagicMock()
+        mock_manager_cls.return_value = mock_manager
+        mock_manager.projects_and_vos.return_value = [('project1', 'vo1')]
+        
+        # Mock keystone to raise an exception
+        mock_manager.extractor_manager.keystone.projects.get.side_effect = Exception("Keystone error")
+        
+        with mock.patch('builtins.print') as mock_print:
+            projects.main()
+            
+            # Verify error messages were printed
+            print_calls = [call[0][0] for call in mock_print.call_args_list]
+            error_messages = [call for call in print_calls if call.startswith("ERROR:")]
+            self.assertTrue(len(error_messages) >= 2)  # Should have at least 2 error messages
+            self.assertTrue(any("Could not get project project1" in msg for msg in error_messages))
+
+    @mock.patch('caso.config.parse_args')
+    @mock.patch('oslo_log.log.setup')
+    @mock.patch('caso.manager.Manager')
+    def test_migrate_dry_run_mode(self, mock_manager_cls, mock_log_setup, mock_parse_args):
+        """Test migrate function in dry run mode."""
+        # Setup mocks
+        mock_manager = mock.MagicMock()
+        mock_manager_cls.return_value = mock_manager
+        mock_manager.extractor_manager.voms_map.items.return_value = [('project1', 'vo1')]
+        
+        # Set dry_run flag
+        self.flags(dry_run=True, vo_property='caso_vo')
+        with mock.patch('builtins.print') as mock_print:
+            projects.migrate()
+            
+            # Verify setup calls
+            mock_parse_args.assert_called_once_with(sys.argv)
+            mock_log_setup.assert_called_once_with(cfg.CONF, "caso")
+            mock_manager._load_managers.assert_called_once()
+            
+            # Verify print statements for dry run
+            print_calls = [call[0][0] for call in mock_print.call_args_list]
+            warning_printed = any("WARNING: Running in 'dry-run' mode" in call for call in print_calls)
+            self.assertTrue(warning_printed)
+            
+            # Should print the openstack command but not execute it
+            openstack_cmd_printed = any("openstack project set --property" in call for call in print_calls)
+            self.assertTrue(openstack_cmd_printed)
+
+    @mock.patch('caso.config.parse_args')
+    @mock.patch('oslo_log.log.setup')
+    @mock.patch('caso.manager.Manager')
+    def test_migrate_with_projects_dry_run(self, mock_manager_cls, mock_log_setup, mock_parse_args):
+        """Test migrate function with project migration in dry run mode."""
+        # Setup mocks
+        mock_manager = mock.MagicMock()
+        mock_manager_cls.return_value = mock_manager
+        mock_manager.extractor_manager.voms_map.items.return_value = []
+        
+        # Set flags for project migration
+        self.flags(dry_run=True, migrate_projects=True, projects=['project1'], caso_tag='caso')
+        with mock.patch('builtins.print') as mock_print:
+            projects.migrate()
+            
+            # Verify print statements for project tagging
+            print_calls = [call[0][0] for call in mock_print.call_args_list]
+            tag_cmd_printed = any("openstack project set --tag caso project1" in call for call in print_calls)
+            self.assertTrue(tag_cmd_printed)
+
+    @mock.patch('caso.config.parse_args')
+    @mock.patch('oslo_log.log.setup')
+    @mock.patch('caso.manager.Manager')
+    def test_migrate_actual_execution(self, mock_manager_cls, mock_log_setup, mock_parse_args):
+        """Test migrate function with actual execution (not dry run)."""
+        # Setup mocks
+        mock_manager = mock.MagicMock()
+        mock_manager_cls.return_value = mock_manager
+        mock_manager.extractor_manager.voms_map.items.return_value = [('project1', 'vo1')]
+        
+        # Mock successful keystone update
+        mock_manager.extractor_manager.keystone.projects.update.return_value = None
+        
+        # Set flags for actual execution
+        self.flags(dry_run=False, vo_property='caso_vo')
+        with mock.patch('builtins.print') as mock_print:
+            projects.migrate()
+            
+            # Verify keystone update was called
+            mock_manager.extractor_manager.keystone.projects.update.assert_called_once_with(
+                'project1', caso_vo='vo1'
+            )
+            
+            # Should not print warning about dry run
+            print_calls = [call[0][0] for call in mock_print.call_args_list]
+            warning_printed = any("WARNING: Running in 'dry-run' mode" in call for call in print_calls)
+            self.assertFalse(warning_printed)
+
+    @mock.patch('caso.config.parse_args')
+    @mock.patch('oslo_log.log.setup')
+    @mock.patch('caso.manager.Manager')
+    def test_migrate_keystone_update_error(self, mock_manager_cls, mock_log_setup, mock_parse_args):
+        """Test migrate function when keystone update fails."""
+        # Setup mocks
+        mock_manager = mock.MagicMock()
+        mock_manager_cls.return_value = mock_manager
+        mock_manager.extractor_manager.voms_map.items.return_value = [('project1', 'vo1')]
+        
+        # Mock keystone update to raise an exception
+        mock_manager.extractor_manager.keystone.projects.update.side_effect = Exception("Update failed")
+        
+        # Set flags for actual execution
+        self.flags(dry_run=False, vo_property='caso_vo')
+        with mock.patch('builtins.print') as mock_print:
+            projects.migrate()
+            
+            # Verify error messages were printed
+            print_calls = [call[0][0] for call in mock_print.call_args_list]
+            error_messages = [call for call in print_calls if call.startswith("ERROR:")]
+            self.assertTrue(len(error_messages) >= 2)  # Should have at least 2 error messages
+            self.assertTrue(any("could not add property for project project1" in msg for msg in error_messages))
+
+    @mock.patch('caso.config.parse_args')
+    @mock.patch('oslo_log.log.setup')
+    @mock.patch('caso.manager.Manager')
+    def test_migrate_project_tagging_error(self, mock_manager_cls, mock_log_setup, mock_parse_args):
+        """Test migrate function when project tagging fails."""
+        # Setup mocks
+        mock_manager = mock.MagicMock()
+        mock_manager_cls.return_value = mock_manager
+        mock_manager.extractor_manager.voms_map.items.return_value = []
+        
+        # Mock project get and tag operations
+        mock_project = mock.MagicMock()
+        mock_project.add_tag.side_effect = Exception("Tag failed")
+        mock_manager.extractor_manager.keystone.projects.get.return_value = mock_project
+        
+        # Set flags for project migration
+        self.flags(dry_run=False, migrate_projects=True, projects=['project1'], caso_tag='caso')
+        with mock.patch('builtins.print') as mock_print:
+            projects.migrate()
+            
+            # Verify error messages were printed
+            print_calls = [call[0][0] for call in mock_print.call_args_list]
+            error_messages = [call for call in print_calls if call.startswith("ERROR:")]
+            self.assertTrue(len(error_messages) >= 2)  # Should have at least 2 error messages
+            self.assertTrue(any("could not add tag for project project1" in msg for msg in error_messages))
