WordPress.org

Make WordPress Core

Opened 4 weeks ago

Last modified 3 weeks ago

#50640 new enhancement

[PHPUnit] Mark test as skipped instead of failing if GD extension is not availble

Reported by: ayeshrajans Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords: has-patch close php8
Focuses: Cc:

Description

There are some tests in WordPress test suite that depend on PHP gd extension. Some of these tests mark the test as a failure if the extension is not available in the system under test.

I believe these tests should be marked as skipped as opposed to marking as a failure, which causes some tests to fail (instead of skipping) in certain PHP 8 builds: #50639

This ticket is not related to PHP 8, but rather on the semantics of the tests.

Attachments (1)

50640-1.patch (4.2 KB) - added by ayeshrajans 4 weeks ago.

Download all attachments as: .zip

Change History (7)

@ayeshrajans
4 weeks ago

#2 @johnbillion
4 weeks ago

  • Keywords close added

The problem with skipping tests when the platform requirements aren't met is that it removes assurance that the tests are actually running. If the test infrastructure changes in the future and a platform requirement such as gd gets accidentally gets removed, the tests should fail.

I recommend wontfix as this reduces the reliability of the tests.

#3 @ayeshrajans
4 weeks ago

With due respect, I would like to appeal for the ticket closure.

WordPress already has several tests being skipped due to extension availability:

  • mbstring
  • xsl
  • fileinfo
  • openssl
  • zip

Further, we already have Tests_Image_Meta class that skips the whole test class if gd extension is not available, which is precisely what we do in this test too. I agree about your point that this reduces test reliability, but failing a test does not improve the system we are testing anyway. Travis CI, for example, will have the gd extension in official PHP builds (all versions WordPress officially supports have Travis tests passing).

So on Travis, we will be running the exact same amount of assertions. We just avoid marking a test as "failure", which should have been a "skipped" test in a system that does not have necessary extensions to start the test in the first place.

#4 @ayeshrajans
4 weeks ago

If the test infrastructure changes in the future and a platform requirement such as gd gets accidentally gets removed, the tests should fail.

If we were to make sure the test platform meets all necessary requirements, I think it should be better done at composer.json file require-dev section (for example with ext-dev: *, ext-mbstring: *).

#5 @SergeyBiryukov
4 weeks ago

For reference, these tests were previously changed from skipping to failures in [40532] / #40533, the patch would revert that decision.

#6 @desrosj
3 weeks ago

  • Keywords php8 added
Note: See TracTickets for help on using tickets.