Skip to content

Cross-cutting endpoint tests do not cover the reco counts or Model Target endpoints #3400

Description

@adamtheturtle

What happens

The endpoint fixture in tests/conftest.py parametrises ten endpoints:

params=[
"add_target",
"database_summary",
"delete_target",
"get_duplicates",
"get_target",
"target_list",
"target_summary",
"update_target",
"query",
"vumark_generate_instance",
],

Six test modules consume it to check concerns which apply to every endpoint rather than to any one of them:

  • test_authorization_header.py — missing, malformed and unknown-key Authorization headers
  • test_date_header.py — date format and clock skew
  • test_content_length.pyContent-Length too large, too small, not an integer
  • test_invalid_json.py — malformed JSON bodies
  • test_unexpected_json.py — bodies sent to endpoints which do not take one
  • test_requests_mock_usage.py

Endpoints missing from that list:

  • POST /imagetargets/databases/{database_id}/reports/recoCounts
  • GET /reports/recoCounts/{report_id}
  • every Model Target Web API route, including POST /oauth2/token

So none of those six cross-cutting concerns is checked for the reco counts endpoints or the Model Target API, on any backend.

What I checked

I compared the reco counts report endpoint against add_target and database_summary across every cross-cutting variant I could exercise, using prepared requests so that requests does not recompute Content-Length:

variant add_target database_summary reco_counts_report
ok 201 TargetCreated 200 Success 200 Success
content-length too small 401 AuthenticationFailure 401 AuthenticationFailure 401 AuthenticationFailure
content-length too large 408 408 408
content-length not int 400 400 400
no content-type 401 AuthenticationFailure 200 Success 401 AuthenticationFailure
empty content-type 401 AuthenticationFailure 200 Success 401 AuthenticationFailure

No divergence. The reco counts endpoint behaves the same as add_target on every one of these. database_summary differs only where a GET with no body legitimately should.

So this issue is not reporting a bug. It is reporting that the mechanism which would catch one is not pointed at these endpoints.

Why it is still worth doing

The reco counts endpoints are the newest VWS routes, added in #3357, and the Model Target API is the newest API. They are the two areas most likely to acquire a cross-cutting inconsistency, and the two with the least verified coverage.

The body-parsing paths in particular are where defects have actually turned up — #3391 and #3392 are both in the category test_invalid_json.py exists to cover, and both were found by hand rather than by a test.

The reason for the gap is structural rather than an oversight: Endpoint in tests/mock_vws/utils/__init__.py carries a secret_key and access_key and is built for VWS-style HMAC signing. Model Target routes use bearer tokens, so they do not fit the type as it stands, and the reco counts download URL takes no authorization at all.

Suggested resolution

Add reco_counts_report to prepared_requests.py and to the endpoint fixture list. It signs like every other VWS endpoint, so it should fit Endpoint unchanged, and it is the cheap half of this.

The Model Target routes need a decision first: either widen Endpoint so it can describe a bearer-token endpoint and accept that some cross-cutting tests do not apply to it, or give the Model Target API its own smaller parametrised fixture covering the concerns which do apply. The second is probably less disruptive, since the VWS date-header and HMAC tests are meaningless for a bearer-token API.

The reco counts download route deserves a note either way: it deliberately takes no authorization, standing in for a presigned URL, so it should be recorded as intentionally out of scope rather than left looking forgotten.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions