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

Support for Property Hooks in Test Doubles #5948

Closed
wants to merge 29 commits into from

Conversation

sebastianbergmann
Copy link
Owner

@sebastianbergmann sebastianbergmann commented Sep 11, 2024

PHP 8.4 introduces property hooks.

PHPUnit's test double functionality should support doubling property hooks and allow the configuration of return values for get hooks and expectations for set hooks.

interface I
{
    public string $property { get; set; }
}

Here is an example of what configuring test stubs and mock objects for the interface I shown above could look like:

$stub = $this->createStub(I::class);
$stub->method(PropertyHook::get('property'))->willReturn('value');

$mock = $this->createMock(I::class);
$mock->expects($this->once())->method(PropertyHook::set('property'))->with('value');
  • Interfaces with property hooks
  • Extendable concrete classes with non-final concrete property hooks

Support for abstract classes with abstract property hooks will not be implemented as getMockForAbstractClass() is deprecated and will be removed in PHPUnit 12.

@sebastianbergmann sebastianbergmann added type/enhancement A new idea that should be implemented feature/test-doubles Test Stubs and Mock Objects labels Sep 11, 2024
Copy link

codecov bot commented Sep 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.69%. Comparing base (d5e986f) to head (094d2f6).
Report is 85 commits behind head on main.

❌ We are unable to process any of the uploaded JUnit XML files. Please ensure your files are in the right format.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #5948      +/-   ##
============================================
+ Coverage     94.66%   94.69%   +0.02%     
- Complexity     6745     6779      +34     
============================================
  Files           712      716       +4     
  Lines         20354    20467     +113     
============================================
+ Hits          19269    19381     +112     
- Misses         1085     1086       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@davidbyoung
Copy link

I know this still in draft, but we should also support mocking abstract properties in abstract classes using similar functionality to the above.

@sebastianbergmann sebastianbergmann force-pushed the issue-5945/property-hooks branch 2 times, most recently from 7cfd5df to 5992c83 Compare September 15, 2024 06:34
@sebastianbergmann sebastianbergmann force-pushed the issue-5945/property-hooks branch 2 times, most recently from 771d2ed to 97375eb Compare September 15, 2024 17:29
@sebastianbergmann sebastianbergmann changed the title Doubling interfaces with property hooks Support for Property Hooks Sep 15, 2024
@sebastianbergmann sebastianbergmann self-assigned this Sep 15, 2024
@sebastianbergmann sebastianbergmann added this to the PHPUnit 11.5 milestone Sep 15, 2024
@sebastianbergmann sebastianbergmann changed the title Support for Property Hooks Support for Property Hooks in Test Doubles Sep 16, 2024
@sebastianbergmann
Copy link
Owner Author

@joseph-sentry Can you provide insight as to why we get We are unable to process any of the uploaded JUnit XML files. Please ensure your files are in the right format. in the comment made by Codecov? I assume (based on #5931) that you're with Sentry / Codecov, sorry in case I'm wrong.

@davidbyoung
Copy link

davidbyoung commented Sep 16, 2024

FYI I'm playing around with what you have so far (thank you for this!!), and found the following bug when mocking a nullable property getter:

use PHPUnit\Framework\TestCase;

interface Foo
{
    public ?Bar $bar { get; }
}

interface Bar {}

class MyTest extends TestCase
{
    public function testNullablePropertyGetter(): void
    {
        $bar = $this->createMock(\Bar::class);
        $foo = $this->createMock(\Foo::class);
        $foo->method('$bar::get')
            ->willReturn($bar);
        // Dummy assertion...
        $this->assertTrue(true);
    }
}

Running this test will give you the following error:

Method $bar::get may not return value of type MockObject_Bar_878f2f68, its declared return type is "?Bar"

@sebastianbergmann
Copy link
Owner Author

found the following bug when mocking a nullable property getter

Thank you, but that is expected. At least for me ;-) Jokes aside, yes, I know that not all bases are covered yet when it comes to type declarations.

@joseph-sentry
Copy link
Contributor

The codecov JUnit error is due to a parser error on our end where I assumed that a classname attribute would always exist for testcase elements, I made a fix for it on the parser side. I will have to integrate the latest version of the parser with our backend before you see it as fixed however.

@sebastianbergmann
Copy link
Owner Author

found the following bug when mocking a nullable property getter

Please try again with the current state of the branch.

@sebastianbergmann
Copy link
Owner Author

Please note that while ...

$foo->method('$bar::get')

... still works, you should use PropertyHook::get('bar') instead. Otherwise your test code depends on PHP implementation details that might change.

@davidbyoung
Copy link

found the following bug when mocking a nullable property getter

Please try again with the current state of the branch.

Updated to the latest commit in this branch, but now I'm back to the same error I got before you implemented the feature, which is

PHP Fatal error:  Class MockObject_Foo_a2ab9d44 contains 1 abstract method and must therefore be declared abstract or implement the remaining methods (Foo::$bar::get)

Running composer show phpunit/phpunit generates the following, which indicates I'm using the latest commit:

name     : phpunit/phpunit
descrip. : The PHP Unit Testing framework.
keywords : phpunit, testing, xunit
versions : * dev-issue-5945/property-hooks
released : 2024-09-17, today
type     : library
license  : BSD 3-Clause "New" or "Revised" License (BSD-3-Clause) (OSI approved) https://spdx.org/licenses/BSD-3-Clause.html#licenseText
homepage : https://phpunit.de/
source   : [git] https://github.com/sebastianbergmann/phpunit.git e3ce86cd281b656541857cffab9d187ffab608f0
dist     : [zip] https://api.github.com/repos/sebastianbergmann/phpunit/zipball/e3ce86cd281b656541857cffab9d187ffab608f0 e3ce86cd281b656541857cffab9d187ffab608f0
path     : /home/dyoung/PHPStormProjects/aphiria/vendor/phpunit/phpunit
names    : phpunit/phpunit

support
issues : https://github.com/sebastianbergmann/phpunit/issues
security : https://github.com/sebastianbergmann/phpunit/security/policy
source : https://github.com/sebastianbergmann/phpunit/tree/issue-5945/property-hooks

autoload
classmap
src/
files

requires
ext-dom *
ext-json *
ext-libxml *
ext-mbstring *
ext-xml *
ext-xmlwriter *
myclabs/deep-copy ^1.12.0
phar-io/manifest ^2.0.4
phar-io/version ^3.2.1
php >=8.2
phpunit/php-code-coverage ^11.0.6
phpunit/php-file-iterator ^5.1.0
phpunit/php-invoker ^5.0.1
phpunit/php-text-template ^4.0.1
phpunit/php-timer ^7.0.1
sebastian/cli-parser ^3.0.2
sebastian/code-unit ^3.0.1
sebastian/comparator ^6.1.0
sebastian/diff ^6.0.2
sebastian/environment ^7.2.0
sebastian/exporter ^6.1.3
sebastian/global-state ^7.0.2
sebastian/object-enumerator ^6.0.1
sebastian/type ^5.1.0
sebastian/version ^5.0.1

suggests
ext-soap To be able to generate mocks based on WSDL files

@sebastianbergmann
Copy link
Owner Author

sebastianbergmann commented Sep 17, 2024

now I'm back to the same error I got before you implemented the feature

You need the current state of PHP 8.4 (with php/php-src@d75a289 and php/php-src@2fce0bb).

@davidbyoung
Copy link

now I'm back to the same error I got before you implemented the feature

You need the current state of PHP 8.4 (with php/php-src@d75a289 and php/php-src@2fce0bb).

Ah, good catch. Compiled latest on master, and can confirm the issue I reported is fixed. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/test-doubles Test Stubs and Mock Objects type/enhancement A new idea that should be implemented
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants