Merge pull request #102 from Novartis/reenable-codecov

reenable-codecov

.github/workflows/pr-checks.yaml

@@ -40,6 +40,7 @@ jobs:
           eval "$(conda shell.bash hook)"
           conda activate cellxgene-gateway
           python setup.py install
+          pip install -r requirements-test.txt
 
       - name: Run tests
         run: |
@@ -76,15 +77,15 @@ jobs:
           name: coverage-summary
           path: coverage.txt
           retention-days: 30
-      # - name: "Upload coverage to Codecov"
-      #   if: ${{ github.event_name == 'push' || (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository) }}
-      #   uses: codecov/codecov-action@v1
-      #   with:
-      #     token: ${{ secrets.CODECOV_TOKEN }}
-      #     files: ./coverage.xml
-      #     flags: unittests
-      #     env_vars: OS,PYTHON
-      #     name: codecov-umbrella
-      #     fail_ci_if_error: true
-      #     path_to_write_report: ./codecov_report.txt
-      #     verbose: true
+      - name: "Upload coverage to Codecov"
+        if: ${{ github.event_name == 'push' || (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository) }}
+        uses: codecov/codecov-action@v1
+        with:
+          token: ${{ secrets.CODECOV_TOKEN }}
+          files: ./coverage.xml
+          flags: unittests
+          env_vars: OS,PYTHON
+          name: codecov-umbrella
+          fail_ci_if_error: true
+          path_to_write_report: ./codecov_report.txt
+          verbose: true

Changelog.md

@@ -1,3 +1,7 @@
+# 0.4.2
+
+* update package name
+
 # 0.4.1
 
 * Fix UnicodeDecodeError when viewing compressed datasets

cellxgene_gateway/__init__.py

@@ -7,4 +7,4 @@
 # OR CONDITIONS OF ANY KIND, either express or implied. See the License for
 # the specific language governing permissions and limitations under the License.
 
-__version__ = "0.4.1"
+__version__ = "0.4.2"

requirements-test.txt

@@ -0,0 +1 @@
+freezegun~=1.5.5

setup.py

@@ -38,7 +38,7 @@ install_reqs = parse_requirements()
 
 setup(
     # mandatory
-    name="cellxgene-gateway",
+    name="cellxgene_gateway",
     # mandatory
     version=get_version("cellxgene_gateway/__init__.py"),
     # mandatory

tests/test_backend_cache.py

@@ -1,7 +1,12 @@
 import unittest
-from unittest.mock import MagicMock, patch
+from http import HTTPStatus
+from unittest.mock import MagicMock, Mock, patch, call
+from freezegun import freeze_time
 
-from cellxgene_gateway.backend_cache import is_port_in_use
+from cellxgene_gateway.backend_cache import BackendCache, is_port_in_use
+from cellxgene_gateway.cache_entry import CacheEntry, CacheEntryStatus
+from cellxgene_gateway.cache_key import CacheKey
+from cellxgene_gateway.cellxgene_exception import CellxgeneException
 
 
 class TestIsPortInUse(unittest.TestCase):
@@ -26,3 +31,332 @@ class TestIsPortInUse(unittest.TestCase):
         self.assertTrue(connectMock.connect_ex.calledOnceWith("a"))
         self.assertTrue(socketMock.calledOnceWith("a"))
         self.assertEqual(is_port_in_use(123), False)
+
+
+class TestBackendCacheInit(unittest.TestCase):
+    def test_GIVEN_new_backend_cache_THEN_entry_list_is_empty(self):
+        """BackendCache should initialize with an empty entry list."""
+        cache = BackendCache()
+        self.assertEqual(cache.entry_list, [])
+
+
+class TestBackendCacheGetPorts(unittest.TestCase):
+    def test_GIVEN_empty_cache_THEN_get_ports_returns_empty_list(self):
+        """get_ports should return empty list when no entries exist."""
+        cache = BackendCache()
+        ports = cache.get_ports()
+        self.assertEqual(ports, [])
+
+    def test_GIVEN_cache_with_entries_THEN_get_ports_returns_all_ports(self):
+        """get_ports should return list of all ports from entries."""
+        cache = BackendCache()
+
+        # Create mock entries with different ports
+        entry1 = Mock()
+        entry1.port = 8000
+        entry2 = Mock()
+        entry2.port = 8001
+        entry3 = Mock()
+        entry3.port = 8002
+
+        cache.entry_list = [entry1, entry2, entry3]
+
+        ports = cache.get_ports()
+        self.assertEqual(ports, [8000, 8001, 8002])
+
+    def test_GIVEN_single_entry_THEN_get_ports_returns_single_port(self):
+        """get_ports should correctly handle a single entry."""
+        cache = BackendCache()
+        entry = Mock()
+        entry.port = 9000
+
+        cache.entry_list = [entry]
+
+        ports = cache.get_ports()
+        self.assertEqual(ports, [9000])
+
+
+class TestBackendCacheCheckPath(unittest.TestCase):
+    def setUp(self):
+        self.cache = BackendCache()
+
+    def test_GIVEN_no_matching_path_THEN_check_path_returns_none(self):
+        """check_path should return None when no entries match the path."""
+        source = Mock()
+        source.name = "test_source"
+
+        cache_entry = Mock()
+        cache_entry.status = CacheEntryStatus.loaded
+        cache_entry.key = Mock()
+        cache_entry.key.source.name = "other_source"
+        cache_entry.key.descriptor = "/some/path"
+
+        self.cache.entry_list = [cache_entry]
+
+        result = self.cache.check_path(source, "/test/path")
+        self.assertIsNone(result)
+
+    def test_GIVEN_terminated_entry_THEN_check_path_ignores_it(self):
+        """check_path should ignore entries with terminated status."""
+        source = Mock()
+        source.name = "test_source"
+
+        cache_entry = Mock()
+        cache_entry.status = CacheEntryStatus.terminated
+        cache_entry.key = Mock()
+        cache_entry.key.source.name = "test_source"
+        cache_entry.key.descriptor = "/data"
+
+        self.cache.entry_list = [cache_entry]
+
+        result = self.cache.check_path(source, "/data/file.txt")
+        self.assertIsNone(result)
+
+    def test_GIVEN_single_matching_entry_THEN_check_path_returns_it(self):
+        """check_path should return the matching entry when exactly one matches."""
+        source = Mock()
+        source.name = "test_source"
+
+        cache_entry = Mock()
+        cache_entry.status = CacheEntryStatus.loaded
+        cache_entry.key = Mock()
+        cache_entry.key.source.name = "test_source"
+        cache_entry.key.descriptor = "/data"
+
+        self.cache.entry_list = [cache_entry]
+
+        result = self.cache.check_path(source, "/data/file.txt")
+        self.assertIs(result, cache_entry)
+
+    def test_GIVEN_path_that_does_not_start_with_descriptor_THEN_check_path_returns_none(
+        self,
+    ):
+        """check_path should return None if path doesn't start with descriptor."""
+        source = Mock()
+        source.name = "test_source"
+
+        cache_entry = Mock()
+        cache_entry.status = CacheEntryStatus.loaded
+        cache_entry.key = Mock()
+        cache_entry.key.source.name = "test_source"
+        cache_entry.key.descriptor = "/data"
+
+        self.cache.entry_list = [cache_entry]
+
+        result = self.cache.check_path(source, "/other/file.txt")
+        self.assertIsNone(result)
+
+    def test_GIVEN_multiple_matching_entries_THEN_check_path_raises_exception(self):
+        """check_path should raise exception when multiple entries match."""
+        source = Mock()
+        source.name = "test_source"
+
+        cache_entry1 = Mock()
+        cache_entry1.status = CacheEntryStatus.loaded
+        cache_entry1.key = Mock()
+        cache_entry1.key.source.name = "test_source"
+        cache_entry1.key.descriptor = "/data"
+
+        cache_entry2 = Mock()
+        cache_entry2.status = CacheEntryStatus.loaded
+        cache_entry2.key = Mock()
+        cache_entry2.key.source.name = "test_source"
+        cache_entry2.key.descriptor = "/data"
+
+        self.cache.entry_list = [cache_entry1, cache_entry2]
+
+        with self.assertRaises(CellxgeneException) as context:
+            self.cache.check_path(source, "/data/file.txt")
+
+        # The CellxgeneException is raised with HTTPStatus as first arg and message as second
+        self.assertEqual(context.exception.message, HTTPStatus.INTERNAL_SERVER_ERROR)
+        self.assertIn("Found 2", context.exception.http_status)
+
+    def test_GIVEN_mixed_entries_THEN_check_path_returns_only_matching_active_entry(
+        self,
+    ):
+        """check_path should correctly filter by source, path, and status."""
+        source = Mock()
+        source.name = "target_source"
+
+        # Terminated entry - should be ignored
+        terminated_entry = Mock()
+        terminated_entry.status = CacheEntryStatus.terminated
+        terminated_entry.key = Mock()
+        terminated_entry.key.source.name = "target_source"
+        terminated_entry.key.descriptor = "/data"
+
+        # Different source - should be ignored
+        other_source_entry = Mock()
+        other_source_entry.status = CacheEntryStatus.loaded
+        other_source_entry.key = Mock()
+        other_source_entry.key.source.name = "other_source"
+        other_source_entry.key.descriptor = "/data"
+
+        # Matching entry - should be returned
+        matching_entry = Mock()
+        matching_entry.status = CacheEntryStatus.loaded
+        matching_entry.key = Mock()
+        matching_entry.key.source.name = "target_source"
+        matching_entry.key.descriptor = "/data"
+
+        self.cache.entry_list = [terminated_entry, other_source_entry, matching_entry]
+
+        result = self.cache.check_path(source, "/data/file.txt")
+        self.assertIs(result, matching_entry)
+
+
+class TestBackendCacheCheckEntry(unittest.TestCase):
+    def setUp(self):
+        self.cache = BackendCache()
+
+    def test_GIVEN_no_matching_entry_THEN_check_entry_returns_none(self):
+        """check_entry should return None when no entries match the key."""
+        key = Mock()
+
+        cache_entry = Mock()
+        cache_entry.status = CacheEntryStatus.loaded
+        cache_entry.key = Mock()
+        cache_entry.key.equals.return_value = False
+
+        self.cache.entry_list = [cache_entry]
+
+        result = self.cache.check_entry(key)
+        self.assertIsNone(result)
+
+    def test_GIVEN_terminated_entry_THEN_check_entry_ignores_it(self):
+        """check_entry should ignore entries with terminated status."""
+        key = Mock()
+
+        cache_entry = Mock()
+        cache_entry.status = CacheEntryStatus.terminated
+        cache_entry.key = Mock()
+        cache_entry.key.equals.return_value = True
+
+        self.cache.entry_list = [cache_entry]
+
+        result = self.cache.check_entry(key)
+        self.assertIsNone(result)
+
+    def test_GIVEN_single_matching_entry_THEN_check_entry_returns_it(self):
+        """check_entry should return the matching entry when exactly one matches."""
+        key = Mock()
+
+        cache_entry = Mock()
+        cache_entry.status = CacheEntryStatus.loaded
+        cache_entry.key = Mock()
+        cache_entry.key.equals.return_value = True
+
+        self.cache.entry_list = [cache_entry]
+
+        result = self.cache.check_entry(key)
+        self.assertIs(result, cache_entry)
+
+    def test_GIVEN_multiple_matching_entries_THEN_check_entry_raises_exception(self):
+        """check_entry should raise exception when multiple entries match."""
+        key = Mock()
+        key.dataset = "test_dataset"
+
+        cache_entry1 = Mock()
+        cache_entry1.status = CacheEntryStatus.loaded
+        cache_entry1.key = Mock()
+        cache_entry1.key.equals.return_value = True
+
+        cache_entry2 = Mock()
+        cache_entry2.status = CacheEntryStatus.loaded
+        cache_entry2.key = Mock()
+        cache_entry2.key.equals.return_value = True
+
+        self.cache.entry_list = [cache_entry1, cache_entry2]
+
+        with self.assertRaises(CellxgeneException) as context:
+            self.cache.check_entry(key)
+
+        # The CellxgeneException is raised with HTTPStatus as first arg and message as second
+        self.assertEqual(context.exception.message, HTTPStatus.INTERNAL_SERVER_ERROR)
+        self.assertIn("Found 2", context.exception.http_status)
+
+    def test_GIVEN_mixed_entries_THEN_check_entry_returns_only_matching_active_entry(
+        self,
+    ):
+        """check_entry should correctly filter by key equality and status."""
+        key = Mock()
+
+        # Terminated entry - should be ignored
+        terminated_entry = Mock()
+        terminated_entry.status = CacheEntryStatus.terminated
+        terminated_entry.key = Mock()
+        terminated_entry.key.equals.return_value = True
+
+        # Non-matching entry - should be ignored
+        non_matching_entry = Mock()
+        non_matching_entry.status = CacheEntryStatus.loaded
+        non_matching_entry.key = Mock()
+        non_matching_entry.key.equals.return_value = False
+
+        # Matching entry - should be returned
+        matching_entry = Mock()
+        matching_entry.status = CacheEntryStatus.loaded
+        matching_entry.key = Mock()
+        matching_entry.key.equals.return_value = True
+
+        self.cache.entry_list = [terminated_entry, non_matching_entry, matching_entry]
+
+        result = self.cache.check_entry(key)
+        self.assertIs(result, matching_entry)
+
+
+class TestBackendCachePrune(unittest.TestCase):
+    def test_GIVEN_entry_in_cache_THEN_prune_removes_it(self):
+        """prune should remove the entry from the cache."""
+        cache = BackendCache()
+
+        entry_mock = Mock()
+        cache.entry_list = [entry_mock]
+
+        cache.prune(entry_mock)
+
+        self.assertEqual(len(cache.entry_list), 0)
+        self.assertNotIn(entry_mock, cache.entry_list)
+
+    def test_GIVEN_entry_in_cache_THEN_prune_terminates_it(self):
+        """prune should call terminate on the entry."""
+        cache = BackendCache()
+
+        entry_mock = Mock()
+        cache.entry_list = [entry_mock]
+
+        cache.prune(entry_mock)
+
+        entry_mock.terminate.assert_called_once()
+
+    def test_GIVEN_multiple_entries_THEN_prune_removes_only_target_entry(self):
+        """prune should only remove the specified entry, not others."""
+        cache = BackendCache()
+
+        entry1 = Mock()
+        entry2 = Mock()
+        entry3 = Mock()
+
+        cache.entry_list = [entry1, entry2, entry3]
+
+        cache.prune(entry2)
+
+        self.assertEqual(len(cache.entry_list), 2)
+        self.assertIn(entry1, cache.entry_list)
+        self.assertNotIn(entry2, cache.entry_list)
+        self.assertIn(entry3, cache.entry_list)
+
+        # Verify only entry2 was terminated
+        entry1.terminate.assert_not_called()
+        entry2.terminate.assert_called_once()
+        entry3.terminate.assert_not_called()
+
+    def test_GIVEN_empty_cache_THEN_prune_raises_value_error(self):
+        """prune should raise ValueError if entry is not in cache."""
+        cache = BackendCache()
+
+        entry_mock = Mock()
+
+        with self.assertRaises(ValueError):
+            cache.prune(entry_mock)