Make WordPress Core

Opened 4 weeks ago

Last modified 45 hours ago

#66073 new task (blessed)

Tests: Let WP_Test_REST_Controller_Testcase reflect read-only routes

Reported by: lancewillett Owned by:
Priority: normal Milestone: 7.2
Component: Build/Test Tools Version:
Severity: normal Keywords: dev-feedback has-patch has-unit-tests
Cc: Focuses: tests

Description

Follow-up to #40538, splitting out the structural half so it can land on its own.

The problem

WP_Test_REST_Controller_Testcase declares nine abstract methods (tests/phpunit/includes/testcase-rest-controller.php, lines 24–40 on trunk today):

test_register_routes()   test_context_param()    test_get_items()
test_get_item()          test_create_item()      test_update_item()
test_delete_item()       test_prepare_item()     test_get_item_schema()

Every controller test must implement all nine. A read-only controller has no create, update or delete route, so those three methods have nothing to assert. Since [53921] and [54058] the accepted way out has been @doesNotPerformAssertions, with the note that it "can be reconsidered in the future".

That has not stayed flat. Trunk today carries 101 @doesNotPerformAssertions annotations in 26 files, and 98 of them are REST controller tests. Twenty files sit under tests/phpunit/tests/rest-api/, plus three under fonts/font-library/, two under icons/, and one unrelated case in user/passwordHash.php. Each new read-only controller inherits the pattern on the day it is written.

Options

  1. Default implementations that assert the negative. Make the six operation methods non-abstract, with a default that dispatches the operation against the controller's collection and item routes and asserts it is rejected (rest_no_route, or the controller's documented error). A controller that supports the operation overrides the method as today. This turns 98 no-ops into real assertions with one change and no per-file work, and it stops the count growing.
  2. A read-only base class. A second testcase with the read-side methods only, and controllers pick the one that fits. Cleaner signature, but every existing read-only controller test has to move, and nothing stops a future controller extending the wrong one.
  3. Leave the abstract alone and keep annotating. The status quo.

Option 1 seems the best trade, but the default needs care: some controllers register a route and reject the method with 403 or 405 rather than rest_no_route, and the schema for a POST body differs per controller. A first patch could dispatch and accept any error response, then tighten.

Once the base class change lands, the remaining annotations outside the abstract methods are a per-case read, which stays on #40538.

Related: #40538, #41463, #55652, [53921], [54058].

Change History (12)

#1 @sukhendu2002
12 days ago

I checked the latest trunk: the nine methods are still abstract, and read-only controller tests still use @doesNotPerformAssertions for unsupported operations. I agree these should become meaningful assertions.

Before changing the base class, I think we need to decide how each test supplies its concrete routes, a valid request, and the expected rejection. Accepting any error could pass for the wrong reason—for example, a permissions failure. POST is not always a create operation, either: block rendering uses POST for a read-like request.

I’d suggest trying an explicit route/request fixture with a few representative controllers first, then making the three base methods concrete and removing the existing no-op overrides. Otherwise, those overrides would continue to shadow the new defaults.

Does that sound like a reasonable first step? Open to other approaches.

#2 @lancewillett
10 days ago

Thanks @sukhendu2002—yes, a small prototype with representative controllers sounds like the right first step. Please go ahead.

Two corrections to my original proposal:

  1. Existing no-op overrides would need removing.
  2. Accepting any error would be too weak.

We should assert the specific expected status and error code, using valid fixtures and appropriate permissions.

Let’s start with a conventional read-only controller and the block renderer, keeping its supported POST behavior covered. The icons tests offer an existing negative-request pattern, though checking rest_no_route alongside the status would make that stronger.

Some empty methods also defer to more specific tests elsewhere; those need separate treatment. Let’s use the prototype to decide whether shared defaults simplify this before changing the base class broadly.

This ticket was mentioned in ​PR #13817 on ​WordPress/wordpress-develop by ​@shailu25.


6 days ago
#3

  • Keywords has-patch has-unit-tests added; needs-patch removed

Trac ticket: #66073

What

Prototype for #66073 that replaces empty @doesNotPerformAssertions create/update/delete (and block-renderer get_items) stubs with real negative assertions on two representative controllers:

  • WP_REST_Menu_Locations_Controller (conventional read-only)
  • WP_REST_Block_Renderer_Controller (read-like POST on the item route)

Each unsupported request asserts rest_no_route with status 404, using valid fixtures and authenticated users.

Why

Read-only REST controller tests currently implement abstract create/update/delete methods as empty no-ops. That hides whether unsupported operations are actually rejected, and the annotation count keeps growing.

This small prototype follows committer feedback: prove specific status + error-code assertions on representative controllers before changing WP_Test_REST_Controller_Testcase broadly.

How

  • Menu locations: POST collection, POST item, and DELETE item → rest_no_route / 404
  • Block renderer:
    • POST / GET collection → rest_no_route / 404 (no collection route)
    • PUT / DELETE item → rest_no_route / 404
    • Item-level POST left covered by existing test_get_item_post_request()
  • Left prepare_item() empty (no HTTP surface; separate treatment as discussed on the ticket)
  • Did not change the abstract base class yet

Testing instructions

  1. Start the local WordPress develop environment.
  2. Run:
npm run test:php -- --filter 'Tests_REST_WpRestMenuLocationsController|REST_Block_Renderer_Controller_Test'

Use of AI tools

Cursor

#4 @shailu25
6 days ago

I've opened a PR for Prototype:​13817

#5 @lancewillett
6 days ago

In 64004:

Tests: Assert unsupported requests for read-only REST controllers.

Replace empty test methods with assertions that unsupported menu-location and block-renderer requests return rest_no_route with status 404. Use authenticated users and valid item fixtures, and retain coverage for the block renderer's supported POST requests.

Leave the abstract test base and prepare-item stubs unchanged for further discussion.

Developed in: ​https://github.com/WordPress/wordpress-develop/pull/13817

Props shailu25.
See #66073.

This ticket was mentioned in ​PR #13949 on ​WordPress/wordpress-develop by ​@shailu25.


2 days ago
#7

Trac ticket: #66073

What

Follow-up for #66073 that replaces empty @doesNotPerformAssertions get/create/update/delete stubs in WP_REST_Block_Directory_Controller_Test with real negative assertions.

Each unsupported request asserts rest_no_route with status 404, using an authenticated administrator and a valid-looking item fixture (guidepost).

Updated file: tests/phpunit/tests/rest-api/rest-block-directory-controller.php

Why

The block directory controller only registers GET /wp/v2/block-directory/search. Empty abstract-method stubs hide whether unsupported operations are actually rejected, and the annotation count keeps growing.

This continues the agreed approach from [64004] / PR #13817 on another conventional read-only controller, before any broader WP_Test_REST_Controller_Testcase change.

How

  • GET /wp/v2/block-directory/guidepost → rest_no_route / 404 (no item route)
  • POST /wp/v2/block-directory/search → rest_no_route / 404 (search is GET-only)
  • POST /wp/v2/block-directory/guidepost → rest_no_route / 404
  • DELETE /wp/v2/block-directory/guidepost → rest_no_route / 404
  • Left prepare_item() unchanged (already has real coverage)
  • Did not change the abstract base class yet

Testing instructions

  1. Start the local WordPress develop environment.
  2. Run:
npm run test:php -- --filter WP_REST_Block_Directory_Controller_Test

Use of AI tools

Cursor

This ticket was mentioned in ​PR #13950 on ​WordPress/wordpress-develop by ​@shailu25.


2 days ago
#8

Trac ticket: #66073

What

Follow-up for #66073 that replaces empty @doesNotPerformAssertions get/create/update/delete stubs in WP_REST_Pattern_Directory_Controller_Test with real negative assertions.

Each unsupported request asserts rest_no_route with status 404, using an authenticated contributor and a valid-looking item fixture (1).

Updated file: tests/phpunit/tests/rest-api/rest-pattern-directory-controller.php

Why

The pattern directory controller only registers GET /wp/v2/pattern-directory/patterns. Empty abstract-method stubs hide whether unsupported operations are actually rejected, and the annotation count keeps growing.

How

  • GET /wp/v2/pattern-directory/patterns/1 → rest_no_route / 404 (no item route)
  • POST /wp/v2/pattern-directory/patterns → rest_no_route / 404 (collection is GET-only)
  • POST /wp/v2/pattern-directory/patterns/1 → rest_no_route / 404
  • DELETE /wp/v2/pattern-directory/patterns/1 → rest_no_route / 404
  • Left prepare_item() unchanged (already has real coverage)
  • Did not change the abstract base class yet

Testing instructions

  1. Start the local WordPress develop environment.
  2. Run:
npm run test:php -- --filter WP_REST_Pattern_Directory_Controller_Test

Use of AI Tools

  • Cursor

#9 @lancewillett
45 hours ago

In 64075:

Tests: Assert unsupported block directory requests.

Replace empty @doesNotPerformAssertions stubs with checks that unsupported item and write requests return rest_no_route with status 404. Use an authenticated administrator so permission errors cannot satisfy the assertions.

Developed in: ​https://github.com/WordPress/wordpress-develop/pull/13949

Props shailu25.
See #66073.

#11 @lancewillett
45 hours ago

In 64076:

Tests: Assert unsupported pattern directory requests.

Replace empty @doesNotPerformAssertions stubs with checks that unsupported item and write requests return rest_no_route with status 404. Use an authenticated contributor so permission errors cannot satisfy the assertions.

Developed in: ​https://github.com/WordPress/wordpress-develop/pull/13950

Props shailu25.
See #66073.

Note: See TracTickets for help on using tickets.