Skip to content

Comments

Fix PHPStan errors in tests#932

Merged
mukeshpanchal27 merged 3 commits intotrunkfrom
fix/phpstan-errors
Jan 12, 2024
Merged

Fix PHPStan errors in tests#932
mukeshpanchal27 merged 3 commits intotrunkfrom
fix/phpstan-errors

Conversation

@westonruter
Copy link
Member

@westonruter westonruter commented Jan 11, 2024

Summary

This fixes two PHPStan errors:

 ------ ---------------------------------------------- 
  Line   tests/admin/load-tests.php  
 ------ ---------------------------------------------- 
  94     Filter callback return statement is missing.  
 ------ ---------------------------------------------- 

The __return_empty_array is the appropriate callback for the perflab_active_modules filter, not __return_null.

 ------ ---------------------------------------------------- 
  Line   tests/modules/images/webp-uploads/helper-tests.php  
 ------ ---------------------------------------------------- 
  367    Filter callback return statement is missing.        
 ------ ---------------------------------------------------- 

In this case, the desire is to test when an invalid return value is supplied for the webp_uploads_upload_image_mime_transforms filter, so in this case the PHPStan error is ignored.

Checklist

  • PR has either [Focus] or Infrastructure label.
  • PR has a [Type] label.
  • PR has a milestone or the no milestone label.

@westonruter westonruter added [Type] Bug An existing feature is broken Infrastructure Issues for the overall performance plugin infrastructure no milestone PRs that do not have a defined milestone for release labels Jan 11, 2024
*/
public function it_should_return_default_transforms_when_filter_returns_non_array_type() {
/** @phpstan-ignore-next-line */
add_filter( 'webp_uploads_upload_image_mime_transforms', '__return_null' );
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strange. This is reported as an error for me locally but it isn't in GHA?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's also an error being reported in #898, but not in trunk.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strange issue 🤷🏻‍♂️

Copy link
Member

@felixarntz felixarntz Jan 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it maybe because of a different PHPStan strictness or other config? Or because of a different PHP version (which could lead to a diffferent PHPStan version)? 🤔

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a composer update fixed it!

@westonruter
Copy link
Member Author

On trunk currently if I try doing composer install, I get an error:

Verifying lock file contents can be installed on current platform.
Your lock file does not contain a compatible set of packages. Please run composer update.

  Problem 1
    - doctrine/instantiator is locked to version 2.0.0 and an update of this package was not requested.
    - doctrine/instantiator 2.0.0 requires php ^8.1 -> your php version (8.0.30) does not satisfy that requirement.
  Problem 2
    - doctrine/instantiator 2.0.0 requires php ^8.1 -> your php version (8.0.30) does not satisfy that requirement.
    - phpunit/phpunit 9.6.13 requires doctrine/instantiator ^1.3.1 || ^2 -> satisfiable by doctrine/instantiator[2.0.0].
    - phpunit/phpunit is locked to version 9.6.13 and an update of this package was not requested.

So that's why I did composer update.

@westonruter
Copy link
Member Author

However, in 5c009ba @felixarntz observed that doctrine/instantiator was getting downgraded, which shouldn't be happening. So I upgraded from PHP 8.0 to 8.1 and re-ran composer update and now this is no longer happening.

@mukeshpanchal27 mukeshpanchal27 merged commit e1c05c7 into trunk Jan 12, 2024
@mukeshpanchal27 mukeshpanchal27 deleted the fix/phpstan-errors branch January 12, 2024 04:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Infrastructure Issues for the overall performance plugin infrastructure no milestone PRs that do not have a defined milestone for release [Type] Bug An existing feature is broken

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants