From 9eb91b326da2963a3ba569d6a7e385a13c4e5bcf Mon Sep 17 00:00:00 2001 From: Adam Dangoor Date: Fri, 4 Sep 2026 19:50:10 +0100 Subject: [PATCH 1/2] Split the test_invalid_json CI job into its three cases The pattern was the critical path of the Test workflow at 51 minutes, where the next slowest was 10. Of that, 48 minutes was tenacity sleeping off Vuforia 429s in ten-second units and six seconds was test work. Split the single parametrised test into the three cases it conflated: a body which is not JSON, valid JSON which is not an object, and a JSON object which is not UTF-8. Each is now its own ci_pattern, so the critical path becomes the five-value case rather than all seven. Splitting adds patterns past the 100 available credentials files, where the secrets index wraps and two jobs share a database. Order the pattern list so that every entry from index 100 on is one which never parametrises over the Real Vuforia backend, which also fixes the existing overlap between test_target_validators.py and test_query.py. Refs #3540 Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/test.yml | 17 ++- tests/mock_vws/test_invalid_json.py | 191 +++++++++++++++------------- 2 files changed, 117 insertions(+), 91 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 44cab5307..d01584a63 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -86,7 +86,9 @@ jobs: - tests/mock_vws/test_get_duplicates.py - tests/mock_vws/test_get_target.py - tests/mock_vws/test_invalid_given_id.py - - tests/mock_vws/test_invalid_json.py::TestInvalidJSON::test_invalid_json + - tests/mock_vws/test_invalid_json.py::TestInvalidJSON::test_not_json + - tests/mock_vws/test_invalid_json.py::TestInvalidJSON::test_not_an_object + - tests/mock_vws/test_invalid_json.py::TestInvalidJSON::test_not_utf_8 - tests/mock_vws/test_invalid_json.py::TestInvalidJSON::test_invalid_json_with_skewed_time - tests/mock_vws/test_target_list.py - tests/mock_vws/test_reco_counts_report.py @@ -117,9 +119,6 @@ jobs: - tests/mock_vws/test_update_target.py::TestUpdate - tests/mock_vws/test_update_target.py::TestWidth - tests/mock_vws/test_update_target.py::TestInactiveProject - - tests/mock_vws/test_requests_mock_usage.py - - tests/mock_vws/test_respx_mock_usage.py - - tests/mock_vws/test_flask_app_usage.py - tests/mock_vws/test_model_target_generation_failure.py - tests/mock_vws/test_model_target_generation_warning.py - tests/mock_vws/test_model_target_failure_response.py @@ -129,6 +128,9 @@ jobs: - tests/mock_vws/test_vumark_generation_api.py - tests/mock_vws/test_vumark_generation_failure.py - tests/mock_vws/test_target_validators.py + - tests/mock_vws/test_requests_mock_usage.py + - tests/mock_vws/test_respx_mock_usage.py + - tests/mock_vws/test_flask_app_usage.py - tests/mock_vws/test_healthcheck.py - tests/mock_vws/test_docker.py - README.rst @@ -155,8 +157,11 @@ jobs: # See the "CI Setup" document for details of how this was set up. ci/decrypt_secret.sh tar xvf "${HOME}"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/secrets/secrets.tar - # Matrix jobs are 0-based; only secrets 0-99 exist. Wrap for overflow - # (e.g. docs/ at index 100 after the suite grew past 100 patterns). + # Matrix jobs are 0-based; only secrets 0-99 exist, so patterns + # past index 99 wrap and share a database with an earlier job. + # The ci_pattern list is therefore ordered so that every pattern + # from index 100 on is one which never parametrises over the Real + # Vuforia backend, and so never uses these credentials. SECRET_INDEX=$((JOB_INDEX % 100)) cp "./ci_secrets/vuforia_secrets_${SECRET_INDEX}.env" ./vuforia_secrets.env diff --git a/tests/mock_vws/test_invalid_json.py b/tests/mock_vws/test_invalid_json.py index 0b4b8a727..a4a639650 100644 --- a/tests/mock_vws/test_invalid_json.py +++ b/tests/mock_vws/test_invalid_json.py @@ -7,6 +7,7 @@ from zoneinfo import ZoneInfo import pytest +from beartype import beartype from freezegun import freeze_time from vws_auth_tools import authorization_header, rfc_1123_date @@ -21,104 +22,124 @@ from tests.mock_vws.utils.too_many_requests import handle_server_errors -@pytest.mark.usefixtures("verify_mock_vuforia") -class TestInvalidJSON: - """Tests for giving invalid JSON to endpoints.""" +@beartype +def _assert_body_rejected(*, endpoint: Endpoint, content: bytes) -> None: + """Send the given body to the given endpoint and assert that it is + rejected. + + Args: + endpoint: The endpoint to send the body to. + content: The body to send, which is not a JSON object encoded as + UTF-8. + """ + gmt = ZoneInfo(key="GMT") + now = datetime.now(tz=gmt) + time_to_freeze = now + with freeze_time(time_to_freeze=time_to_freeze): + date = rfc_1123_date() + + authorization_string = authorization_header( + access_key=endpoint.access_key, + secret_key=endpoint.secret_key, + method=endpoint.method, + content=content, + content_type=endpoint.auth_header_content_type, + date=date, + request_path=endpoint.path_url, + ) - @staticmethod - @pytest.mark.parametrize( - argnames="content", - argvalues=[ - b"a", - b"[]", - b'"hello"', - b"5", - b"null", - b"true", - # JSON which is an object, but which is encoded as latin-1 rather - # than UTF-8. - '{"name": "café"}'.encode(encoding="latin-1"), - ], + new_headers = { + **endpoint.headers, + "Authorization": authorization_string, + "Date": date, + "Content-Length": str(object=len(content)), + } + + new_endpoint = Endpoint( + base_url=endpoint.base_url, + path_url=endpoint.path_url, + method=endpoint.method, + headers=new_headers, + data=content, + successful_headers_result_code=endpoint.successful_headers_result_code, + successful_headers_status_code=endpoint.successful_headers_status_code, + access_key=endpoint.access_key, + secret_key=endpoint.secret_key, ) - def test_invalid_json(endpoint: Endpoint, content: bytes) -> None: - """Giving invalid, non-object or non-UTF-8 JSON returns error - responses. - """ - gmt = ZoneInfo(key="GMT") - now = datetime.now(tz=gmt) - time_to_freeze = now - with freeze_time(time_to_freeze=time_to_freeze): - date = rfc_1123_date() - authorization_string = authorization_header( - access_key=endpoint.access_key, - secret_key=endpoint.secret_key, - method=endpoint.method, - content=content, - content_type=endpoint.auth_header_content_type, - date=date, - request_path=endpoint.path_url, - ) + response = new_endpoint.send() - new_headers = { - **endpoint.headers, - "Authorization": authorization_string, - "Date": date, - "Content-Length": str(object=len(content)), - } + handle_server_errors(response=response) - new_endpoint = Endpoint( - base_url=endpoint.base_url, - path_url=endpoint.path_url, - method=endpoint.method, - headers=new_headers, - data=content, - successful_headers_result_code=endpoint.successful_headers_result_code, - successful_headers_status_code=endpoint.successful_headers_status_code, - access_key=endpoint.access_key, - secret_key=endpoint.secret_key, + takes_json_data = endpoint.auth_header_content_type == "application/json" + + assert_valid_date_header(response=response) + + if takes_json_data: + expected_result_code = ( + ResultCodes.BAD_REQUEST + if endpoint.path_url.endswith("/instances") + else ResultCodes.FAIL ) + assert_vws_failure( + response=response, + status_code=HTTPStatus.BAD_REQUEST, + result_code=expected_result_code, + ) + return + + netloc = urlparse(url=endpoint.base_url).netloc + if netloc == "cloudreco.vuforia.com": + assert_vwq_failure( + response=response, + status_code=HTTPStatus.BAD_REQUEST, + content_type="application/json", + cache_control=None, + www_authenticate=None, + connection="keep-alive", + ) + expected_text = "No image." + assert response.text == expected_text + return - response = new_endpoint.send() + assert response.status_code == HTTPStatus.BAD_REQUEST + assert not response.text + assert "Content-Type" not in response.headers - handle_server_errors(response=response) - takes_json_data = ( - endpoint.auth_header_content_type == "application/json" - ) +@pytest.mark.usefixtures("verify_mock_vuforia") +class TestInvalidJSON: + """Tests for giving invalid JSON to endpoints. - assert_valid_date_header(response=response) + The three cases are separate tests, rather than one test parametrised + over every body, because each one costs a request per endpoint against + Real Vuforia and CI runs one ``ci_pattern`` per job. See + https://github.com/VWS-Python/vws-python-mock/issues/3540. + """ - if takes_json_data: - expected_result_code = ( - ResultCodes.BAD_REQUEST - if endpoint.path_url.endswith("/instances") - else ResultCodes.FAIL - ) - assert_vws_failure( - response=response, - status_code=HTTPStatus.BAD_REQUEST, - result_code=expected_result_code, - ) - return + @staticmethod + def test_not_json(endpoint: Endpoint) -> None: + """Giving a body which is not JSON returns an error response.""" + _assert_body_rejected(endpoint=endpoint, content=b"a") - netloc = urlparse(url=endpoint.base_url).netloc - if netloc == "cloudreco.vuforia.com": - assert_vwq_failure( - response=response, - status_code=HTTPStatus.BAD_REQUEST, - content_type="application/json", - cache_control=None, - www_authenticate=None, - connection="keep-alive", - ) - expected_text = "No image." - assert response.text == expected_text - return + @staticmethod + @pytest.mark.parametrize( + argnames="content", + argvalues=[b"[]", b'"hello"', b"5", b"null", b"true"], + ) + def test_not_an_object(endpoint: Endpoint, content: bytes) -> None: + """Giving valid JSON which is not an object returns an error + response. + """ + _assert_body_rejected(endpoint=endpoint, content=content) - assert response.status_code == HTTPStatus.BAD_REQUEST - assert not response.text - assert "Content-Type" not in response.headers + @staticmethod + def test_not_utf_8(endpoint: Endpoint) -> None: + """Giving a JSON object which is not encoded as UTF-8 returns an + error response. + """ + content = '{"name": "café"}'.encode(encoding="latin-1") + _assert_body_rejected(endpoint=endpoint, content=content) @staticmethod def test_invalid_json_with_skewed_time(endpoint: Endpoint) -> None: From 3086c4a6a22099acd93a5d3036eb6c35dc8c67b8 Mon Sep 17 00:00:00 2001 From: Adam Dangoor Date: Fri, 4 Sep 2026 19:54:06 +0100 Subject: [PATCH 2/2] Avoid a word the docstring spell checker rejects pylint's wrong-spelling-in-docstring check does not accept "parametrised". Reword rather than add it to the private dictionary, since the rest of the codebase does not use the word in prose. Co-Authored-By: Claude Opus 5 (1M context) --- tests/mock_vws/test_invalid_json.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/mock_vws/test_invalid_json.py b/tests/mock_vws/test_invalid_json.py index a4a639650..a35addaa2 100644 --- a/tests/mock_vws/test_invalid_json.py +++ b/tests/mock_vws/test_invalid_json.py @@ -111,8 +111,8 @@ def _assert_body_rejected(*, endpoint: Endpoint, content: bytes) -> None: class TestInvalidJSON: """Tests for giving invalid JSON to endpoints. - The three cases are separate tests, rather than one test parametrised - over every body, because each one costs a request per endpoint against + The three cases are separate tests, rather than one test covering + every body, because each body costs a request per endpoint against Real Vuforia and CI runs one ``ci_pattern`` per job. See https://github.com/VWS-Python/vws-python-mock/issues/3540. """