From 41f540c534d970b0f05f6290726754e7c2d7d0ba Mon Sep 17 00:00:00 2001 From: Nick Jackson Date: Tue, 12 Nov 2024 10:27:54 +0000 Subject: [PATCH 1/4] test(mypy): mypy now includes typing of client in tests Since the non-strict tests were scoped exclusively to the tests folder, mypy was ignoring type violations which originated in the client code. This is now fixed, and mypy is aware of the entire codebase in checking the typing of tests. --- .pre-commit-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 044f1b58..89c90f45 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -49,7 +49,7 @@ repos: - types-python-dateutil - types-pytz - ds-caselaw-utils~=2.0.0 - files: ^tests/ + exclude: ^smoketest/ id: mypy name: mypy-tests repo: https://github.com/pre-commit/mirrors-mypy From 46e60afda8daedf3d561164ae32fe9c174701758 Mon Sep 17 00:00:00 2001 From: Nick Jackson Date: Tue, 12 Nov 2024 11:35:07 +0000 Subject: [PATCH 2/4] refactor(tests): simpler test changes to pass type checking This covers off the simpler changes to tests to get them passing type checking, such as making sure things are cast correctly and we're adhering to typed dicts properly. --- src/caselawclient/models/judgments.py | 5 +- src/caselawclient/models/press_summaries.py | 5 +- .../client/test_checkout_checkin_judgment.py | 17 ++-- tests/client/test_client.py | 15 ++-- tests/client/test_eval_xslt.py | 19 ++--- tests/client/test_get_document_by_uri.py | 11 +-- .../client/test_get_judgment_and_versions.py | 7 +- ...st_get_press_summaries_for_document_uri.py | 3 +- tests/client/test_get_set_metadata.py | 23 +++--- tests/client/test_get_set_properties.py | 15 ++-- tests/client/test_get_version_annotation.py | 3 +- .../test_get_version_created_datetime.py | 3 +- .../client/test_save_copy_delete_judgment.py | 15 ++-- tests/client/test_user_privileges.py | 4 +- tests/client/test_validate_document.py | 5 +- tests/models/documents/test_document_body.py | 7 +- .../documents/test_document_validation.py | 28 +++---- tests/models/documents/test_document_verbs.py | 28 ++++--- tests/models/documents/test_documents.py | 79 ++++++++++--------- tests/models/test_judgments.py | 7 +- tests/models/test_press_summaries.py | 12 +-- tests/models/utilities/test_utilities.py | 4 +- tests/responses/test_search_result.py | 2 +- tests/test_factories.py | 2 +- 24 files changed, 173 insertions(+), 146 deletions(-) diff --git a/src/caselawclient/models/judgments.py b/src/caselawclient/models/judgments.py index 65646c50..61366662 100644 --- a/src/caselawclient/models/judgments.py +++ b/src/caselawclient/models/judgments.py @@ -47,7 +47,8 @@ def linked_document(self) -> Optional["PressSummary"]: """ try: uri = self.uri + "/press-summary/1" - PressSummary = importlib.import_module("caselawclient.models.press_summaries").PressSummary - return PressSummary(uri, self.api_client) # type: ignore + if not TYPE_CHECKING: # This isn't nice, but will be cleaned up when we refactor how related documents work + PressSummary = importlib.import_module("caselawclient.models.press_summaries").PressSummary + return PressSummary(uri, self.api_client) except DocumentNotFoundError: return None diff --git a/src/caselawclient/models/press_summaries.py b/src/caselawclient/models/press_summaries.py index 801af635..39c9cacc 100644 --- a/src/caselawclient/models/press_summaries.py +++ b/src/caselawclient/models/press_summaries.py @@ -48,7 +48,8 @@ def linked_document(self) -> Optional[Judgment]: """ try: uri = self.uri.removesuffix("/press-summary/1") - Judgment = importlib.import_module("caselawclient.models.judgments").Judgment - return Judgment(uri, self.api_client) # type: ignore + if not TYPE_CHECKING: # This isn't nice, but will be cleaned up when we refactor how related documents work + Judgment = importlib.import_module("caselawclient.models.judgments").Judgment + return Judgment(uri, self.api_client) except DocumentNotFoundError: return None diff --git a/tests/client/test_checkout_checkin_judgment.py b/tests/client/test_checkout_checkin_judgment.py index da2a3d10..dc49427a 100644 --- a/tests/client/test_checkout_checkin_judgment.py +++ b/tests/client/test_checkout_checkin_judgment.py @@ -5,6 +5,7 @@ from unittest.mock import patch from caselawclient.Client import ROOT_DIR, MarklogicApiClient +from caselawclient.models.documents import DocumentURIString class TestGetCheckoutStatus(unittest.TestCase): @@ -13,7 +14,7 @@ def setUp(self): def test_checkout_judgment(self): with patch.object(self.client, "eval") as mock_eval: - uri = "/ewca/civ/2004/632" + uri = DocumentURIString("/ewca/civ/2004/632") annotation = "locked by A KITTEN" expected_vars = { "uri": "/ewca/civ/2004/632.xml", @@ -34,7 +35,7 @@ def test_checkout_judgment_with_midnight_timeout(self): "calculate_seconds_until_midnight", return_value=3600, ): - uri = "/ewca/civ/2004/632" + uri = DocumentURIString("/ewca/civ/2004/632") annotation = "locked by A KITTEN" expires_at_midnight = True expected_vars = { @@ -49,7 +50,7 @@ def test_checkout_judgment_with_midnight_timeout(self): def test_checkout_judgment_with_timeout_seconds(self): with patch.object(self.client, "eval") as mock_eval: - uri = "/ewca/civ/2004/632" + uri = DocumentURIString("/ewca/civ/2004/632") annotation = "locked by A KITTEN" timeout_seconds = 1234 expected_vars = { @@ -64,7 +65,7 @@ def test_checkout_judgment_with_timeout_seconds(self): def test_checkin_judgment(self): with patch.object(self.client, "eval") as mock_eval: - uri = "/ewca/civ/2004/632" + uri = DocumentURIString("/ewca/civ/2004/632") expected_vars = {"uri": "/ewca/civ/2004/632.xml"} self.client.checkin_judgment(uri) @@ -73,7 +74,7 @@ def test_checkin_judgment(self): def test_get_checkout_status(self): with patch.object(self.client, "eval") as mock_eval: - uri = "judgment/uri" + uri = DocumentURIString("judgment/uri") expected_vars = {"uri": "/judgment/uri.xml"} self.client.get_judgment_checkout_status(uri) @@ -100,7 +101,7 @@ def test_get_checkout_status_message(self): b"\r\n" b"--595658fa1db1aa98--\r\n" ) - result = self.client.get_judgment_checkout_status_message("/ewca/2002/2") + result = self.client.get_judgment_checkout_status_message(DocumentURIString("/ewca/2002/2")) assert result == "locked by a kitten" def test_get_checkout_status_message_empty(self): @@ -117,7 +118,7 @@ def test_get_checkout_status_message_empty(self): b"\r\n" b"--595658fa1db1aa98--\r\n" ) - result = self.client.get_judgment_checkout_status_message("/ewca/2002/2") + result = self.client.get_judgment_checkout_status_message(DocumentURIString("/ewca/2002/2")) assert result is None def test_calculate_seconds_until_midnight(self): @@ -133,7 +134,7 @@ def test_break_judgment_checkout(self): client = MarklogicApiClient("", "", "", False) with patch.object(client, "eval") as mock_eval: - uri = "judgment/uri" + uri = DocumentURIString("judgment/uri") expected_vars = { "uri": "/judgment/uri.xml", } diff --git a/tests/client/test_client.py b/tests/client/test_client.py index 3aef000e..8586c341 100644 --- a/tests/client/test_client.py +++ b/tests/client/test_client.py @@ -15,6 +15,7 @@ get_single_string_from_marklogic_response, ) from caselawclient.errors import GatewayTimeoutError +from caselawclient.models.documents import DocumentURIString class TestErrors(unittest.TestCase): @@ -100,12 +101,12 @@ def test_eval_and_decode(self, mock_eval): b"true\r\n" b"--595658fa1db1aa98--\r\n" ) - assert self.client._eval_and_decode({"url": "/2029/eat/1"}, "myfile.xqy") == "true" + assert self.client._eval_and_decode({}, "myfile.xqy") == "true" @patch("caselawclient.Client.MarklogicApiClient._eval_and_decode") def test_document_exists(self, mock_decode): mock_decode.return_value = "true" - assert self.client.document_exists("/2029/eat/1") is True + assert self.client.document_exists(DocumentURIString("/2029/eat/1")) is True mock_decode.assert_called_with( {"uri": "/2029/eat/1.xml"}, "document_exists.xqy", @@ -114,7 +115,7 @@ def test_document_exists(self, mock_decode): @patch("caselawclient.Client.MarklogicApiClient._eval_and_decode") def test_document_not_exists(self, mock_decode): mock_decode.return_value = "false" - assert self.client.document_exists("/2029/eat/1") is False + assert self.client.document_exists(DocumentURIString("/2029/eat/1")) is False mock_decode.assert_called_with( {"uri": "/2029/eat/1.xml"}, "document_exists.xqy", @@ -159,19 +160,19 @@ def test_invoke_calls_request(self, MockPath): ) def test_format_uri(self): - uri = "/ewca/2022/123" + uri = DocumentURIString("/ewca/2022/123") assert self.client._format_uri_for_marklogic(uri) == "/ewca/2022/123.xml" def test_format_uri_no_leading_slash(self): - uri = "ewca/2022/123" + uri = DocumentURIString("ewca/2022/123") assert self.client._format_uri_for_marklogic(uri) == "/ewca/2022/123.xml" def test_format_uri_trailing_slash(self): - uri = "ewca/2022/123/" + uri = DocumentURIString("ewca/2022/123/") assert self.client._format_uri_for_marklogic(uri) == "/ewca/2022/123.xml" def test_format_uri_all_the_slashes(self): - uri = "/ewca/2022/123/" + uri = DocumentURIString("/ewca/2022/123/") assert self.client._format_uri_for_marklogic(uri) == "/ewca/2022/123.xml" def test_user_agent(self): diff --git a/tests/client/test_eval_xslt.py b/tests/client/test_eval_xslt.py index 8d6b27ac..e7779a2b 100644 --- a/tests/client/test_eval_xslt.py +++ b/tests/client/test_eval_xslt.py @@ -4,7 +4,8 @@ import unittest from unittest.mock import patch -from caselawclient.Client import ROOT_DIR, MarklogicApiClient +from caselawclient.Client import ROOT_DIR, MarklogicApiClient, MarkLogicDocumentURIString +from caselawclient.models.documents import DocumentURIString from caselawclient.xquery_type_dicts import XsltTransformDict @@ -19,9 +20,9 @@ def test_eval_xslt_user_can_view_unpublished(self): "user_can_view_unpublished_judgments", return_value=True, ): - uri = "/judgment/uri" + uri = DocumentURIString("/judgment/uri") expected_vars: XsltTransformDict = { - "uri": "/judgment/uri.xml", + "uri": MarkLogicDocumentURIString("/judgment/uri.xml"), "version_uri": None, "show_unpublished": True, "img_location": "imagepath", @@ -42,9 +43,9 @@ def test_eval_xslt_user_cannot_view_unpublished(self): "user_can_view_unpublished_judgments", return_value=False, ), patch.object(logging, "warning") as mock_logging: - uri = "/judgment/uri" + uri = DocumentURIString("/judgment/uri") expected_vars: XsltTransformDict = { - "uri": "/judgment/uri.xml", + "uri": MarkLogicDocumentURIString("/judgment/uri.xml"), "version_uri": None, "show_unpublished": False, "img_location": "imagepath", @@ -66,9 +67,9 @@ def test_eval_xslt_with_filename(self): "user_can_view_unpublished_judgments", return_value=True, ): - uri = "/judgment/uri" + uri = DocumentURIString("/judgment/uri") expected_vars: XsltTransformDict = { - "uri": "/judgment/uri.xml", + "uri": MarkLogicDocumentURIString("/judgment/uri.xml"), "version_uri": None, "show_unpublished": True, "img_location": "imagepath", @@ -91,10 +92,10 @@ def test_eval_xslt_with_query(self): "user_can_view_unpublished_judgments", return_value=True, ): - uri = "/judgment/uri" + uri = DocumentURIString("/judgment/uri") query = "the query string" expected_vars: XsltTransformDict = { - "uri": "/judgment/uri.xml", + "uri": MarkLogicDocumentURIString("/judgment/uri.xml"), "version_uri": None, "show_unpublished": True, "img_location": "imagepath", diff --git a/tests/client/test_get_document_by_uri.py b/tests/client/test_get_document_by_uri.py index b041ac55..619926af 100644 --- a/tests/client/test_get_document_by_uri.py +++ b/tests/client/test_get_document_by_uri.py @@ -9,6 +9,7 @@ DocumentHasNoTypeCollection, MarklogicApiClient, ) +from caselawclient.models.documents import DocumentURIString from caselawclient.models.judgments import Judgment from caselawclient.models.press_summaries import PressSummary @@ -22,7 +23,7 @@ def setUp(self): def test_get_document_by_uri(self, mock_get_document_type, mock_judgment): mock_get_document_type.return_value = mock_judgment - document = self.client.get_document_by_uri(uri="test/1234") + document = self.client.get_document_by_uri(uri=DocumentURIString("test/1234")) mock_get_document_type.assert_called_with("test/1234") mock_judgment.assert_called_with("test/1234", self.client, search_query=None) @@ -34,7 +35,7 @@ def test_get_document_by_uri(self, mock_get_document_type, mock_judgment): def test_get_document_by_uri_with_search_query(self, mock_get_document_type, mock_judgment): mock_get_document_type.return_value = mock_judgment - document = self.client.get_document_by_uri(uri="test/1234", search_query="Test search") + document = self.client.get_document_by_uri(uri=DocumentURIString("test/1234"), search_query="Test search") mock_get_document_type.assert_called_with("test/1234") mock_judgment.assert_called_with("test/1234", self.client, search_query="Test search") @@ -54,7 +55,7 @@ def test_get_document_type_from_uri_if_judgment( mock_eval, ): mock_get_marklogic_response.return_value = [DOCUMENT_COLLECTION_URI_JUDGMENT] - document_type = self.client.get_document_type_from_uri(uri="test/1234") + document_type = self.client.get_document_type_from_uri(uri=DocumentURIString("test/1234")) assert document_type == Judgment @patch("caselawclient.Client.MarklogicApiClient._send_to_eval") @@ -67,7 +68,7 @@ def test_get_document_type_from_uri_if_press_summary( mock_get_marklogic_response.return_value = [ DOCUMENT_COLLECTION_URI_PRESS_SUMMARY, ] - document_type = self.client.get_document_type_from_uri(uri="test/1234") + document_type = self.client.get_document_type_from_uri(uri=DocumentURIString("test/1234")) assert document_type == PressSummary @patch("caselawclient.Client.MarklogicApiClient._send_to_eval") @@ -80,4 +81,4 @@ def test_get_document_type_from_uri_if_no_valid_collection( mock_get_marklogic_response.return_value = [] with pytest.raises(DocumentHasNoTypeCollection): - self.client.get_document_type_from_uri(uri="test/1234") + self.client.get_document_type_from_uri(uri=DocumentURIString("test/1234")) diff --git a/tests/client/test_get_judgment_and_versions.py b/tests/client/test_get_judgment_and_versions.py index 1f0a335c..dde9c453 100644 --- a/tests/client/test_get_judgment_and_versions.py +++ b/tests/client/test_get_judgment_and_versions.py @@ -4,6 +4,7 @@ from unittest.mock import patch from caselawclient.Client import ROOT_DIR, MarklogicApiClient +from caselawclient.models.documents import DocumentURIString class TestGetJudgment(unittest.TestCase): @@ -28,7 +29,7 @@ def test_get_judgment_xml(self): b"" ) - result = self.client.get_judgment_xml("/judgment/uri") + result = self.client.get_judgment_xml(DocumentURIString("/judgment/uri")) expected = ( '\n' @@ -41,7 +42,7 @@ def test_get_judgment_xml(self): def test_get_judgment_version(self): with patch.object(self.client, "eval") as mock_eval: - uri = "/ewca/civ/2004/632" + uri = DocumentURIString("/ewca/civ/2004/632") version = 3 expected_vars = {"uri": "/ewca/civ/2004/632.xml", "version": "3"} self.client.get_judgment_version(uri, version) @@ -51,7 +52,7 @@ def test_get_judgment_version(self): def test_list_judgment_versions(self): with patch.object(self.client, "eval") as mock_eval: - uri = "/ewca/civ/2004/632" + uri = DocumentURIString("/ewca/civ/2004/632") expected_vars = {"uri": "/ewca/civ/2004/632.xml"} self.client.list_judgment_versions(uri) diff --git a/tests/client/test_get_press_summaries_for_document_uri.py b/tests/client/test_get_press_summaries_for_document_uri.py index c1cb8033..b81ddae1 100644 --- a/tests/client/test_get_press_summaries_for_document_uri.py +++ b/tests/client/test_get_press_summaries_for_document_uri.py @@ -2,6 +2,7 @@ from unittest.mock import call, patch from caselawclient.Client import MarklogicApiClient +from caselawclient.models.documents import DocumentURIString class TestGetPressSummariesForDocumentUri(TestCase): @@ -22,7 +23,7 @@ def test_get_press_summaries_for_document_uri( for uri in ["foo/bar", "/foo/bar"]: with self.subTest(uri=uri): - self.client.get_press_summaries_for_document_uri(uri) + self.client.get_press_summaries_for_document_uri(DocumentURIString(uri)) mock_get_marklogic_response.assert_called_with("EVAL") mock_eval.assert_called_with( diff --git a/tests/client/test_get_set_metadata.py b/tests/client/test_get_set_metadata.py index 26e37dbb..df38b9e2 100644 --- a/tests/client/test_get_set_metadata.py +++ b/tests/client/test_get_set_metadata.py @@ -5,12 +5,13 @@ from unittest.mock import patch from caselawclient.Client import ROOT_DIR, MarklogicApiClient +from caselawclient.models.documents import DocumentURIString @patch("caselawclient.Client.get_single_string_from_marklogic_response") @patch("caselawclient.Client.MarklogicApiClient.eval") def test_get_properties_for_search_results(send, decode): - uris = ["judgment/uri"] + uris = [DocumentURIString("judgment/uri")] expected_vars = {"uris": ["/judgment/uri.xml"]} decode.return_value = "decoded_value" # The decoder is called retval = MarklogicApiClient("", "", "", False).get_properties_for_search_results( @@ -28,7 +29,7 @@ def setUp(self): def test_set_judgment_citation(self): with patch.object(self.client, "eval") as mock_eval: - uri = "judgment/uri" + uri = DocumentURIString("judgment/uri") content = "new neutral citation" expected_vars = {"uri": "/judgment/uri.xml", "content": content} self.client.set_judgment_citation(uri, content) @@ -38,7 +39,7 @@ def test_set_judgment_citation(self): def test_set_judgment_citation_whitespace_stripping(self): with patch.object(self.client, "eval") as mock_eval: - uri = "judgment/uri" + uri = DocumentURIString("judgment/uri") content = " [2033] UKSC 1234 " expected_vars = {"uri": "/judgment/uri.xml", "content": "[2033] UKSC 1234"} self.client.set_judgment_citation(uri, content) @@ -48,7 +49,7 @@ def test_set_judgment_citation_whitespace_stripping(self): def test_set_document_court(self): with patch.object(self.client, "eval") as mock_eval: - uri = "judgment/uri" + uri = DocumentURIString("judgment/uri") content = "new court" expected_vars = {"uri": "/judgment/uri.xml", "content": content} self.client.set_document_court(uri, content) @@ -58,7 +59,7 @@ def test_set_document_court(self): def test_set_document_jurisdiction(self): with patch.object(self.client, "eval") as mock_eval: - uri = "judgment/uri" + uri = DocumentURIString("judgment/uri") content = "new jurisdiction" expected_vars = {"uri": "/judgment/uri.xml", "content": content} self.client.set_document_jurisdiction(uri, content) @@ -70,7 +71,7 @@ def test_set_document_court_and_jurisdiction_when_both_passed(self): # It splits the provided value on '/' # and sets both court and jurisdiction with patch.object(self.client, "eval") as mock_eval: - uri = "judgment/uri" + uri = DocumentURIString("judgment/uri") court_content = "court" jurisdiction_content = "jurisdiction" court_expected_vars = {"uri": "/judgment/uri.xml", "content": court_content} @@ -96,7 +97,7 @@ def test_set_document_court_and_jurisdiction_when_just_court_passed(self): # When no jurisdiction is included # It sets the court and deletes the jurisdiction. with patch.object(self.client, "eval") as mock_eval: - uri = "judgment/uri" + uri = DocumentURIString("judgment/uri") content = "court" court_expected_vars = {"uri": "/judgment/uri.xml", "content": content} jurisdiction_expected_vars = {"uri": "/judgment/uri.xml", "content": ""} @@ -116,7 +117,7 @@ def test_set_document_court_and_jurisdiction_when_just_court_passed(self): def test_set_document_date(self): with patch.object(self.client, "eval") as mock_eval: - uri = "judgment/uri" + uri = DocumentURIString("judgment/uri") content = "01-01-2023" expected_vars = {"uri": "/judgment/uri.xml", "content": content} self.client.set_document_work_expression_date(uri, content) @@ -135,7 +136,7 @@ def test_set_judgment_date_warn(self): self.client, "eval", ) as mock_eval: - uri = "judgment/uri" + uri = DocumentURIString("judgment/uri") content = "2022-01-01" expected_vars = {"uri": "/judgment/uri.xml", "content": "2022-01-01"} self.client.set_judgment_date(uri, content) @@ -152,7 +153,7 @@ def test_set_judgment_date_warn(self): def test_set_internal_uri_leading_slash(self): with patch.object(self.client, "eval") as mock_eval: - uri = "/judgment/uri" + uri = DocumentURIString("/judgment/uri") expected_vars = { "uri": "/judgment/uri.xml", "content_with_id": "https://caselaw.nationalarchives.gov.uk/id/judgment/uri", @@ -169,7 +170,7 @@ def test_set_internal_uri_leading_slash(self): def test_set_internal_uri_no_leading_slash(self): with patch.object(self.client, "eval") as mock_eval: - uri = "judgment/uri" + uri = DocumentURIString("judgment/uri") expected_vars = { "uri": "/judgment/uri.xml", "content_with_id": "https://caselaw.nationalarchives.gov.uk/id/judgment/uri", diff --git a/tests/client/test_get_set_properties.py b/tests/client/test_get_set_properties.py index 3961a0fd..fbbcbebe 100644 --- a/tests/client/test_get_set_properties.py +++ b/tests/client/test_get_set_properties.py @@ -4,6 +4,7 @@ from unittest.mock import patch from caselawclient.Client import ROOT_DIR, MarklogicApiClient +from caselawclient.models.documents import DocumentURIString class TestGetSetJudgmentProperties(unittest.TestCase): @@ -12,7 +13,7 @@ def setUp(self): def test_set_boolean_property_true(self): with patch.object(self.client, "eval") as mock_eval: - uri = "/judgment/uri" + uri = DocumentURIString("/judgment/uri") expected_vars = { "uri": "/judgment/uri.xml", "value": "true", @@ -28,7 +29,7 @@ def test_set_boolean_property_true(self): def test_set_boolean_property_false(self): with patch.object(self.client, "eval") as mock_eval: - uri = "/judgment/uri" + uri = DocumentURIString("/judgment/uri") expected_vars = { "uri": "/judgment/uri.xml", "value": "false", @@ -45,7 +46,7 @@ def test_set_boolean_property_false(self): def test_get_unset_boolean_property(self): with patch.object(self.client, "eval") as mock_eval: mock_eval.return_value.content = "" - result = self.client.get_boolean_property("/judgment/uri", "my-property") + result = self.client.get_boolean_property(DocumentURIString("/judgment/uri"), "my-property") assert result is False @@ -64,7 +65,7 @@ def test_get_boolean_property(self): b"\r\ntrue\r\n" b"--595658fa1db1aa98--\r\n" ) - result = self.client.get_boolean_property("/judgment/uri", "my-property") + result = self.client.get_boolean_property(DocumentURIString("/judgment/uri"), "my-property") assert result is True @@ -83,20 +84,20 @@ def test_get_property(self): b"\r\nmy-content\r\n" b"--595658fa1db1aa98--\r\n" ) - result = self.client.get_property("/judgment/uri", "my-property") + result = self.client.get_property(DocumentURIString("/judgment/uri"), "my-property") assert result == "my-content" def test_get_unset_property(self): with patch.object(self.client, "eval") as mock_eval: mock_eval.return_value.content = "" - result = self.client.get_property("/judgment/uri", "my-property") + result = self.client.get_property(DocumentURIString("/judgment/uri"), "my-property") assert result == "" def test_set_property(self): with patch.object(self.client, "eval") as mock_eval: - uri = "/judgment/uri" + uri = DocumentURIString("/judgment/uri") expected_vars = { "uri": "/judgment/uri.xml", "value": "my-value", diff --git a/tests/client/test_get_version_annotation.py b/tests/client/test_get_version_annotation.py index b5a42c95..ef674a21 100644 --- a/tests/client/test_get_version_annotation.py +++ b/tests/client/test_get_version_annotation.py @@ -2,6 +2,7 @@ from unittest.mock import patch from caselawclient.Client import MarklogicApiClient +from caselawclient.models.documents import DocumentURIString class TestGetVersionAnnotation(unittest.TestCase): @@ -22,6 +23,6 @@ def test_get_version_annotation(self): b"\r\nthis is an annotation\r\n" b"--595658fa1db1aa98--\r\n" ) - result = self.client.get_version_annotation("/judgment/uri") + result = self.client.get_version_annotation(DocumentURIString("/judgment/uri")) assert result == "this is an annotation" diff --git a/tests/client/test_get_version_created_datetime.py b/tests/client/test_get_version_created_datetime.py index 230af922..054dedb4 100644 --- a/tests/client/test_get_version_created_datetime.py +++ b/tests/client/test_get_version_created_datetime.py @@ -3,6 +3,7 @@ from unittest.mock import patch from caselawclient.Client import MarklogicApiClient +from caselawclient.models.documents import DocumentURIString class TestGetVersionCreatedDatetime(unittest.TestCase): @@ -23,7 +24,7 @@ def test_get_version_created_datetime(self): b"\r\n2022-04-11T16:12:33.548954+01:00\r\n" b"--595658fa1db1aa98--\r\n" ) - result = self.client.get_version_created_datetime("/judgment/uri") + result = self.client.get_version_created_datetime(DocumentURIString("/judgment/uri")) assert result == datetime.datetime( 2022, diff --git a/tests/client/test_save_copy_delete_judgment.py b/tests/client/test_save_copy_delete_judgment.py index 36465ef1..86520a65 100644 --- a/tests/client/test_save_copy_delete_judgment.py +++ b/tests/client/test_save_copy_delete_judgment.py @@ -10,6 +10,7 @@ from caselawclient.Client import ROOT_DIR, MarklogicApiClient from caselawclient.client_helpers import VersionAnnotation, VersionType from caselawclient.errors import InvalidContentHashError +from caselawclient.models.documents import DocumentURIString class TestSaveCopyDeleteJudgment(unittest.TestCase): @@ -24,7 +25,7 @@ def setUp(self): def test_update_document_xml(self): with patch.object(self.client, "eval") as mock_eval: - uri = "/ewca/civ/2004/632" + uri = DocumentURIString("/ewca/civ/2004/632") judgment_str = "My updated judgment" judgment_xml = ElementTree.fromstring(judgment_str) expected_vars = { @@ -64,7 +65,7 @@ def test_save_locked_judgment_xml(self): with patch.object(caselawclient.Client, "validate_content_hash"), patch.object( self.client, "eval" ) as mock_eval: - uri = "/ewca/civ/2004/632" + uri = DocumentURIString("/ewca/civ/2004/632") judgment_str = "My updated judgment" judgment_xml = judgment_str.encode("utf-8") expected_vars = { @@ -105,7 +106,7 @@ def test_save_locked_judgment_xml_checks_content_hash(self): caselawclient.Client, "validate_content_hash", ) as mock_validate_hash: - uri = "/ewca/civ/2004/632" + uri = DocumentURIString("/ewca/civ/2004/632") judgment_str = "My updated judgment" judgment_xml = judgment_str.encode("utf-8") mock_validate_hash.side_effect = InvalidContentHashError() @@ -123,7 +124,7 @@ def test_save_locked_judgment_xml_checks_content_hash(self): def test_insert_document_xml(self): with patch.object(self.client, "eval") as mock_eval: - uri = "/ewca/civ/2004/632/" + uri = DocumentURIString("/ewca/civ/2004/632/") document_str = "My judgment" document_xml = ElementTree.fromstring(document_str) expected_vars = { @@ -156,7 +157,7 @@ def test_insert_document_xml(self): def test_delete_document(self): with patch.object(self.client, "eval") as mock_eval: - uri = "/judgment/uri" + uri = DocumentURIString("/judgment/uri") expected_vars = { "uri": "/judgment/uri.xml", } @@ -167,8 +168,8 @@ def test_delete_document(self): def test_copy_document(self): with patch.object(self.client, "eval") as mock_eval: - old_uri = "/judgment/old_uri" - new_uri = "/judgment/new_uri" + old_uri = DocumentURIString("/judgment/old_uri") + new_uri = DocumentURIString("/judgment/new_uri") expected_vars = { "old_uri": "/judgment/old_uri.xml", "new_uri": "/judgment/new_uri.xml", diff --git a/tests/client/test_user_privileges.py b/tests/client/test_user_privileges.py index 41bb40de..c2a85794 100644 --- a/tests/client/test_user_privileges.py +++ b/tests/client/test_user_privileges.py @@ -3,7 +3,7 @@ import unittest from unittest.mock import patch -from caselawclient.Client import ROOT_DIR, MarklogicApiClient +from caselawclient.Client import ROOT_DIR, MarklogicApiClient, MarkLogicPrivilegeURIString class TestUserPrivileges(unittest.TestCase): @@ -13,7 +13,7 @@ def setUp(self): def test_user_has_privilege(self): with patch.object(self.client, "eval") as mock_eval: user = "laura" - privilege_uri = "https://caselaw.nationalarchives.gov.uk/custom/uri" + privilege_uri = MarkLogicPrivilegeURIString("https://caselaw.nationalarchives.gov.uk/custom/uri") privilege_action = "execute" expected_vars = { "user": "laura", diff --git a/tests/client/test_validate_document.py b/tests/client/test_validate_document.py index 4c13c8e9..40e895ae 100644 --- a/tests/client/test_validate_document.py +++ b/tests/client/test_validate_document.py @@ -2,6 +2,7 @@ from unittest.mock import patch from caselawclient.Client import MarklogicApiClient +from caselawclient.models.documents import DocumentURIString class TestValidateDocument(unittest.TestCase): @@ -23,7 +24,7 @@ def test_validation_success(self): b"--c878f7cb55370005--\r\n" ) - assert self.client.validate_document("/foo/bar/123") is True + assert self.client.validate_document(DocumentURIString("/foo/bar/123")) is True def test_validation_failure(self): with patch.object(self.client, "eval") as mock_eval: @@ -43,4 +44,4 @@ def test_validation_failure(self): b"--c878f7cb55370005--\r\n" ) - assert self.client.validate_document("/foo/bar/123") is False + assert self.client.validate_document(DocumentURIString("/foo/bar/123")) is False diff --git a/tests/models/documents/test_document_body.py b/tests/models/documents/test_document_body.py index 608aa3a6..6703c17d 100644 --- a/tests/models/documents/test_document_body.py +++ b/tests/models/documents/test_document_body.py @@ -249,9 +249,10 @@ def test_dates(self, mock_api_client): """) - assert body.enrichment_datetime.year == 2024 - assert body.transformation_datetime.year == 2026 - assert body.get_latest_manifestation_datetime().year == 2026 + assert body.enrichment_datetime and body.enrichment_datetime.year == 2024 + assert body.transformation_datetime and body.transformation_datetime.year == 2026 + latest_manifestation_datetime = body.get_latest_manifestation_datetime() + assert latest_manifestation_datetime and latest_manifestation_datetime.year == 2026 assert body.get_latest_manifestation_type() == "transform" assert [x.year for x in body.get_manifestation_datetimes("tna-enriched")] == [2024, 2023] diff --git a/tests/models/documents/test_document_validation.py b/tests/models/documents/test_document_validation.py index 6daec77b..221f2bc3 100644 --- a/tests/models/documents/test_document_validation.py +++ b/tests/models/documents/test_document_validation.py @@ -1,14 +1,12 @@ import pytest -from caselawclient.models.documents import ( - Document, -) +from caselawclient.models.documents import Document, DocumentURIString class TestDocumentValidation: def test_judgment_is_failure(self, mock_api_client): - successful_document = Document("test/1234", mock_api_client) - failing_document = Document("failures/test/1234", mock_api_client) + successful_document = Document(DocumentURIString("test/1234"), mock_api_client) + failing_document = Document(DocumentURIString("failures/test/1234"), mock_api_client) successful_document.body.failed_to_parse = False failing_document.body.failed_to_parse = True @@ -17,30 +15,30 @@ def test_judgment_is_failure(self, mock_api_client): assert failing_document.is_failure is True def test_judgment_is_parked(self, mock_api_client): - normal_document = Document("test/1234", mock_api_client) - parked_document = Document("parked/a1b2c3d4", mock_api_client) + normal_document = Document(DocumentURIString("test/1234"), mock_api_client) + parked_document = Document(DocumentURIString("parked/a1b2c3d4"), mock_api_client) assert normal_document.is_parked is False assert parked_document.is_parked is True def test_has_name(self, mock_api_client): - document_with_name = Document("test/1234", mock_api_client) + document_with_name = Document(DocumentURIString("test/1234"), mock_api_client) document_with_name.body.name = "Judgment v Judgement" - document_without_name = Document("test/1234", mock_api_client) + document_without_name = Document(DocumentURIString("test/1234"), mock_api_client) document_without_name.body.name = "" assert document_with_name.has_name is True assert document_without_name.has_name is False def test_has_court_is_covered_by_has_valid_court(self, mock_api_client): - document_with_court = Document("test/1234", mock_api_client) + document_with_court = Document(DocumentURIString("test/1234"), mock_api_client) document_with_court.body.court = "UKSC" - document_without_court = Document("test/1234", mock_api_client) + document_without_court = Document(DocumentURIString("test/1234"), mock_api_client) document_without_court.body.court = "" - document_with_court_and_jurisdiction = Document("test/1234", mock_api_client) + document_with_court_and_jurisdiction = Document(DocumentURIString("test/1234"), mock_api_client) document_with_court_and_jurisdiction.body.court = "UKFTT-GRC" document_with_court_and_jurisdiction.body.jurisdiction = "InformationRights" @@ -69,7 +67,7 @@ def test_document_is_publishable_conditions( has_valid_court, publishable, ): - document = Document("test/1234", mock_api_client) + document = Document(DocumentURIString("test/1234"), mock_api_client) document.is_failure = is_failure document.is_parked = is_parked document.is_held = is_held @@ -88,7 +86,7 @@ def test_document_validation_failure_messages_if_no_messages(self, mock_api_clie """ - document = Document("test/1234", mock_api_client) + document = Document(DocumentURIString("test/1234"), mock_api_client) document.is_parked = False document.is_held = False document.has_valid_court = True @@ -96,7 +94,7 @@ def test_document_validation_failure_messages_if_no_messages(self, mock_api_clie assert document.validation_failure_messages == [] def test_judgment_validation_failure_messages_if_failing(self, mock_api_client): - document = Document("test/1234", mock_api_client) + document = Document(DocumentURIString("test/1234"), mock_api_client) document.is_failure = True document.is_parked = True document.is_held = True diff --git a/tests/models/documents/test_document_verbs.py b/tests/models/documents/test_document_verbs.py index 45881066..aabb25c7 100644 --- a/tests/models/documents/test_document_verbs.py +++ b/tests/models/documents/test_document_verbs.py @@ -11,14 +11,16 @@ CannotPublishUnpublishableDocument, Document, DocumentNotSafeForDeletion, + DocumentURIString, ) from caselawclient.models.judgments import Judgment +from caselawclient.models.neutral_citation_mixin import NeutralCitationString class TestDocumentPublish: def test_publish_fails_if_not_publishable(self, mock_api_client): with pytest.raises(CannotPublishUnpublishableDocument): - document = Document("test/1234", mock_api_client) + document = Document(DocumentURIString("test/1234"), mock_api_client) document.is_publishable = False document.publish() mock_api_client.set_published.assert_not_called() @@ -33,7 +35,7 @@ def test_publish( mock_announce_document_event, mock_api_client, ): - document = Document("test/1234", mock_api_client) + document = Document(DocumentURIString("test/1234"), mock_api_client) document.is_publishable = True document.publish() mock_publish_documents.assert_called_once_with("test/1234") @@ -54,7 +56,7 @@ def test_unpublish( mock_announce_document_event, mock_api_client, ): - document = Document("test/1234", mock_api_client) + document = Document(DocumentURIString("test/1234"), mock_api_client) document.unpublish() mock_unpublish_documents.assert_called_once_with("test/1234") mock_api_client.set_published.assert_called_once_with("test/1234", False) @@ -69,7 +71,7 @@ class TestDocumentEnrich: @time_machine.travel(datetime.datetime(1955, 11, 5, 6)) @patch("caselawclient.models.documents.announce_document_event") def test_force_enrich(self, mock_announce_document_event, mock_api_client): - document = Document("test/1234", mock_api_client) + document = Document(DocumentURIString("test/1234"), mock_api_client) document.force_enrich() mock_api_client.set_property.assert_called_once_with( @@ -86,7 +88,7 @@ def test_force_enrich(self, mock_announce_document_event, mock_api_client): @patch("caselawclient.models.documents.Document.force_enrich") def test_no_enrich_when_can_enrich_is_false(self, force_enrich, mock_api_client): - document = Document("test/1234", mock_api_client) + document = Document(DocumentURIString("test/1234"), mock_api_client) with patch.object( Document, "can_enrich", @@ -98,7 +100,7 @@ def test_no_enrich_when_can_enrich_is_false(self, force_enrich, mock_api_client) @patch("caselawclient.models.documents.Document.force_enrich") def test_enrich_when_can_enrich_is_true(self, force_enrich, mock_api_client): - document = Document("test/1234", mock_api_client) + document = Document(DocumentURIString("test/1234"), mock_api_client) with patch.object( Document, "can_enrich", @@ -111,7 +113,7 @@ def test_enrich_when_can_enrich_is_true(self, force_enrich, mock_api_client): class TestDocumentHold: def test_hold(self, mock_api_client): - document = Document("test/1234", mock_api_client) + document = Document(DocumentURIString("test/1234"), mock_api_client) document.hold() mock_api_client.set_property.assert_called_once_with( "test/1234", @@ -120,7 +122,7 @@ def test_hold(self, mock_api_client): ) def test_unhold(self, mock_api_client): - document = Document("test/1234", mock_api_client) + document = Document(DocumentURIString("test/1234"), mock_api_client) document.unhold() mock_api_client.set_property.assert_called_once_with( "test/1234", @@ -131,20 +133,20 @@ def test_unhold(self, mock_api_client): class TestDocumentDelete: def test_not_safe_to_delete_if_published(self, mock_api_client): - document = Document("test/1234", mock_api_client) + document = Document(DocumentURIString("test/1234"), mock_api_client) document.is_published = True assert document.safe_to_delete is False def test_safe_to_delete_if_unpublished(self, mock_api_client): - document = Document("test/1234", mock_api_client) + document = Document(DocumentURIString("test/1234"), mock_api_client) document.is_published = False assert document.safe_to_delete is True @patch("caselawclient.models.documents.delete_documents_from_private_bucket") def test_delete_if_safe(self, mock_aws_delete_documents, mock_api_client): - document = Document("test/1234", mock_api_client) + document = Document(DocumentURIString("test/1234"), mock_api_client) document.safe_to_delete = True document.delete() @@ -154,7 +156,7 @@ def test_delete_if_safe(self, mock_aws_delete_documents, mock_api_client): @patch("caselawclient.models.documents.delete_documents_from_private_bucket") def test_delete_if_unsafe(self, mock_aws_delete_documents, mock_api_client): - document = Document("test/1234", mock_api_client) + document = Document(DocumentURIString("test/1234"), mock_api_client) document.safe_to_delete = False with pytest.raises(DocumentNotSafeForDeletion): @@ -224,7 +226,7 @@ def test_force_reparse_empty(self, sns, mock_api_client): def test_force_reparse_full(self, sns, mock_api_client): document = Judgment("test/2023/123", mock_api_client) - document.neutral_citation = "[2023] Test 123" + document.neutral_citation = NeutralCitationString("[2023] Test 123") document.consignment_reference = "TDR-12345" document.body.name = "Judgment v Judgement" diff --git a/tests/models/documents/test_documents.py b/tests/models/documents/test_documents.py index 87436451..0127ab5b 100644 --- a/tests/models/documents/test_documents.py +++ b/tests/models/documents/test_documents.py @@ -16,6 +16,7 @@ DOCUMENT_STATUS_NEW, DOCUMENT_STATUS_PUBLISHED, Document, + DocumentURIString, ) from caselawclient.models.judgments import Judgment from tests.test_helpers import MockMultipartResponse @@ -28,28 +29,28 @@ def test_has_sensible_repr_with_name_and_judgment(self, mock_api_client): assert str(document) == "" def test_has_sensible_repr_without_name_or_subclass(self, mock_api_client): - document = Document("test/1234", mock_api_client) + document = Document(DocumentURIString("test/1234"), mock_api_client) assert str(document) == "" def test_uri_strips_slashes(self, mock_api_client): - document = Document("////test/1234/////", mock_api_client) + document = Document(DocumentURIString("////test/1234/////"), mock_api_client) assert document.uri == "test/1234" def test_public_uri(self, mock_api_client): - document = Document("test/1234", mock_api_client) + document = Document(DocumentURIString("test/1234"), mock_api_client) assert document.public_uri == "https://caselaw.nationalarchives.gov.uk/test/1234" def test_document_exists_check(self, mock_api_client): mock_api_client.document_exists.return_value = False with pytest.raises(DocumentNotFoundError): - Document("not_a_real_judgment", mock_api_client) + Document(DocumentURIString("not_a_real_judgment"), mock_api_client) def test_judgment_is_published(self, mock_api_client): mock_api_client.get_published.return_value = True - document = Document("test/1234", mock_api_client) + document = Document(DocumentURIString("test/1234"), mock_api_client) assert document.is_published is True mock_api_client.get_published.assert_called_once_with("test/1234") @@ -57,27 +58,27 @@ def test_judgment_is_published(self, mock_api_client): def test_judgment_is_held(self, mock_api_client): mock_api_client.get_property.return_value = False - document = Document("test/1234", mock_api_client) + document = Document(DocumentURIString("test/1234"), mock_api_client) assert document.is_held is False mock_api_client.get_property.assert_called_once_with("test/1234", "editor-hold") def test_judgment_is_locked(self, mock_api_client): mock_api_client.get_judgment_checkout_status_message.return_value = "Judgment locked" - document = Document("test/1234", mock_api_client) + document = Document(DocumentURIString("test/1234"), mock_api_client) assert document.is_locked is True def test_judgment_is_not_locked(self, mock_api_client): mock_api_client.get_judgment_checkout_status_message.return_value = None - document = Document("test/1234", mock_api_client) + document = Document(DocumentURIString("test/1234"), mock_api_client) assert document.is_locked is False def test_judgment_source_name(self, mock_api_client): mock_api_client.get_property.return_value = "Test Name" - document = Document("test/1234", mock_api_client) + document = Document(DocumentURIString("test/1234"), mock_api_client) assert document.source_name == "Test Name" mock_api_client.get_property.assert_called_once_with("test/1234", "source-name") @@ -85,7 +86,7 @@ def test_judgment_source_name(self, mock_api_client): def test_judgment_source_email(self, mock_api_client): mock_api_client.get_property.return_value = "testemail@example.com" - document = Document("test/1234", mock_api_client) + document = Document(DocumentURIString("test/1234"), mock_api_client) assert document.source_email == "testemail@example.com" mock_api_client.get_property.assert_called_once_with( @@ -96,7 +97,7 @@ def test_judgment_source_email(self, mock_api_client): def test_judgment_consignment_reference(self, mock_api_client): mock_api_client.get_property.return_value = "TDR-2023-ABC" - document = Document("test/1234", mock_api_client) + document = Document(DocumentURIString("test/1234"), mock_api_client) assert document.consignment_reference == "TDR-2023-ABC" mock_api_client.get_property.assert_called_once_with( @@ -108,7 +109,7 @@ def test_judgment_consignment_reference(self, mock_api_client): def test_judgment_docx_url(self, mock_url_generator, mock_api_client): mock_url_generator.return_value = "https://example.com/mock.docx" - document = Document("test/1234", mock_api_client) + document = Document(DocumentURIString("test/1234"), mock_api_client) assert document.docx_url == "https://example.com/mock.docx" mock_url_generator.assert_called_once @@ -117,7 +118,7 @@ def test_judgment_docx_url(self, mock_url_generator, mock_api_client): def test_judgment_pdf_url(self, mock_url_generator, mock_api_client): mock_url_generator.return_value = "https://example.com/mock.pdf" - document = Document("test/1234", mock_api_client) + document = Document(DocumentURIString("test/1234"), mock_api_client) assert document.pdf_url == "https://example.com/mock.pdf" mock_url_generator.assert_called_once @@ -125,61 +126,61 @@ def test_judgment_pdf_url(self, mock_url_generator, mock_api_client): def test_judgment_assigned_to(self, mock_api_client): mock_api_client.get_property.return_value = "testuser" - document = Document("test/1234", mock_api_client) + document = Document(DocumentURIString("test/1234"), mock_api_client) assert document.assigned_to == "testuser" mock_api_client.get_property.assert_called_once_with("test/1234", "assigned-to") def test_document_status(self, mock_api_client): - new_document = Document("test/1234", mock_api_client) + new_document = Document(DocumentURIString("test/1234"), mock_api_client) new_document.is_held = False new_document.is_published = False new_document.assigned_to = "" assert new_document.status == DOCUMENT_STATUS_NEW - in_progress_document = Document("test/1234", mock_api_client) + in_progress_document = Document(DocumentURIString("test/1234"), mock_api_client) in_progress_document.is_held = False in_progress_document.is_published = False in_progress_document.assigned_to = "duck" assert in_progress_document.status == DOCUMENT_STATUS_IN_PROGRESS - on_hold_document = Document("test/1234", mock_api_client) + on_hold_document = Document(DocumentURIString("test/1234"), mock_api_client) on_hold_document.is_held = True on_hold_document.is_published = False on_hold_document.assigned_to = "duck" assert on_hold_document.status == DOCUMENT_STATUS_HOLD - published_document = Document("test/1234", mock_api_client) + published_document = Document(DocumentURIString("test/1234"), mock_api_client) published_document.is_held = False published_document.is_published = True published_document.assigned_to = "duck" assert published_document.status == DOCUMENT_STATUS_PUBLISHED def test_document_best_identifier(self, mock_api_client): - example_document = Document("uri", mock_api_client) + example_document = Document(DocumentURIString("uri"), mock_api_client) assert example_document.best_human_identifier is None def test_document_version_of_a_version_fails(self, mock_api_client): - version_document = Document("test/1234_xml_versions/9-1234", mock_api_client) + version_document = Document(DocumentURIString("test/1234_xml_versions/9-1234"), mock_api_client) with pytest.raises(NotSupportedOnVersion): version_document.versions_as_documents def test_document_versions_happy_case(self, mock_api_client): - version_document = Document("test/1234", mock_api_client) + version_document = Document(DocumentURIString("test/1234"), mock_api_client) version_document.versions = [ - {"uri": "test/1234_xml_versions/2-1234.xml"}, - {"uri": "test/1234_xml_versions/1-1234.xml"}, + {"uri": "test/1234_xml_versions/2-1234.xml", "version": 2}, + {"uri": "test/1234_xml_versions/1-1234.xml", "version": 1}, ] version_document.versions_as_documents[0].uri = "test/1234_xml_versions/2-1234.xml" def test_document_version_number_when_not_version(self, mock_api_client): - base_document = Document("test/1234", mock_api_client) + base_document = Document(DocumentURIString("test/1234"), mock_api_client) with pytest.raises(OnlySupportedOnVersion): base_document.version_number assert not base_document.is_version def test_document_version_number_when_is_version(self, mock_api_client): - version_document = Document("test/1234_xml_versions/9-1234", mock_api_client) + version_document = Document(DocumentURIString("test/1234_xml_versions/9-1234"), mock_api_client) assert version_document.version_number == 9 assert version_document.is_version @@ -192,7 +193,7 @@ def test_number_of_mentions_when_no_mentions(self, mock_api_client): """, ) - document = Document("test/1234", mock_api_client) + document = Document(DocumentURIString("test/1234"), mock_api_client) assert document.number_of_mentions("some") == 0 @@ -207,21 +208,21 @@ def test_number_of_mentions_when_mentions(self, mock_api_client): """, ) - document = Document("test/1234", mock_api_client) + document = Document(DocumentURIString("test/1234"), mock_api_client) assert document.number_of_mentions("some") == 2 def test_validates_against_schema(self, mock_api_client): mock_api_client.validate_document.return_value = True - document = Document("test/1234", mock_api_client) + document = Document(DocumentURIString("test/1234"), mock_api_client) assert document.validates_against_schema is True mock_api_client.validate_document.assert_called_with(document.uri) def test_document_initialises_with_search_query_string(self, mock_api_client): - document = Document("test/1234", mock_api_client, search_query="test search query") + document = Document(DocumentURIString("test/1234"), mock_api_client, search_query="test search query") mock_api_client.get_judgment_xml_bytestring.assert_called_with( document.uri, show_unpublished=True, search_query="test search query" @@ -230,13 +231,13 @@ def test_document_initialises_with_search_query_string(self, mock_api_client): class TestDocumentEnrichedRecently: def test_enriched_recently_returns_false_when_never_enriched(self, mock_api_client): - document = Document("test/1234", mock_api_client) + document = Document(DocumentURIString("test/1234"), mock_api_client) mock_api_client.get_property.return_value = "" assert document.enriched_recently is False def test_enriched_recently_returns_true_within_cooldown(self, mock_api_client): - document = Document("test/1234", mock_api_client) + document = Document(DocumentURIString("test/1234"), mock_api_client) document.body.enrichment_datetime = datetime.datetime.now( tz=datetime.timezone.utc, ) - datetime.timedelta(seconds=30) @@ -244,7 +245,7 @@ def test_enriched_recently_returns_true_within_cooldown(self, mock_api_client): assert document.enriched_recently is True def test_enriched_recently_returns_false_outside_cooldown(self, mock_api_client): - document = Document("test/1234", mock_api_client) + document = Document(DocumentURIString("test/1234"), mock_api_client) document.body.enrichment_datetime = datetime.datetime.now( tz=datetime.timezone.utc, ) - datetime.timedelta(days=2) @@ -285,7 +286,7 @@ def test_returns_true_when_enriched_recently_is_true_and_validates_against_schem validates_against_schema, can_enrich, ): - document = Document("test/1234", mock_api_client) + document = Document(DocumentURIString("test/1234"), mock_api_client) with patch.object( Document, "enriched_recently", @@ -302,20 +303,26 @@ def test_returns_true_when_enriched_recently_is_true_and_validates_against_schem class TestMethodMissing: def test_attribute_on_body(self, mock_api_client): - doc = DocumentFactory.build(uri="test/1234", body=DocumentBodyFactory.build(name="docname")) + doc = DocumentFactory.build( + uri=DocumentURIString("test/1234"), body=DocumentBodyFactory.build(name="docname") + ) with pytest.deprecated_call(): name = doc.name assert name == "docname" def test_real_attribute(self, mock_api_client): - doc = DocumentFactory.build(uri="test/1234", body=DocumentBodyFactory.build(name="docname")) + doc = DocumentFactory.build( + uri=DocumentURIString("test/1234"), body=DocumentBodyFactory.build(name="docname") + ) with warnings.catch_warnings(): identifier = doc.best_human_identifier assert identifier is None def test_absent_item(self, mock_api_client): - doc = DocumentFactory.build(uri="test/1234", body=DocumentBodyFactory.build(name="docname")) + doc = DocumentFactory.build( + uri=DocumentURIString("test/1234"), body=DocumentBodyFactory.build(name="docname") + ) with pytest.raises( AttributeError, match="Neither 'Document' nor 'DocumentBody' objects have an attribute 'x'" ): diff --git a/tests/models/test_judgments.py b/tests/models/test_judgments.py index 883b7c5a..e63394a7 100644 --- a/tests/models/test_judgments.py +++ b/tests/models/test_judgments.py @@ -5,22 +5,23 @@ from caselawclient.errors import DocumentNotFoundError from caselawclient.factories import PressSummaryFactory from caselawclient.models.judgments import Judgment +from caselawclient.models.neutral_citation_mixin import NeutralCitationString class TestJudgment: def test_best_identifier(self, mock_api_client): judgment = Judgment("test/1234", mock_api_client) - judgment.neutral_citation = "[2023] TEST 1234" + judgment.neutral_citation = NeutralCitationString("[2023] TEST 1234") assert judgment.best_human_identifier == judgment.neutral_citation class TestJudgmentValidation: def test_has_ncn(self, mock_api_client): document_with_ncn = Judgment("test/1234", mock_api_client) - document_with_ncn.neutral_citation = "[2023] TEST 1234" + document_with_ncn.neutral_citation = NeutralCitationString("[2023] TEST 1234") document_without_ncn = Judgment("test/1234", mock_api_client) - document_without_ncn.neutral_citation = "" + document_without_ncn.neutral_citation = NeutralCitationString("") assert document_with_ncn.has_ncn is True assert document_without_ncn.has_ncn is False diff --git a/tests/models/test_press_summaries.py b/tests/models/test_press_summaries.py index 5cb30b17..d0224b16 100644 --- a/tests/models/test_press_summaries.py +++ b/tests/models/test_press_summaries.py @@ -4,23 +4,25 @@ from caselawclient.errors import DocumentNotFoundError from caselawclient.factories import JudgmentFactory +from caselawclient.models.documents import DocumentURIString +from caselawclient.models.neutral_citation_mixin import NeutralCitationString from caselawclient.models.press_summaries import PressSummary class TestPressSummary: def test_best_identifier(self, mock_api_client): summary = PressSummary("test/1234", mock_api_client) - summary.neutral_citation = "[2023] TEST 1234" + summary.neutral_citation = NeutralCitationString("[2023] TEST 1234") assert summary.best_human_identifier == summary.neutral_citation class TestPressSummaryValidation: def test_has_ncn(self, mock_api_client): - document_with_ncn = PressSummary("test/1234", mock_api_client) - document_with_ncn.neutral_citation = "[2023] TEST 1234" + document_with_ncn = PressSummary(DocumentURIString("test/1234"), mock_api_client) + document_with_ncn.neutral_citation = NeutralCitationString("[2023] TEST 1234") - document_without_ncn = PressSummary("test/1234", mock_api_client) - document_without_ncn.neutral_citation = "" + document_without_ncn = PressSummary(DocumentURIString("test/1234"), mock_api_client) + document_without_ncn.neutral_citation = NeutralCitationString("") assert document_with_ncn.has_ncn is True assert document_without_ncn.has_ncn is False diff --git a/tests/models/utilities/test_utilities.py b/tests/models/utilities/test_utilities.py index 3981c0ac..4b012381 100644 --- a/tests/models/utilities/test_utilities.py +++ b/tests/models/utilities/test_utilities.py @@ -6,6 +6,8 @@ import ds_caselaw_utils from moto import mock_aws +from caselawclient.models.documents import DocumentURIString +from caselawclient.models.neutral_citation_mixin import NeutralCitationString from caselawclient.models.utilities import extract_version, move, render_versions from caselawclient.models.utilities.aws import ( build_new_key, @@ -98,7 +100,7 @@ def test_move_judgment_success( ds_caselaw_utils.neutral_url = MagicMock(return_value="new/uri") fake_api_client = MagicMock() fake_api_client.document_exists.return_value = False - move.update_document_uri("old/uri", "[2023] EAT 1", fake_api_client) + move.update_document_uri(DocumentURIString("old/uri"), NeutralCitationString("[2023] EAT 1"), fake_api_client) fake_api_client.copy_document.assert_called_with("old/uri", "new/uri") fake_metadata.assert_called_with("old/uri", "new/uri", fake_api_client) diff --git a/tests/responses/test_search_result.py b/tests/responses/test_search_result.py index 58784dd6..95517769 100644 --- a/tests/responses/test_search_result.py +++ b/tests/responses/test_search_result.py @@ -151,7 +151,7 @@ def test_create_from_node_with_invalid_jurisdiction_code(self): node = etree.fromstring(xml) search_result = SearchResult(node, self.client) - assert search_result.court.name == "First-tier Tribunal (General Regulatory Chamber)" + assert search_result.court and search_result.court.name == "First-tier Tribunal (General Regulatory Chamber)" class TestSearchResultMeta: diff --git a/tests/test_factories.py b/tests/test_factories.py index 1df97733..7036bfd5 100644 --- a/tests/test_factories.py +++ b/tests/test_factories.py @@ -11,7 +11,7 @@ def test_content_as_html(): class TestSearchStatusBehaviour: def test_status(self): search = SearchResultFactory.build() - assert search.metadata.editor_status == "New" + assert search.metadata.editor_status == "New" # type: ignore[attr-defined] class TestDocumentNCNBehaviour: From dfa345579a29e142c952ec64677102956456ea69 Mon Sep 17 00:00:00 2001 From: Nick Jackson Date: Tue, 12 Nov 2024 14:51:24 +0000 Subject: [PATCH 3/4] test(tests): fix not using patched functions correctly in assertions We were incorrectly calling assertion methods on the original name for the patched function, not the patched object, leading to typing warnings. This is now fixed. --- tests/client/test_advanced_search.py | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/tests/client/test_advanced_search.py b/tests/client/test_advanced_search.py index 37687859..5d38f7a8 100644 --- a/tests/client/test_advanced_search.py +++ b/tests/client/test_advanced_search.py @@ -24,10 +24,10 @@ def test_invoke_called_with_default_params_when_optional_parameters_not_provided parameters and return the response """ - with patch.object(self.client, "invoke"): + with patch.object(self.client, "invoke") as patched_invoke: response = self.client.advanced_search(SearchParameters()) - self.client.invoke.assert_called_with( + patched_invoke.assert_called_with( "/judgments/search/search-v2.xqy", json.dumps( { @@ -50,7 +50,7 @@ def test_invoke_called_with_default_params_when_optional_parameters_not_provided ), ) - assert response == self.client.invoke.return_value + assert response == patched_invoke.return_value def test_invoke_called_with_all_params_when_all_parameters_provided(self): """ @@ -60,7 +60,7 @@ def test_invoke_called_with_all_params_when_all_parameters_provided(self): Then it should call the MarkLogic module with all the parameters and return the response """ - with patch.object(self.client, "invoke"): + with patch.object(self.client, "invoke") as patched_invoke: response = self.client.advanced_search( SearchParameters( query="test query", @@ -80,7 +80,7 @@ def test_invoke_called_with_all_params_when_all_parameters_provided(self): ), ) - self.client.invoke.assert_called_with( + patched_invoke.assert_called_with( "/judgments/search/search-v2.xqy", json.dumps( { @@ -103,7 +103,7 @@ def test_invoke_called_with_all_params_when_all_parameters_provided(self): ), ) - assert response == self.client.invoke.return_value + assert response == patched_invoke.return_value def test_exception_raised_when_invoke_raises_an_exception(self): """ @@ -113,8 +113,8 @@ def test_exception_raised_when_invoke_raises_an_exception(self): Then it should raise that same exception """ exception = Exception("Error message from MarkLogic") - with patch.object(self.client, "invoke"): - self.client.invoke.side_effect = exception + with patch.object(self.client, "invoke") as patched_invoke: + patched_invoke.side_effect = exception with pytest.raises(Exception) as e: self.client.advanced_search(SearchParameters(query="test query")) assert e.value == exception @@ -177,7 +177,7 @@ def test_user_can_view_unpublished_and_show_unpublished_is_true( When the advanced_search method is called with the show_unpublished parameter set to True Then it should call the MarkLogic module with the expected query parameters """ - with patch.object(self.client, "invoke"), patch.object( + with patch.object(self.client, "invoke") as patched_invoke, patch.object( self.client, "user_can_view_unpublished_judgments", return_value=True, @@ -193,7 +193,7 @@ def test_user_can_view_unpublished_and_show_unpublished_is_true( show_unpublished=True, ), ) - assert '"show_unpublished": "true"' in self.client.invoke.call_args.args[1] + assert '"show_unpublished": "true"' in patched_invoke.call_args.args[1] def test_user_cannot_view_unpublished_but_show_unpublished_is_true( self, @@ -204,7 +204,7 @@ def test_user_cannot_view_unpublished_but_show_unpublished_is_true( When the advanced_search method is called with the show_unpublished parameter set to True Then it should call the MarkLogic module with the show_unpublished parameter set to False and log a warning """ - with patch.object(self.client, "invoke"), patch.object( + with patch.object(self.client, "invoke") as patched_invoke, patch.object( self.client, "user_can_view_unpublished_judgments", return_value=False, @@ -221,7 +221,7 @@ def test_user_cannot_view_unpublished_but_show_unpublished_is_true( ), ) - assert '"show_unpublished": "false"' in self.client.invoke.call_args.args[1] + assert '"show_unpublished": "false"' in patched_invoke.call_args.args[1] mock_logging.assert_called() def test_no_page_0(self): @@ -231,11 +231,11 @@ def test_no_page_0(self): When the advanced_search method is called with the page parameter set to 0 Then it should call the MarkLogic module with the page parameter set to 1 """ - with patch.object(self.client, "invoke"): + with patch.object(self.client, "invoke") as patched_invoke: self.client.advanced_search( SearchParameters( page=0, ), ) - assert ', "page": 1,' in self.client.invoke.call_args.args[1] + assert ', "page": 1,' in patched_invoke.call_args.args[1] From f14444776125e687d4aec0b0e145c5a64bd8249f Mon Sep 17 00:00:00 2001 From: Nick Jackson Date: Wed, 13 Nov 2024 09:52:08 +0000 Subject: [PATCH 4/4] test(tests): fix incorrect patching of a method --- tests/client/test_client.py | 50 ++++++++++++++++++------------------- 1 file changed, 24 insertions(+), 26 deletions(-) diff --git a/tests/client/test_client.py b/tests/client/test_client.py index 8586c341..0f51d509 100644 --- a/tests/client/test_client.py +++ b/tests/client/test_client.py @@ -126,38 +126,36 @@ def test_eval_calls_request(self, MockPath): mock_path_instance = MockPath.return_value mock_path_instance.read_text.return_value = "mock-query" - self.client.session.request = MagicMock() - - self.client.eval("mock-query-path.xqy", vars='{{"testvar":"test"}}') - - self.client.session.request.assert_called_with( - "POST", - url=self.client._path_to_request_url("LATEST/eval"), - headers={ - "Content-type": "application/x-www-form-urlencoded", - "Accept": "multipart/mixed", - }, - data={"xquery": "mock-query", "vars": '{{"testvar":"test"}}'}, - ) + with patch.object(self.client.session, "request") as patched_request: + self.client.eval("mock-query-path.xqy", vars='{{"testvar":"test"}}') + + patched_request.assert_called_with( + "POST", + url=self.client._path_to_request_url("LATEST/eval"), + headers={ + "Content-type": "application/x-www-form-urlencoded", + "Accept": "multipart/mixed", + }, + data={"xquery": "mock-query", "vars": '{{"testvar":"test"}}'}, + ) @patch("caselawclient.Client.Path") def test_invoke_calls_request(self, MockPath): mock_path_instance = MockPath.return_value mock_path_instance.read_text.return_value = "mock-query" - self.client.session.request = MagicMock() - - self.client.invoke("mock-query-path.xqy", vars='{{"testvar":"test"}}') - - self.client.session.request.assert_called_with( - "POST", - url=self.client._path_to_request_url("LATEST/invoke"), - headers={ - "Content-type": "application/x-www-form-urlencoded", - "Accept": "multipart/mixed", - }, - data={"module": "mock-query-path.xqy", "vars": '{{"testvar":"test"}}'}, - ) + with patch.object(self.client.session, "request") as patched_request: + self.client.invoke("mock-query-path.xqy", vars='{{"testvar":"test"}}') + + patched_request.assert_called_with( + "POST", + url=self.client._path_to_request_url("LATEST/invoke"), + headers={ + "Content-type": "application/x-www-form-urlencoded", + "Accept": "multipart/mixed", + }, + data={"module": "mock-query-path.xqy", "vars": '{{"testvar":"test"}}'}, + ) def test_format_uri(self): uri = DocumentURIString("/ewca/2022/123")