Make WordPress Core

Opened 9 years ago

Last modified 3 weeks ago

#40538 accepted task (blessed)

Fix or remove useless PHPUnit tests

Reported by: johnbillion Owned by: johnbillion
Priority: normal Milestone: Future Release
Component: Build/Test Tools Version:
Severity: normal Keywords: needs-unit-tests ongoing has-patch
Cc: Focuses:

Description

There are 29 tests in the test suite which don't perform an assertion. They should be fixed or removed.

PHPUnit 6 has switched to being strict about useless tests by default, so that gives us an additional reason to address them. In addition, there's no reason for core's default PHPUnit configuration to not be strict about useless tests so the same behaviour is seen when running older versions of PHPUnit.

Previously: #36016

Attachments (2)

40538.diif (2.7 KB ) - added by Mte90 9 years ago.
new 4 tests for rest api
rest-pages-controller.diff (1.4 KB ) - added by tomepajk 8 years ago.
tests for WP_Test_REST_Pages_Controller::test_create_item and WP_Test_REST_Pages_Controller::test_get_item

Download all attachments as: .zip

Change History (17)

#1 @johnbillion
9 years ago

  • Owner set to johnbillion
  • Status newaccepted

#2 @johnbillion
9 years ago

In 40534:

Build/Test Tools: Be strict about tests that do not test anything.

See #40538

#3 @johnbillion
9 years ago

In 40535:

Build/Test Tools: Ensure that WP_UnitTestCase::expectedDeprecated() performs an assertion to avoid risky test notices.

See #40538

#4 @johnbillion
9 years ago

In 40541:

Build/Test Tools: Only perform an assertion for deprecated calls and wrongdoings if any are expected.

This avoids masking risky tests that don't otherwise perform an assertion.

See #40538

#5 @johnbillion
9 years ago

In 40542:

Build/Test Tools: More tweaks to the deprecated calls assertion. This needs to be triggered when there are unexpected deprecated calls or wrongdoings too.

See #40538

This ticket was mentioned in Slack in #core by obenland. View the logs.


9 years ago

#7 @obenland
9 years ago

@johnbillion Can this be closed for 4.8?

#8 @johnbillion
9 years ago

  • Keywords needs-unit-tests ongoing added
  • Milestone 4.8Future Release

There are still a bunch of tests that don't perform any assertions.

@Mte90
9 years ago

new 4 tests for rest api

#9 @Mte90
9 years ago

  • Keywords has-patch added

This patch remove the empty units for 4 tests :-)

Are missing:

There were 4 risky tests:

1) WP_Test_REST_Pages_Controller::test_prepare_item
This test did not perform any assertions

2) WP_Test_REST_Settings_Controller::test_context_param
This test did not perform any assertions

3) WP_Test_REST_Settings_Controller::test_prepare_item
This test did not perform any assertions

4) WP_Test_REST_Settings_Controller::test_get_item_schema
This test did not perform any assertions

I am not sure about prepare, context and get_item_schema tests about what they have to do.

#10 @johnbillion
9 years ago

Thanks for the patch, @Mte90. AFAICT these tests are actually blocked by a bug I found when working on #41463. See 4:ticket:41463.

This ticket was mentioned in Slack in #core by tomepajk. View the logs.


8 years ago

@tomepajk
8 years ago

tests for WP_Test_REST_Pages_Controller::test_create_item and WP_Test_REST_Pages_Controller::test_get_item

#12 @SergeyBiryukov
4 years ago

In 53921:

Tests: Consistently skip tests for non-implemented methods in REST API test classes.

WordPress core test suite uses PHPUnit's beStrictAboutTestsThatDoNotTestAnything option set to true, which marks a test as risky when no assertions are performed.

REST API test classes have some empty tests for non-implemented methods because these test classes extend the abstract WP_Test_REST_Controller_Testcase class, which requires several methods to be implemented that don't necessarily make sense for all REST API routes.

Some of these empty tests were already marked as skipped, but not in a consistent manner. Since skipping these tests is intentional for the time being, this commit aims to bring some consistency and adjust them all to be more accurately reported as skipped instead of risky.

The skipping can be reconsidered in the future when the tests are either removed as unnecessary or updated to actually perform assertions related to their behavior.

Follow-up to [40534], [41176], [41228].

Props Mte90, tomepajk, johnbillion, zieladam, SergeyBiryukov.
See #40538, #41463, #55652.

#13 @SergeyBiryukov
4 years ago

In 54058:

Tests: Explicitly mark empty REST API tests as not performing any assertions.

WordPress core test suite uses PHPUnit's beStrictAboutTestsThatDoNotTestAnything option set to true, which marks a test as risky when no assertions are performed.

REST API test classes have some empty tests for non-implemented methods because these test classes extend the abstract WP_Test_REST_Controller_Testcase class, which requires several methods to be implemented that don't necessarily make sense for all REST API routes.

As these tests are intentionally empty, they were previously marked as skipped, so that they are not reported as risky.

This commit aims to further reduce noise in the test suite and effectively ignores these empty tests altogether, which seems like a more appropriate option at this time.

The @doesNotPerformAssertions annotation can be reconsidered in the future when the tests are either removed as unnecessary or updated to actually perform assertions related to their behavior.

Follow-up to [40534], [41176], [41228], [53921].

See #40538, #41463, #55652.

#14 @lancewillett
3 weeks ago

Updated count for August 16, 2026

trunk today has 101 occurrences of @doesNotPerformAssertions across 26 files:

  • 88 in 20 files under tests/phpunit/tests/rest-api/
  • 13 in 6 files elsewhere, but 5 of those are also REST controllers, under icons/ and fonts/font-library/. Only user/passwordHash.php is unrelated.
  • It has spread into directories that did not exist when [53921] and [54058] applied it, so the count grows with each new controller rather than staying flat.

Looks structural rather than per-test: WP_Test_REST_Controller_Testcase declares nine abstract methods, including test_create_item(), test_update_item() and test_delete_item(). Read-only routes cannot assert on those. The themes controller docblocks read "The create_item() method does not exist for themes."

The strictness from [40534] is intact: beStrictAboutTestsThatDoNotTestAnything and failOnRisky are both still true.

Two tasks:

  1. Make the abstract reflect read-only routes, so new controllers stop inheriting this.
  2. Per-case read for tests that could assert but do not.

#15 @lancewillett
3 weeks ago

I created this AI skill to help with unit tests audits: https://github.com/WordPress/core-contributor-skills/tree/trunk/skills/unit-tests

Works with both Claude and Codex

Note: See TracTickets for help on using tickets.