Investigate new test failure in PHP 8.2

[desrosj]

There's a new unit test failure being reported on PHP 8.2 all of a sudden.

There was 1 failure:

1) Tests_Functions::test_wp_check_filetype_and_ext_with_filtered_woff
Failed asserting that two arrays are identical.
--- Expected
+++ Actual
@@ @@
 Array &0 (
-    'ext' => 'woff'
-    'type' => 'application/font-woff'
+    'ext' => false
+    'type' => false
     'proper_filename' => false
 )

/var/www/tests/phpunit/tests/functions.php:1428
/var/www/vendor/bin/phpunit:123

After some investigation, this seems to be caused by the PHP 8.2 Docker container used during testing.

The containers are usually rebuilt weekly on Sunday, ensuring the latest point releases (or beta/RC versions in this case) are used. Unfortunately, GitHub disables workflows that run on schedule when a repository does not have activity for a period of time, so this had not run since August 27th.

After re-enabling the workflow, the containers were rebuilt and pushed. This updated the version of PHP 8.2 from ​beta 3 to ​RC3.

The timing also lines up. The ​first failure occurred approximately 1.5 hours after the containers were updated.

Something changed between 8.2.0beta3 and 8.2.0RC3 that's causing this failure.

[desrosj]

This is likely somehow being caused by ​https://github.com/php/php-src/issues/8805, which was ​included in PHP 8.2.0RC3. But still not sure how.

[oandregal]

For cross-reference, I've experienced this bug in this PR meant for WordPress 6.1 RC2, ​https://github.com/WordPress/wordpress-develop/actions/runs/3241375868/jobs/5314643672

There's also other failures. I'm not sure if all of them stem from the same issue, but wanted to share, so it's documented.

[jrf]

As a temporary measure to prevent PRs/patches/commits being blocked, I'd like to suggest adding a selective (PHP 8.2 only) $this->markTestSkipped() at the top of the test with a link to this issue.

This issue can then stay open to investigate this in more depth and determine whether this is a test issue or a src issue.

[desrosj]

56817.diff​ fixes the test failure in PHP 8.2, but will cause a failure in all other versions. But at least the problem is more clear now.

I think skipping temporarily is fine for now.

[SergeyBiryukov]

56817-skip.2.diff​ looks good to me as a temporary measure 👍

[SergeyBiryukov]

Since woff is not an extension supported by core by default, I guess the question for further discussion is, should it be up to plugins to adjust $types['woff'] = 'application/font-woff'; based on the PHP version, or should core provide backward compatibility for that?

[jrf]

@desrosj Please add a comment with a link to this ticket to the 56817-skip.2.diff patch. Other than that looks good.

[jrf]

Also - I previously identified a similar issue with mimetypes for webp on PHP 5.6 (and 7.0 I think). This was not solved yet and there is a draft PR which has more (debug) info available: ​https://github.com/WordPress/wordpress-develop/pull/2453

Note: all the test + documentation improvements in that PR have been committed in the mean time, it's only the debug commits which still remain. Hmm.. maybe I should rebase it at some point to make it clearer what I was debugging there.

[desrosj]

In 54508:

Tests: Temporarily skip WOFF file test on PHP 8.2.

A recent change to how WOFF files are processed in PHP 8.2 RC3 has caused a new test failure.

This temporarily marks the test_wp_check_filetype_and_ext_with_filtered_woff() test skipped until a deeper analysis can be performed.

Props SergeyBiryukov, jrf, desrosj, oandregal.
See #56817.

[desrosj]

Moving this to 6.2 as it's not something that will be fixed for 6.1.

[desrosj]

In 54509:

Tests: Revert unintentional change in [54508].

Unprops desrosj.
See #56817.

[SergeyBiryukov]

In 54724:

Tests: Temporarily skip WOFF file test on PHP 8.1.

A recent change to how WOFF files are processed in PHP 8.2 RC3 has caused a new test failure.

The tests was previously skipped on PHP 8.2, however, apparently after a fileinfo extension update, it started failing on PHP 8.1 too.

This commit adjusts the skipping condition to include PHP 8.1.

Follow-up to [54508], [54509].

See #56817.

[SergeyBiryukov]

In 54732:

Tests: Temporarily skip WOFF file test on PHP 8.1.

A recent change to how WOFF files are processed in PHP 8.2 RC3 has caused a new test failure.

The tests was previously skipped on PHP 8.2, however, apparently after a fileinfo extension update, it started failing on PHP 8.1 too.

This commit adjusts the skipping condition to include PHP 8.1.

Follow-up to [54508], [54509].

Reviewed by desrosj, SergeyBiryukov.
Merges [54724] to the 6.1 branch.
See #56817.

[costdev]

Changing the filter_mime_types_woff() callback in the tests to this makes the test pass on PHP 7.4/8.2:

<?php 
if ( PHP_VERSION_ID >= 80200 ) {
        $mimes['woff'] = 'font/woff';
} else {
        $mimes['woff'] = 'application/font-woff';
}
return $mimes;

However, as noted in comment:13, on PHP 8.1, success will depend on the version of the fileinfo extension.

What is the currently thinking about the desirable end goal?

• That the accurate PHP/fileinfo-dependent type is returned (application/font-woff vs font/woff)? Or
• That Core returns a consistent type across all PHP/fileinfo versions (application/font-woff)?

[SergeyBiryukov]

Replying to costdev:

What is the currently thinking about the desirable end goal?

• That the accurate PHP/fileinfo-dependent type is returned (application/font-woff vs font/woff)?

I think this would be the preferred result here. Let's adjust the tests as needed.

• Or that Core returns a consistent type across all PHP/fileinfo versions (application/font-woff)?

Returning a consistent type sounds good in theory, but if it was changed to font/woff upstream, introducing a discrepancy with that change doesn't seem worth it.

[costdev]

This ticket was discussed in the bug scrub and it was agreed to move this ticket to the Build/Test Tools component, to pursue a test-only change now and see if a slurce change is needed in a future cycle.

Trac ticket: https://core.trac.wordpress.org/ticket/56817

[SergeyBiryukov]

In 55462:

Tests: Adjust the expected mime type for WOFF fonts on PHP 8.1.12+.

As of PHP 8.1.12, which includes libmagic/file update to version 5.42, the expected mime type for WOFF files has changed to font/woff, so the type needs to be adjusted accordingly in wp_check_filetype_and_ext() tests.

References:

• ​php-src: #8805: finfo returns wrong mime type for woff/woff2 files
• ​PHP 8.1.12 changelog

Follow-up to [40124], [54508], [54509], [54724].

Props costdev, SergeyBiryukov.
Fixes #56817.

Merged in r55462.

[SergeyBiryukov]

In 55463:

Tests: Adjust the expected mime type for WOFF fonts on PHP 8.1.12+.

As of PHP 8.1.12, which includes libmagic/file update to version 5.42, the expected mime type for WOFF files has changed to font/woff, so the type needs to be adjusted accordingly in wp_check_filetype_and_ext() tests.

References:

• ​php-src: #8805: finfo returns wrong mime type for woff/woff2 files
• ​PHP 8.1.12 changelog

Follow-up to [40124], [54508], [54509], [54724].

Props costdev, SergeyBiryukov.
Merges [55462] to the 6.1 branch.
Fixes #56817.

[desrosj]

It looks like this should be backported to the ​6.0 and ​5.9 branches as well.

The PHP 8.1 job is allowed to fail in these branches, but since this doesn't involve any source code changes, I don't think it hurts to fix the test. Any objections to this?

[jrf]

No objection from me.

[SergeyBiryukov]

In 55497:

Tests: Adjust the expected mime type for WOFF fonts on PHP 8.1.12+.

As of PHP 8.1.12, which includes libmagic/file update to version 5.42, the expected mime type for WOFF files has changed to font/woff, so the type needs to be adjusted accordingly in wp_check_filetype_and_ext() tests.

References:

• ​php-src: #8805: finfo returns wrong mime type for woff/woff2 files
• ​PHP 8.1.12 changelog

Follow-up to [40124], [54508], [54509], [54724].

Props desrosj, jrf, costdev, SergeyBiryukov.
Merges [55462] to the 6.0 branch.
Fixes #56817.

[SergeyBiryukov]

In 55498:

Tests: Adjust the expected mime type for WOFF fonts on PHP 8.1.12+.

As of PHP 8.1.12, which includes libmagic/file update to version 5.42, the expected mime type for WOFF files has changed to font/woff, so the type needs to be adjusted accordingly in wp_check_filetype_and_ext() tests.

References:

• ​php-src: #8805: finfo returns wrong mime type for woff/woff2 files
• ​PHP 8.1.12 changelog

Follow-up to [40124], [54508], [54509], [54724].

Props desrosj, jrf, costdev, SergeyBiryukov.
Merges [55462] to the 5.9 branch.
Fixes #56817.