Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Treat assertNotWPError() and assertWPError() as type-narrowing functions #30

Closed
johnbillion opened this issue Dec 11, 2020 · 12 comments · Fixed by #124
Closed

Treat assertNotWPError() and assertWPError() as type-narrowing functions #30

johnbillion opened this issue Dec 11, 2020 · 12 comments · Fixed by #124

Comments

@johnbillion
Copy link
Contributor

In the WordPress core test suite the following assertion methods are available:

  • $this->assertNotWPError()
  • $this->assertWPError()

It would be great if this extension treated these methods as type-narrowing functions, so the following code would not throw an error in PHPStan:

$terms = wp_get_post_terms( $post_id, $taxonomy );
$this->assertNotWPError( $terms );
$this->assertCount( 0, $terms );

Currently this triggers the following error because $terms is assumed to be array|WP_Error on the last line.

[phpstan] Parameter #2 $haystack of method PHPUnit\Framework\Assert::assertCount() expects Countable|iterable, array|WP_Error given.
@szepeviktor
Copy link
Owner

szepeviktor commented Dec 11, 2020

All right.
So you ask for parameter in WP_UnitTestCase_Base::assertNotWPError() never be of WP_Error and assertWPError() should do the opposite?

@szepeviktor
Copy link
Owner

Could you try installing phpstan/phpstan-phpunit?
https://github.com/phpstan/phpstan-phpunit#readme

@johnbillion
Copy link
Contributor Author

Yes the idea is that any parameter passed to assertNotWPError() cannot be a WP_Error in subsequent lines.

https://phpstan.org/writing-php-code/narrowing-types

I tried phpstan-phpunit but it had no effect, but I didn't play with it much.

@szepeviktor
Copy link
Owner

szepeviktor commented Dec 11, 2020

I hope we can tell PHPStan that assertNotWPError is really $this->assertNotInstanceOf( 'WP_Error', $actual, $message ); from https://github.com/phpstan/phpstan-phpunit/blob/master/src/Type/PHPUnit/Assert/AssertTypeSpecifyingExtensionHelper.php

@ondrejmirtes Could you give us directions?

@szepeviktor
Copy link
Owner

@johnbillion Could you point me to a simple WordPress plugin that has a unit test with assertNotWPError ?

@ondrejmirtes
Copy link
Contributor

Yes, you need to write and register a similar extension that phpstan-phpunit has: https://github.com/phpstan/phpstan-phpunit/blob/master/src/Type/PHPUnit/Assert/AssertMethodTypeSpecifyingExtension.php

@ondrejmirtes
Copy link
Contributor

Some info about them also here: https://phpstan.org/developing-extensions/type-specifying-extensions

@szepeviktor
Copy link
Owner

Thank you. Now we need those hands writing code.

@szepeviktor
Copy link
Owner

@monojp Please advise.
What would you do in this case?

@herndlm
Copy link
Contributor

herndlm commented Mar 2, 2021

I'm not an expert and never used those WP test helpers. But it looks they are at least not part of the official WP release. Are they WP internal? Does WP advise to use them for plugin developers and if the do - how? I quickly searched for it but did not find out much
UPDATE: I just realised that @johnbillion wants to use this in core :) Well I think the extension could be just added then.. I can take a look later today or tomorrow. If you can provide an example / link to a test case or so that would be perfect.

@szepeviktor
Copy link
Owner

@mono Here are your suspects
https://github.com/WordPress/wordpress-develop/blob/7d0cb86544dd28df8eaa0cb21f271e99e7e7cc47/tests/phpunit/includes/abstract-testcase.php#L598-L619
part of the WordPress Test Suite.

The problem is these method narrow down types.

@herndlm
Copy link
Contributor

herndlm commented Jul 30, 2021

@johnbillion are you still interested in this? If you could just provide me with an easy to work on example I can implement this next Wednesday, should not be hard. I just don't want to spend ages setting up the WP testing framework that I don't know anything about yet

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants