rectorphp / rector-phpunit Goto Github PK
View Code? Open in Web Editor NEWRector upgrade rules for PHPUnit
Home Page: http://getrector.com
License: MIT License
Rector upgrade rules for PHPUnit
Home Page: http://getrector.com
License: MIT License
Currently, this class refactors all occurences of cover annotations, as described in the example. However, method-based attributes are not supported and never will be (see sebastianbergmann/phpunit#5175 and sebastianbergmann/phpunit#4502).
Applying this rule to a testsuite that uses method-based annotations will now result into a broken tests suite (Attribute "PHPUnit\Framework\Attributes\CoversClass" cannot target method (allowed targets: class)
).
I think there are two possible solutions:
Hey!
As the title suggests I'd like a rector the can turn an @test
annotation into a test
prefix for the test function.
Example:
/**
* @test
*/
public function onePlusOneIsTwo(): void {
$this->assertSame(2, 1+1);
}
Should turn into
/**
* @test
*/
public function testOnePlusOneIsTwo(): void {
$this->assertSame(2, 1+1);
}
The @test should be preserved and removed with a different rector in my opinion to keep this rector simple.
The implementation of WithConsecutiveRector
seems to be incorrect.
Before:
->withConsecutive( [1, 2], [3, 4] );
After:
->willReturnCallback(function () use ($matcher) { return match ($matcher->numberOfInvocations()) { 1 => [1, 2], 2 => [3, 4] };
The function withConsecutive
checks if a function is called with specific parameter. The replacement function willReturnCallback
only gets a function which determines the return value but doesn't checks the parameter. Furthermore the function returns the parameter.
I will create a PR with a (hopefully correct) solution.
Given the following code:
$expected = ['date' => new DateTimeImmutable('2022-10-31 12:32:33')];
$date = new DateTimeImmutable('2022-10-31 12:32:33');
$actual = compact('date');
self::assertEquals($expected, $actual);
The new version of the rule will change the assertion to assertSame
but this would lead to an error given this will check the two arrays contain exactly the same object.
There exists a rector rule called AddVoidReturnTypeWhereNoReturnRector
. It would be nice if we could add to rector-phpunit
to the PHPUNIT_CODE_QUALITY
a rule to add this to all classes extending from PHPUnits TestCase
class or any other class extending from PHPUnit TestClass. The AddVoidReturnTypeWhereNoReturnRector
is currently final.
What do you think the best way is to implement this. Currently we can not extend from AddVoidReturnTypeWhereNoReturnRector
to achieve this simple here. Another option could be that we extend the AddVoidReturnTypeWhereNoReturnRector
Rule in rector itself to filter by a class / interface and apply the rule only for that kind of classes, not sure if that kind of things make sense as a "General" Rector feature. That all rules could be limited to specific classes extending or implementing from a specific interface?
Subject | Details |
---|---|
Rector version | v0.10.6 |
Installed as | prefixed Rector |
Using the MigrateAtToConsecutiveExpectationsRector
rule. If a mock was configured without calling the expects()
method, an error would be threw.
Omitting expects()
while configuring a mock is valid syntax.
<?php
namespace Rector\PHPUnit\Tests\Rector\ClassMethod\MigrateAtToConsecutiveExpectationsRector\Fixture;
class Foo
{
public function someMethod()
{
}
}
final class SkipMissingExpects extends \PHPUnit\Framework\TestCase
{
protected $mock;
protected function setUp(): void
{
parent::setUp();
$this->mock = $this->createMock(Foo::class);
}
public function test(): void
{
$this->mock
->method('someMethod')
->willReturn(1);
$mock = $this->createMock(Foo::class);
$mock
->method('someOtherMethod')
->willReturn(1);
}
}
return static function (ContainerConfigurator $containerConfigurator): void {
$services = $containerConfigurator->services();
$services->set(\Rector\PHPUnit\Rector\ClassMethod\MigrateAtToConsecutiveExpectationsRector::class);
};
[ERROR] Could not process "..." file,
due to:
"Argument 1 passed to Rector\PHPUnit\NodeAnalyzer\ExpectationAnalyzer::isValidExpectsCall() must be an instance
of PhpParser\Node\Expr\MethodCall, instance of PhpParser\Node\Expr\PropertyFetch given, called in
vendor/rector/rector-prefixed/vendor/rector/rector-phpunit/src/NodeAnalyzer/ExpectationAnalyzer.php:49".
Since PHPUnit 10, it is not allowed to name abstract test classes with *Test
suffix, for example: abstract class MyBaseTest
should be renamed to abstract class MyBaseTestCase
, etc.
Many existing projects use this naming convention and need refactoring.
Please note that base *Test
and target *TestCase
suffixes should be customizable (users may choose different suffixes in phpunit.xml
using <directory suffix="Test.php">
).
For reference: sebastianbergmann/phpunit#4979
Hi, we have our directory structure in our project like this:
So for class App\Services\SomeService
we have test with FQCN App\Tests\Unit\Services\SomeService
, the Unit
being what breaks the default TestClassNameResolver
implementation.
Do you think it would be a good idea to allow the TestClassNameResolver
to be replaced via configuration by custom implementation, or perhaps by a callback, or should we try to find a different solution?
Thanks!
We have a big codebase in @sulu and lots of our testcases are not really analyzeable by phpstan as there are lot of false typehints for mocks (we have a very very big baseline). We are using prophecy
for mocking. And there are different false typehints in our codebase:
Variant A (class but didnt define mock):
class MyClassTest extends TestCase {
/**
* @var MyClass
*/
private $my;
public function setUp(): void
{
$this->my = $this->prophesize(MyClass::class);
}
}
Variant B (mock but didnt define class):
class MyClassTest extends TestCase {
/**
* @var ObjectProphecy
*/
private $my;
public function setUp(): void
{
$this->my = $this->prophesize(MyClass::class);
}
}
Variant C (mock or class (is still false but works sometimes)):
class MyClassTest extends TestCase {
/**
* @var ObjectProphecy|MyClass
*/
private $my;
public function setUp(): void
{
$this->my = $this->prophesize(MyClass::class);
}
}
Variant D (nothing defined):
class MyClassTest extends TestCase {
private $my;
public function setUp(): void
{
$this->my = $this->prophesize(MyClass::class);
}
}
Expected would be ObjectProphecy Generic with the class:
class MyClassTest extends TestCase {
/**
* @var ObjectProphecy<MyClass>
*/
private $my;
public function setUp(): void
{
$this->my = $this->prophesize(MyClass::class);
}
}
So I asked myself if rector could be used to provide a rule to analyze all $this->prophesize
calls and set the correct type for this classes. Think it would be a great rule which could be shipped with rector via a CodeQuality ruleset. Any hints are welcome how to achieve this :)
Hello I don't know if it is an issue or a expect behaviour.
But there is some functions in codeception that contain indirect assert like this:
https://github.com/Codeception/module-doctrine2/blob/master/src/Codeception/Module/Doctrine2.php#L845
Should we prevent this in this rule?
Because even I check inside of the function it could have assert just in low level like ->MethodA()->b()->c().....
or should I check one level only, if yes did you have any example, how to check this?
Based on class/method name and test suffix
Shouldn't this rule DataProviderArrayItemsNewlinedRector be placed in this repository instead of main repository?
๐
Rector 130b634b5e348554c3349ed3b3f52fa3d08bfb6e
PHPUnit 8.5
PHP Fatal error: Declaration of Rector\Testing\PHPUnit\PlatformAgnosticAssertions::assertStringEqualsFile($expectedFile, $actualString, $message = ''): void must be compatible with PHPUnit\Framework\Assert::assertStringEqualsFile(string $expectedFile, string $actualString, string $message = '', bool $canonicalize = false, bool $ignoreCase = false): void in /var/www/html/vendor/rector/rector/packages/Testing/PHPUnit/AbstractTestCase.php on line 41
Given the following code:
<?php
use Mockery;
use Mockery\Adapter\Phpunit\MockeryTestCase;
class ExampleTest extends MockeryTestCase
{
public function test_run(): void
{
$someMock = $this->getMock();
// ...
}
private function getMock()
{
return Mockery::mock('dep')
->expects('run')
->with(1, 2)
->getMock();
}
}
The GetMockRector
rector will alter it to be $someMock = $this->createMock();
, which is now invalid since in this context getMock
points to a private method, not the PHPUnit mocking method.
Another context relates to Mockery exceptions. E.g.
<?php
use Mockery\Adapter\Phpunit\MockeryTestCase;
use Mockery\Exception\NoMatchingExpectationException;
abstract class CustomizedTestCase extends MockeryTestCase
{
public function runTest(): void
{
try {
parent::runTest();
} catch (NoMatchingExpectationException $ex) {
/** @var Mockery\MockInterface $failedMock */
$failedMock = $ex->getMock();
// ...
}
}
}
Would be updated to $failedMock = $ex->createMock();
, which of course is invalid due to that method not existing on that exception type.
I wonder why the PreferPHPUnitSelfCallRector is not part of any setlist?
Addition to https://tomasvotruba.com/blog/how-to-remove-dead-mock-calls-from-phpunit-tests/
-$this->getMockBuilder(...)
- ->onlyMethods()
- ->getMock()
+$this->createMock(...);
Hello guys I want to build a rule to replace @testWith
to dataProvider
.
In case you have any rule that do something similar, please post here, it could help me to provide a rule for it ;)
1) tests/system/Email/EmailTest.php:138
---------- begin diff ----------
@@ @@
public function testDestructDoesNotThrowException()
{
- $email = $this->getMockBuilder(Email::class)
- ->disableOriginalConstructor()
- ->onlyMethods(['sendCommand'])
- ->getMock();
+ $email = $this->createMock(Email::class);
$email->method('sendCommand')
->willThrowException(new ErrorException('SMTP Error.'));
----------- end diff -----------
Applied rules:
* GetMockBuilderGetMockToCreateMockRector (https://github.com/lmc-eu/steward/pull/187/files#diff-c7e8c65e59b8b4ff8b54325814d4ba55L80)
But when I run the test, I got the error:
$ vendor/bin/phpunit tests/system/Email/EmailTest.php
PHPUnit 9.5.21 #StandWithUkraine
Runtime: PHP 8.0.20
Configuration: .../CodeIgniter4/phpunit.xml
........E 9 / 9 (100%)E 9 / 9 (100%)
Time: 00:00.358, Memory: 14.00 MB
There was 1 error:
1) CodeIgniter\Email\EmailTest::testDestructDoesNotThrowException
PHPUnit\Framework\MockObject\MethodNameNotConfiguredException: Method name is not configured
ERRORS!
Tests: 9, Assertions: 20, Errors: 1.
There is a rule provided in phpunit90 rule set that converts TestListener usages to TestHook usages. This was done because phpunit deprecated TestListener and provided somewhat similar functionality under their TestHook system. This has a couple gotchas where I think maybe removing this from the rule set may be a good idea?
First is the TestListener service provides wildly different functionality to the TestHook system, For example TestListener provides the actual test object whereas the TestHook only provides a string representation. Unfortunately this means that there is a large part of functionality available that a simple method signature change can't ever address.
I think the more important issue though is that TestHook is not a system they recommend to use nor is it actually staying long term, a new event system is under development for phpunit 10. You can see some of the discussion around that here sebastianbergmann/phpunit#3390.
The main reason I suggest this change is that its a potentially breaking change rule since the interface isn't at all the same but its also not even a system that will be in use going forward. I can make a PR to do this but wanted to discuss first what your thoughts were on this?
Just for reference the rule I"m talking about.
https://github.com/rectorphp/rector-phpunit/blob/main/config/sets/phpunit90.php#L13
I'm currently seeing some "fighting" between php-cs-fixer adding @coversNothing, and then rector removing it and adding the annotation (again).
I can suppress @coversNothing at php-cs-fixer, but annotations shouldn't be doubled up.
Would add
#[PHPUnit\Framework\Attributes\CoversNothing]
Imagine a Test class, let's call it A, extending some other (potentially) abstract Test class, B, that has defined example dataProvider methods and class A is using data providers from class B.
This makes DataProviderClassMethodFinder throw exception as the docblock referenced method is not contained in the same class.
The simplest solution would be to change exception throwing into a continue
step, disadvantage is that the extended class method won't be recognized as a data provider - but this is already happening for the base class that provides the data provider methods only ๐.
abstract class SomeAbstractTest extends TestCase
{
public function provideData()
{
yield [1];
}
}
final class SomeTest extends SomeAbstractTest
{
/**
* @dataProvider provideData()
*/
public function test()
{
}
}
Hello
is there anyway to apply rules by phpunit version
or just php version
?
As I can see in the doc this #[DataProvider] attribute is available in phpunit 10
only
9.x - https://docs.phpunit.de/en/9.6/writing-tests-for-phpunit.html#data-providers
10.x - https://docs.phpunit.de/en/10.4/writing-tests-for-phpunit.html#data-providers
\Rector\PHPUnit\PhpDoc\PhpDocValueToNodeMapper::mapGenericTagValueNode
is used for both the expected exception and the expected exception message when refactoring php doc tags into exception methods, which opportunistically tries to use class constants.
If there happens to be ::
in the exception message, then the exception message isn't treated as a string. Would it be possible to handle exception messages (and maybe regexps) as strings always? Otherwise invalid class constants can be written and generate invalid PHP.
I'm using Rector\PHPUnit\Set\PHPUnitSetList::PHPUNIT_100
Here is the basic hierarchy before the rector run:
namespace App\Test;
use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
abstract class AbstractWebTest extends WebTestCase
namespace App\Test\Controller;
use App\Test\AbstractWebTest;
abstract class AbstractControllerTest extends AbstractWebTest
namespace App\Test\Controller;
class ExampleControllerTest extends AbstractControllerTest
namespace App\Test;
use App\A;
use App\B;
use App\C;
use App\D;
use DateTime;
use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
abstract class AbstractWebTest extends WebTestCase
{
...
}
namespace App\Test;
use DateTime;
# why FQCN below instead of use statement?
abstract class AbstractWebTestCase extends \Symfony\Bundle\FrameworkBundle\Test\WebTestCase
{
# why are these down here?
# why a leading slash?
# what happened to the App\C and App\D imports, which are now direct FQCN references in the code below?
use \App\A;
use \App\B;
}
namespace App\Test\Controller;
use App\Test\AbstractWebTest;
abstract class AbstractControllerTest extends AbstractWebTest
namespace App\Test\Controller;
# while the class name suffix was updated from Test to TestCase, the AbstractWebTest was not updated to AbstractWebTestCase
abstract class AbstractControllerTestCase extends \Coe\Cee\Tests\Common\AbstractWebTest
This class was completely untouched by rector, when its extends
clause should have been changed to reflect the new name of the child abstract (AbstractControllerTest --> AbstractControllerTestCase
)
class ExampleControllerTest extends AbstractControllerTestCase
is there any way to add a blank like between trait and method?
Should apply
+ use ProphecyTrait;
+
But it is not adding this blank line after trait.
Should it be handled here or other tool should do it like phpcs?
Hi,
I noticed some @group โฆ
directives not being adapted and after a bit debugging I notice that in \Rector\PHPUnit\AnnotationsToAttributes\Rector\Class_\AnnotationWithValueToAttributeRector::refactor
the call to getTagsByName
returned DoctrineAnnotationTagValueNode
instead of GenericTagValueNode
.
But there is this check which makes it not pass:
if (!$desiredTagValueNode->value instanceof GenericTagValueNode) {
continue;
}
As to why: well, I've a (namespaces) class \Project\Models\Group
imported in some tests and I guess rector treats the class Group
special with @group
. However, in my case there's no relation here and since the default config in annotations-to-attributes.php
uses the FQCN anyway, there's no clash.
What can I do to still have such @group
being converted?
I'm talking about using this in CI, so I'm not looking for a one-time transformation here.
Thanks
Hello guys
I'm bumping phpunit to version 9 and I faced this issue in some places
$this->assertSame(15.28, $container->getUnsettledAmount());
//error:
//Failed asserting that 15.280000000000001 is identical to 15.28.
is there any rule to change
//from
$this->assertSame(15.28, $container->getUnsettledAmount());
//to
$this->assertEqualsWithDelta(15.28, $container->getUnsettledAmount(), 0.01);
Consider having a property that is typed as follows:
private ?string $myProperty;
Now using this property as the expected value in an assertEquals
and running AssertEqualsToSameRector
rule over it, will not change it to assertSame
even though it would be desired.
Is there any other rule that takes care of that or is that just something that wasn't considered yet?
running phpunit on 8a1a163 I get errors like on my mac
โ rector-phpunit git:(main) vendor/bin/phpunit
PHPUnit 10.1.0 by Sebastian Bergmann and contributors.
Runtime: PHP 8.1.18
Configuration: /Users/staabm/workspace/rector-phpunit/phpunit.xml
.............................F.F............................... 63 / 182 ( 34%)
............................................................... 126 / 182 ( 69%)
........................................................ 182 / 182 (100%)
Time: 00:18.524, Memory: 1.80 GB
There were 2 failures:
1) Rector\PHPUnit\Tests\Rector\ClassMethod\CreateMockToAnonymousClassRector\CreateMockToAnonymousClassRectorTest::test with data set #0
assert($startLine > 0)
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/vendor/ondrejmirtes/better-reflection/src/Reflection/ReflectionClass.php:189
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/vendor/ondrejmirtes/better-reflection/src/Reflection/ReflectionClass.php:261
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/src/Reflection/BetterReflection/BetterReflectionProvider.php:211
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/src/Reflection/ReflectionProvider/MemoizingReflectionProvider.php:59
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/MutatingScope.php:1102
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/MutatingScope.php:558
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/MutatingScope.php:744
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/MutatingScope.php:558
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:1473
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:567
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:362
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:531
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:331
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/packages/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php:374
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/packages/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php:227
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/src/Application/ChangedNodeScopeRefresher.php:95
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/src/Rector/AbstractRector.php:367
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/src/Rector/AbstractRector.php:260
/Users/staabm/workspace/rector-phpunit/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php:200
/Users/staabm/workspace/rector-phpunit/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php:114
/Users/staabm/workspace/rector-phpunit/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php:223
/Users/staabm/workspace/rector-phpunit/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php:114
/Users/staabm/workspace/rector-phpunit/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php:223
/Users/staabm/workspace/rector-phpunit/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php:91
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/src/PhpParser/NodeTraverser/RectorNodeTraverser.php:33
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/src/Application/FileProcessor.php:41
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/src/Application/FileProcessor/PhpFileProcessor.php:71
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/src/Application/ApplicationFileProcessor.php:127
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:188
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:154
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:128
/Users/staabm/workspace/rector-phpunit/tests/Rector/ClassMethod/CreateMockToAnonymousClassRector/CreateMockToAnonymousClassRectorTest.php:16
2) Rector\PHPUnit\Tests\Rector\ClassMethod\CreateMockToAnonymousClassRector\CreateMockToAnonymousClassRectorTest::test with data set #2
assert($startLine > 0)
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/vendor/ondrejmirtes/better-reflection/src/Reflection/ReflectionClass.php:189
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/vendor/ondrejmirtes/better-reflection/src/Reflection/ReflectionClass.php:261
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/src/Reflection/BetterReflection/BetterReflectionProvider.php:211
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/src/Reflection/ReflectionProvider/MemoizingReflectionProvider.php:59
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/MutatingScope.php:1102
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/MutatingScope.php:558
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/MutatingScope.php:744
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/MutatingScope.php:558
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:1473
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:567
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:362
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:531
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:331
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/packages/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php:374
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/packages/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php:227
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/src/Application/ChangedNodeScopeRefresher.php:95
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/src/Rector/AbstractRector.php:367
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/src/Rector/AbstractRector.php:260
/Users/staabm/workspace/rector-phpunit/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php:200
/Users/staabm/workspace/rector-phpunit/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php:114
/Users/staabm/workspace/rector-phpunit/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php:223
/Users/staabm/workspace/rector-phpunit/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php:114
/Users/staabm/workspace/rector-phpunit/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php:223
/Users/staabm/workspace/rector-phpunit/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php:91
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/src/PhpParser/NodeTraverser/RectorNodeTraverser.php:33
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/src/Application/FileProcessor.php:41
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/src/Application/FileProcessor/PhpFileProcessor.php:71
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/src/Application/ApplicationFileProcessor.php:127
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:188
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:154
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:128
/Users/staabm/workspace/rector-phpunit/tests/Rector/ClassMethod/CreateMockToAnonymousClassRector/CreateMockToAnonymousClassRectorTest.php:16
FAILURES!
Tests: 182, Assertions: 183, Failures: 2.
โ rector-phpunit git:(main) php -v
PHP 8.1.18 (cli) (built: Apr 12 2023 12:31:46) (NTS)
Copyright (c) The PHP Group
Zend Engine v4.1.18, Copyright (c) Zend Technologies
does anyone has an idea whats going on?
Desired behavior:
+ #[CoversClass(SomeService::class)]
class SomeServiceTest extends TestCase
{
}
Actually, I've already implemented it as a custom rule as we need it in Infection. It works great and covered by tests
infection/infection@03da557#diff-8a5ee371c08d2d066c7dfe04dea926a679e3ff5fb35ff93a228aa2a5c46e3699R65
But I would like to integrate it into rector/rector-phpunit
itself, as I'm pretty sure it will be useful for other teams that would like to automatically add #[CoversClass()]
to their test classes and automatically guess source file used as a value of this attribute. This is pretty convenient, and existing tools, including PHP-CS-Fixer, doesn't help with it.
But before creating a PR, I would like to know if it's going to be accepted to not waste time.
Also, this issue will popup in a search so even if it's not going to be accepted, one will have a link to a custom implementation.
Mockery provides a MockeryTestCase
base class that extends the default \PHPUnit\Framework\TestCase
that better integrates mockery into PHPUnit. One part of this is adding its expectations to PHPUnit's assertion count.
Currently the AddDoesNotPerformAssertionToNonAssertingTestRector
rector does not take this into consideration, resulting in the @doesNotPerformAssertions
being added to methods that technically have no core PHPUnit assertions, but do have Mockery assertions. For example, something like:
<?php
use \Mockery;
use \Mockery\Adapter\Phpunit\MockeryTestCase;
class ExampleTest extends MockeryTestCase
{
public function test_run(): void
{
$dep = Mockery::mock('dep')
->expects('run')
->with(1, 2)
->getMock();
$target = new class($dep) {
public function __construct(
private $dep
) { }
public function run(): void
{
$this->dep->run(1, 2);
}
};
$target->run();
}
}
Running PHPUnit against that would pass, running rector adds the annotation, then PHPUnit could fail, e.g. if used with the --fail-on-risky
flag since this test would be marked as risky.
Subject | Details |
---|---|
Rector version | ^0.17.1 |
Installed as | composer |
System error: "array_merge(): Argument rectorphp/rector#2 must be of type array, null given on line 104.
See https://getrector.com/demo/75a8fde7-5d55-4b3c-84a2-3df83076349c
<?php
use PHPUnit\Framework\TestCase;
final class DemoFileTest extends TestCase
{
public function testThrowsBadMethodCallException(): void
{
try {
throw new BadMethodCallException();
} catch (BadMethodCallException $badMethodCallException) {
self::assertSame($badMethodCallException::class, BadMethodCallException::class);
return;
}
self::fail('Exception was not thrown');
}
}
class BadMethodCallException extends \RuntimeException {
}
TryCatchToExpectExceptionRector
to run and remain unchanged.
I noticed that the most correct type hint for yield
would be Generator
but YieldDataProviderRector
adds Iterator
.
I think this is because of $classMethod->returnType = new FullyQualified('Iterator');
in YieldDataProviderRector
.
In rector, AddReturnTypeDeclarationFromYieldsRector
does work correctly and adds Generator.
It even worked in my test, if I remove the mentioned line which assigns Iterator
to returnType
and let rector run. It executes YieldDataProviderRector
& AddReturnTypeDeclarationFromYieldsRector
, which leads to function provideData(): Generator
Given this situation, I'm not sure what would be the correct fix. Looking forward to your help.
Well I faced a new issue
this rule: https://github.com/rectorphp/rector-phpunit/blob/main/docs/rector_rules_overview.md#testwithannotationtoattributerector, is using PHP attribute
I will check others functions that are still using php 8 and add MinPhpVersionInterface
Subject | Details |
---|---|
Rector version | dev-main of rector-phpunit |
Link to PR with failing test case: #16
The rector should ignore assertions that are making use of a class that implements the magic __isset function as PHPUnit uses reflection to check the property and does not call isset(xxx)
PHPUnit will deprecate using non-static methods as data providers: sebastianbergmann/phpunit@9caafe2
It would be great to have a rector that automatically turns data provider methods (i.e. methods referenced in @dataProvider
annotations or the equivalent attribute) into static methods (the extra safety would do that only if they are not called or overridden somewhere else)
Given the code:
/**
*
* @testWith ["foo"]
* ["bar"]
* ["baz"]
*/
public function testFoo(string $foo): void {}
Is currently refactored to:
/**
*
* #[TestWith('["Foo"]
* ["Bar"]
* ["Baz"]')]
*/
public function testFoo(string $foo): void {}
Expected:
/**
*
* #[TestWith(['Foo'])
* #[TestWith(['Bar'])
* #[TestWith(['Baz'])
*/
public function testFoo(string $foo): void {}
cf.
https://docs.phpunit.de/en/10.2/attributes.html#testwith
Consider test method and its related methods:
public function testSomething(): void
{
$this->mockSomething(); // This makes assertion internally by using `MockObject::expects()`
$this->runSomething(true);
}
private function mockSomething(): void
{
$this->something // This is mock prepared in setUp()
->expects(self::once())
->method('handle');
}
Currently AddDoesNotPerformAssertionToNonAssertingTestRector
adds @doesNotPerformAssertions
annotation to such tests, which is not true. Method itself does not have assertion calls, but other method(s) called within test makes assertions with MockObject::expects()
.
Subject | Details |
---|---|
Rector version | ^0.17.1 |
Installed as | composer |
See https://getrector.com/demo/3be3c0d9-dbea-4e1d-8c59-ea7fa84c4440
<?php
use PHPUnit\Framework\TestCase;
final class DemoFileTest extends TestCase
{
/**
* @var class-string<ExceptionInterface::class> $exception
* @dataProvider exceptionsDataProvider
**/
public function testExceptionIsInstanceOfExceptionInterface(string $exception): void
{
self::assertTrue(is_a($exception, ExceptionInterface::class, true));
}
public static function exceptionsDataProvider(): iterable
{
yield from [
ExceptionInterface::class => [ExceptionInterface::class]
];
}
}
interface ExceptionInterface {
}
AssertTrueFalseToSpecificMethodRector
to remain unchanged.
Subject | Details |
---|---|
Rector version | v0.10.6 |
Installed as | prefixed Rector |
Using the MigrateAtToConsecutiveExpectationsRector
rule. If a mock was configured in the setUp()
method and stored as a property, the at()
would be ignore and not refactored.
class Foo
{
public function someMethod()
{
}
}
final class HandlePropertyFetch extends \PHPUnit\Framework\TestCase
{
protected $mock;
protected function setUp(): void
{
parent::setUp();
$this->mock = $this->createMock(Foo::class);
}
public function test(): void
{
$this->mock
->expects($this->at(0))
->method('someMethod')
->willReturn(1);
$this->mock
->expects($this->at(1))
->method('someMethod')
->willReturn(2);
}
}
return static function (ContainerConfigurator $containerConfigurator): void {
$services = $containerConfigurator->services();
$services->set(\Rector\PHPUnit\Rector\ClassMethod\MigrateAtToConsecutiveExpectationsRector::class);
};
Test is not changed.
As described in sebastianbergmann/phpunit#5235 and sebastianbergmann/phpunit#5513 the CodeCoverageIgnore
attribute was a mistake and is now deprecated, therefore Rector should not try to replace annotation with the attribute.
I noticed a problem with how RemoveDataProviderTestPrefixRector that I think is related to how I'm using inheritance in tests for a project of mine.
I have a base class for tests with a test method that uses an illegally named data provider:
/**
* blah blah blah
*
* @dataProvider testExceptionsProvider
*/
public function testExceptions($filename, $message) {
The rector rule correctly updates the annotation to read:
/**
* blah blah blah
*
* @dataProvider exceptionsProvider
*/
public function testExceptions($filename, $message) {
However, the annotation in the base class is the only place that the data provider name is changed.
The rector rule seems to be getting "confused" because of how I'm relying on inheritance in these test classes. Specifically:
testExceptions()
is only declared in the base class.testExceptionsProvider()
is not declared in the base test class. It is declared in each of the test classes that extend the base class.For the record, I am seeing this on a Drupal project that I maintain: https://www.drupal.org/project/crossword/issues/3286830
Thanks!
It seems setMethods on mockBuilder is deprecated in PHPUNIT 10, this rule is currently under the codestyle setlist, but it seems it should be in the phpunit10 setlist? Am i missing something?
sebastianbergmann/phpunit#3687
RemoveSetMethodsMethodCallRector::class,
is the class im talking about.
i.e it works fine on class like this
use PHPUnit\Framework\TestCase;
class DateTest extends TestCase
{
}
but it does not work on
use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
class BoostTest extends WebTestCase
{
}
the issue seems to be that TestsNodeAnalyzer->isInTestClass
does not consider them as test class
I'm trying to dig more in the issue
Note: I'm not sure if of any importance but my rector is installed in a different directory than my actual projet (i.e I have a common "rector" tool for all my projects , with its own composer.json )
PHP Fatal error: Uncaught Error: Class "RectorPrefix20210513\Nette\Loaders\RobotLoader" not found in /home/runner/work/coding-standard/coding-standard/vendor/rector/rector/vendor/rector/rector-phpunit/src/RobotLoader/RobotLoaderFactory.php:14
A declarative, efficient, and flexible JavaScript library for building user interfaces.
๐ Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. ๐๐๐
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google โค๏ธ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.