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

PHPUnit setup methods should be treated equally to constructors #107

Open
morozov opened this issue Feb 23, 2021 · 5 comments
Open

PHPUnit setup methods should be treated equally to constructors #107

morozov opened this issue Feb 23, 2021 · 5 comments
Labels
enhancement New feature or request

Comments

@morozov
Copy link

morozov commented Feb 23, 2021

PHPUnit executes setUp() and other @before methods on each test run, so with regards to the test (test*() and @test) methods they behave more or less like a constructor.

When analyzing test classes, Psalm and the PHPUnit plugin report the properties initialized in setup methods as not initialized in constructor.

$ composer show | grep psalm
psalm/plugin-phpunit                           0.15.1    Psalm plugin for PHPUnit
vimeo/psalm                                    4.5.2     A static analysis tool for find...
<?php

namespace Tests;

use PHPUnit\Framework\TestCase;
use stdClass;

class ObjectTest extends TestCase
{
    /** @var stdClass */
    private $object;

    protected function setUp(): void
    {
        $this->object = new stdClass();
    }

    public function testObject(): void
    {
        self::assertIsNotNull($this->object);
    }
}
$ psalm tests/ObjectTest.php

ERROR: PropertyNotSetInConstructor - tests/ObjectTest.php:11:13 - PropertyTests\ObjectTest::$object is not defined in constructor of Tests\ObjectTest and in any private or final methods called in the constructor (see https://psalm.dev/074)
    private $object;

While it's technically true, it's irrelevant for the test cases. By the time when the test method is invoked, the property will be initialized.

It is possible to rework such tests to initialize the object explicitly for each test method but this is less convenient, more verbose and in fact invalidates the PHPUnit setup approach.

@weirdan
Copy link
Member

weirdan commented Feb 23, 2021

An easy, albeit not 100% correct approach could be to suppress PropertyNotSetInConstructor when there's an initializer present (setUp() or @before), similar to how it's done for MissingConstructor.

@weirdan weirdan added the enhancement New feature or request label Feb 23, 2021
@morozov
Copy link
Author

morozov commented Feb 23, 2021

This is what I'm about to do at the project level but you're right, it will suppress other issues than this. E.g. sebastianbergmann/phpunit#4307 which still exists for some other properties like $backupStaticAttributes and $runTestInSeparateProcess (to be reported to PHPUnit).

@VincentLanglet
Copy link
Contributor

Would it be possible to introduce something like an annotation to tell psalm setup works like a __construct method ?

@devbanana
Copy link

Also having this issue. Has there been any progress on this?

@VincentLanglet
Copy link
Contributor

Also having this issue. Has there been any progress on this?

As you can see, the issue is still opened. Feel free to work on it if you look for progress ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants