diff --git a/.github/workflows/pr-checks.yaml b/.github/workflows/pr-checks.yaml index f9b07f3..62a0931 100644 --- a/.github/workflows/pr-checks.yaml +++ b/.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 diff --git a/requirements-test.txt b/requirements-test.txt new file mode 100644 index 0000000..3a993a5 --- /dev/null +++ b/requirements-test.txt @@ -0,0 +1 @@ +freezegun~=1.5.5 \ No newline at end of file diff --git a/tests/test_backend_cache.py b/tests/test_backend_cache.py index ef631bd..70a06d7 100644 --- a/tests/test_backend_cache.py +++ b/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)