From 914db68e5ed06687237537fa8f18fcfaebfeb9e4 Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Sun, 3 Mar 2024 21:52:36 -0800 Subject: [PATCH] Apply suggestions from code review --- src/documents/tests/test_api_status.py | 72 ++++++++++++++++++++++++++ src/documents/views.py | 2 +- 2 files changed, 73 insertions(+), 1 deletion(-) diff --git a/src/documents/tests/test_api_status.py b/src/documents/tests/test_api_status.py index fbb3af8b9..37b635adf 100644 --- a/src/documents/tests/test_api_status.py +++ b/src/documents/tests/test_api_status.py @@ -20,6 +20,14 @@ class TestSystemStatus(APITestCase): ) def test_system_status(self): + """ + GIVEN: + - A user is logged in + WHEN: + - The user requests the system status + THEN: + - The response contains relevant system status information + """ self.client.force_login(self.user) response = self.client.get(self.ENDPOINT) self.assertEqual(response.status_code, status.HTTP_200_OK) @@ -38,6 +46,14 @@ class TestSystemStatus(APITestCase): self.assertIsNotNone(response.data["tasks"]["redis_error"]) def test_system_status_insufficient_permissions(self): + """ + GIVEN: + - A user is not logged in or does not have permissions + WHEN: + - The user requests the system status + THEN: + - The response contains a 401 status code or a 403 status code + """ response = self.client.get(self.ENDPOINT) self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) normal_user = User.objects.create_user(username="normal_user") @@ -46,6 +62,14 @@ class TestSystemStatus(APITestCase): self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) def test_system_status_container_detection(self): + """ + GIVEN: + - The application is running in a containerized environment + WHEN: + - The user requests the system status + THEN: + - The response contains the correct install type + """ self.client.force_login(self.user) os.environ["PNGX_CONTAINERIZED"] = "1" response = self.client.get(self.ENDPOINT) @@ -57,6 +81,14 @@ class TestSystemStatus(APITestCase): @mock.patch("redis.Redis.execute_command") def test_system_status_redis_ping(self, mock_ping): + """ + GIVEN: + - Redies ping returns True + WHEN: + - The user requests the system status + THEN: + - The response contains the correct redis status + """ mock_ping.return_value = True self.client.force_login(self.user) response = self.client.get(self.ENDPOINT) @@ -65,6 +97,14 @@ class TestSystemStatus(APITestCase): @mock.patch("celery.app.control.Inspect.ping") def test_system_status_celery_ping(self, mock_ping): + """ + GIVEN: + - Celery ping returns pong + WHEN: + - The user requests the system status + THEN: + - The response contains the correct celery status + """ mock_ping.return_value = {"hostname": {"ok": "pong"}} self.client.force_login(self.user) response = self.client.get(self.ENDPOINT) @@ -74,6 +114,14 @@ class TestSystemStatus(APITestCase): @override_settings(INDEX_DIR="/tmp/index") @mock.patch("whoosh.index.FileIndex.last_modified") def test_system_status_index_ok(self, mock_last_modified): + """ + GIVEN: + - The index last modified time is set + WHEN: + - The user requests the system status + THEN: + - The response contains the correct index status + """ mock_last_modified.return_value = 1707839087 self.client.force_login(self.user) response = self.client.get(self.ENDPOINT) @@ -84,6 +132,14 @@ class TestSystemStatus(APITestCase): @override_settings(INDEX_DIR="/tmp/index/") @mock.patch("documents.index.open_index", autospec=True) def test_system_status_index_error(self, mock_open_index): + """ + GIVEN: + - The index is not found + WHEN: + - The user requests the system status + THEN: + - The response contains the correct index status + """ mock_open_index.return_value = None mock_open_index.side_effect = Exception("Index error") self.client.force_login(self.user) @@ -95,6 +151,14 @@ class TestSystemStatus(APITestCase): @override_settings(DATA_DIR="/tmp/does_not_exist/data/") def test_system_status_classifier_ok(self): + """ + GIVEN: + - The classifier is found + WHEN: + - The user requests the system status + THEN: + - The response contains the correct classifier status + """ load_classifier() test_classifier = DocumentClassifier() test_classifier.save() @@ -105,6 +169,14 @@ class TestSystemStatus(APITestCase): self.assertIsNone(response.data["tasks"]["classifier_error"]) def test_system_status_classifier_error(self): + """ + GIVEN: + - The classifier is not found + WHEN: + - The user requests the system status + THEN: + - The response contains an error classifier status + """ with override_settings(MODEL_FILE="does_not_exist"): self.client.force_login(self.user) response = self.client.get(self.ENDPOINT) diff --git a/src/documents/views.py b/src/documents/views.py index 5abb349ac..bd0b6fa0f 100644 --- a/src/documents/views.py +++ b/src/documents/views.py @@ -1586,7 +1586,7 @@ class SystemStatusView(GenericAPIView, PassUserMixin): media_stats = os.statvfs(settings.MEDIA_ROOT) - redis_url = settings._CELERY_REDIS_URL + redis_url = settings._CHANNELS_REDIS_URL redis_error = None with Redis.from_url(url=redis_url) as client: try: