rectorphp / rector-phpunit

Rector upgrade rules for PHPUnit
http://getrector.com
MIT License
61 stars 46 forks source link

[CodeQuality] Skip call parent::__construct() indirect parent on ConstructClassMethodToSetUpTestCaseRector #251

Closed samsonasik closed 10 months ago

samsonasik commented 10 months ago

Continue of PR:

this ensure when there is parent::__construct() call on indirect parent, it should be skipped as indirect use.

samsonasik commented 10 months ago

Init early should also be skipped:

final class SkipParameterUsed extends TestCase
{
    public function __construct($param)
    {
        $this->initEarly($param);
    }

    private function initEarly($param)
    {
        echo 'init';
    }
}

It currently cause invalid result:

 final class SkipParameterUsed extends TestCase
 {
-    public function __construct($param)
+    protected function setUp()
     {
         $this->initEarly($param);

which param removed but used in the caller.

samsonasik commented 10 months ago

15 errors because rules-tests was never executed https://github.com/rectorphp/rector-phpunit/pull/251#pullrequestreview-1618707993

Run vendor/bin/phpunit
  vendor/bin/phpunit
  shell: /usr/bin/bash -e {0}
  env:
    COMPOSER_ROOT_VERSION: dev-main
    COMPOSER_PROCESS_TIMEOUT: 0
    COMPOSER_NO_INTERACTION: 1
    COMPOSER_NO_AUDIT: 1
PHPUnit 10.3.3 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.1.23
Configuration: /home/runner/work/rector-phpunit/rector-phpunit/phpunit.xml

..................FFF.......F.F.........FF.FF..................  63 / 195 ( 32%)
...............F............F.................................. 126 / 195 ( 64%)
F..........................................FFF................. 189 / 195 ( 96%)
......                                                          195 / 195 (100%)

Time: 00:03.898, Memory: 146.50 MB

There were 4 PHPUnit test runner warnings:

1) No tests found in class "Rector\PHPUnit\Tests\AnnotationsToAttributes\Rector\ClassMethod\DependsAnnotationWithValueToAttributeRector\Source\AnotherTest".

2) Class DifferentNamespaceTest cannot be found in /home/runner/work/rector-phpunit/rector-phpunit/rules-tests/CodeQuality/Rector/Class_/AddSeeTestAnnotationRector/Source/DifferentNamespaceTest.php

3) Class SkipSimpleClassCommentTest cannot be found in /home/runner/work/rector-phpunit/rector-phpunit/rules-tests/CodeQuality/Rector/Class_/AddSeeTestAnnotationRector/Source/SkipSimpleClassCommentTest.php

4) Class SomeExistingTest cannot be found in /home/runner/work/rector-phpunit/rector-phpunit/rules-tests/CodeQuality/Rector/Class_/AddSeeTestAnnotationRector/Source/SomeExistingTest.php

--

There were 15 failures:

1) Rector\PHPUnit\Tests\AnnotationsToAttributes\Rector\ClassMethod\TestWithAnnotationToAttributeRector\TestWithAnnotationToAttributeRectorTest::test with data set #0
Failed on fixture file "some_fixture.php.inc"
Failed asserting that string matches format description.
--- Expected
+++ Actual
@@ @@

 final class SomeFixture extends TestCase
 {
-    #[\PHPUnit\Framework\Attributes\TestWith(['foo'])]
-    #[\PHPUnit\Framework\Attributes\TestWith(['bar'])]
+    /**
+     * @testWith ["foo"]
+     *           ["bar"]
+     */
     public function testOne(): void
     {
     }

/home/runner/work/rector-phpunit/rector-phpunit/vendor/rector/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:245
/home/runner/work/rector-phpunit/rector-phpunit/vendor/rector/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:169
/home/runner/work/rector-phpunit/rector-phpunit/rules-tests/AnnotationsToAttributes/Rector/ClassMethod/TestWithAnnotationToAttributeRector/TestWithAnnotationToAttributeRectorTest.php:16

2) Rector\PHPUnit\Tests\AnnotationsToAttributes\Rector\ClassMethod\TestWithAnnotationToAttributeRector\TestWithAnnotationToAttributeRectorTest::test with data set #1
Failed on fixture file "some_lower_cased.php.inc"
Failed asserting that string matches format description.
--- Expected
+++ Actual
@@ @@

 final class SomeLowerCased extends TestCase
 {
-    #[\PHPUnit\Framework\Attributes\TestWith(['rum'])]
-    #[\PHPUnit\Framework\Attributes\TestWith(['fim'])]
+    /**
+     * @testwith ["rum"]
+     * @testwith    ["fim"]
+     */
     public function testOne(): void
     {
     }

/home/runner/work/rector-phpunit/rector-phpunit/vendor/rector/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:245
/home/runner/work/rector-phpunit/rector-phpunit/vendor/rector/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:169
/home/runner/work/rector-phpunit/rector-phpunit/rules-tests/AnnotationsToAttributes/Rector/ClassMethod/TestWithAnnotationToAttributeRector/TestWithAnnotationToAttributeRectorTest.php:16

3) Rector\PHPUnit\Tests\AnnotationsToAttributes\Rector\ClassMethod\TestWithAnnotationToAttributeRector\TestWithAnnotationToAttributeRectorTest::test with data set #2
Failed on fixture file "the_most_complex_json.php.inc"
Failed asserting that string matches format description.
--- Expected
+++ Actual
@@ @@

 final class TheMostComplexJson extends TestCase
 {
-    #[\PHPUnit\Framework\Attributes\TestWith([['day' => 'monday', 'conditions' => 'sunny'], ['day', 'conditions']])]
+    /**
+     * @testWith [{"day": "monday", "conditions": "sunny"}, ["day", "conditions"]]
+     */
      public function testOne(): void
      {
      }

/home/runner/work/rector-phpunit/rector-phpunit/vendor/rector/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:245
/home/runner/work/rector-phpunit/rector-phpunit/vendor/rector/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:169
/home/runner/work/rector-phpunit/rector-phpunit/rules-tests/AnnotationsToAttributes/Rector/ClassMethod/TestWithAnnotationToAttributeRector/TestWithAnnotationToAttributeRectorTest.php:16

4) Rector\PHPUnit\Tests\CodeQuality\Rector\ClassMethod\RemoveEmptyTestMethodRector\RemoveEmptyTestMethodRectorTest::test with data set #0
Failed on fixture file "fixture.php.inc"
Failed asserting that string matches format description.
--- Expected
+++ Actual
@@ @@

 class SomeTest extends \PHPUnit\Framework\TestCase
 {
+    /**
+     * testGetTranslatedModelField method
+     *
+     * @return void
+     */
+    public function testGetTranslatedModelField()
+    {
+    }
 }

 ?>

/home/runner/work/rector-phpunit/rector-phpunit/vendor/rector/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:245
/home/runner/work/rector-phpunit/rector-phpunit/vendor/rector/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:169
/home/runner/work/rector-phpunit/rector-phpunit/rules-tests/CodeQuality/Rector/ClassMethod/RemoveEmptyTestMethodRector/RemoveEmptyTestMethodRectorTest.php:16

5) Rector\PHPUnit\Tests\CodeQuality\Rector\ClassMethod\ReplaceTestAnnotationWithPrefixedFunctionRector\ReplaceTestAnnotationWithPrefixedFunctionRectorTest::test with data set #1
Failed on fixture file "skip_already_prefixed_function.php.inc"
Failed asserting that string matches format description.
--- Expected
+++ Actual
@@ @@

 class SkipAlreadyPrefixedFunction extends \PHPUnit\Framework\TestCase
 {
-    /**
-     * @test
-     */
-    public function testOnePlusOneShouldBeTwo()
+    public function testTestOnePlusOneShouldBeTwo()
     {
         $this->assertSame(2, 1+1);
     }

/home/runner/work/rector-phpunit/rector-phpunit/vendor/rector/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:245
/home/runner/work/rector-phpunit/rector-phpunit/vendor/rector/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:169
/home/runner/work/rector-phpunit/rector-phpunit/rules-tests/CodeQuality/Rector/ClassMethod/ReplaceTestAnnotationWithPrefixedFunctionRector/ReplaceTestAnnotationWithPrefixedFunctionRectorTest.php:16

6) Rector\PHPUnit\Tests\CodeQuality\Rector\Class_\ConstructClassMethodToSetUpTestCaseRector\ConstructClassMethodToSetUpTestCaseRectorTest::test with data set #2
Failed on fixture file "skip_call_parent_construct.php.inc"
Failed asserting that string matches format description.
--- Expected
+++ Actual
@@ @@
 {
     private $someValue;

-    public function __construct()
+    protected function setUp()
     {
         $this->someValue = 1000;
-
-        parent::__construct();
     }
 }

/home/runner/work/rector-phpunit/rector-phpunit/vendor/rector/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:245
/home/runner/work/rector-phpunit/rector-phpunit/vendor/rector/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:169
/home/runner/work/rector-phpunit/rector-phpunit/rules-tests/CodeQuality/Rector/Class_/ConstructClassMethodToSetUpTestCaseRector/ConstructClassMethodToSetUpTestCaseRectorTest.php:16

7) Rector\PHPUnit\Tests\CodeQuality\Rector\Class_\ConstructClassMethodToSetUpTestCaseRector\ConstructClassMethodToSetUpTestCaseRectorTest::test with data set #3
Failed on fixture file "skip_parameter_used.php.inc"
Failed asserting that string matches format description.
--- Expected
+++ Actual
@@ @@

 final class SkipParameterUsed extends TestCase
 {
-    public function __construct($param)
+    protected function setUp()
     {
         $this->initEarly($param);
     }

/home/runner/work/rector-phpunit/rector-phpunit/vendor/rector/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:245
/home/runner/work/rector-phpunit/rector-phpunit/vendor/rector/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:169
/home/runner/work/rector-phpunit/rector-phpunit/rules-tests/CodeQuality/Rector/Class_/ConstructClassMethodToSetUpTestCaseRector/ConstructClassMethodToSetUpTestCaseRectorTest.php:16

8) Rector\PHPUnit\Tests\CodeQuality\Rector\Class_\PreferPHPUnitThisCallRector\PreferPHPUnitThisCallRectorTest::test with data set #0
Failed on fixture file "replace_none_static_skip_static_function.php.inc"
Failed asserting that string matches format description.
--- Expected
+++ Actual
@@ @@
 {
     public function testMe()
     {
-        $this->assertSame(1, 1);
+        self::assertSame(1, 1);
     }

     public static function testMe2()

/home/runner/work/rector-phpunit/rector-phpunit/vendor/rector/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:245
/home/runner/work/rector-phpunit/rector-phpunit/vendor/rector/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:169
/home/runner/work/rector-phpunit/rector-phpunit/rules-tests/CodeQuality/Rector/Class_/PreferPHPUnitThisCallRector/PreferPHPUnitThisCallRectorTest.php:16

9) Rector\PHPUnit\Tests\CodeQuality\Rector\Class_\PreferPHPUnitThisCallRector\PreferPHPUnitThisCallRectorTest::test with data set #1
Failed on fixture file "short_classes.php.inc"
Failed asserting that string matches format description.
--- Expected
+++ Actual
@@ @@
 {
     public function testMe()
     {
-        $this->assertSame(1, 1);
+        self::assertSame(1, 1);
     }
 }

/home/runner/work/rector-phpunit/rector-phpunit/vendor/rector/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:245
/home/runner/work/rector-phpunit/rector-phpunit/vendor/rector/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:169
/home/runner/work/rector-phpunit/rector-phpunit/rules-tests/CodeQuality/Rector/Class_/PreferPHPUnitThisCallRector/PreferPHPUnitThisCallRectorTest.php:16

10) Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\AssertIssetToSpecificMethodRector\AssertIssetToSpecificMethodRectorTest::test with data set #3
Failed on fixture file "skip_if_magic_method_isset_exists_in_parent.php.inc"
Failed asserting that string matches format description.
--- Expected
+++ Actual
@@ @@
     public function test()
     {
         $foo = new \Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\AssertIssetToSpecificMethodRector\Fixture\CustomIsset2();
-        $this->assertObjectHasAttribute('bar', $foo);
+        $this->assertTrue(isset($foo->bar));
     }
 }

/home/runner/work/rector-phpunit/rector-phpunit/vendor/rector/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:245
/home/runner/work/rector-phpunit/rector-phpunit/vendor/rector/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:169
/home/runner/work/rector-phpunit/rector-phpunit/rules-tests/CodeQuality/Rector/MethodCall/AssertIssetToSpecificMethodRector/AssertIssetToSpecificMethodRectorTest.php:16

11) Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\AssertTrueFalseToSpecificMethodRector\AssertTrueFalseToSpecificMethodRectorTest::test with data set #3
Failed on fixture file "fixture.php.inc"
Failed asserting that string matches format description.
--- Expected
+++ Actual
@@ @@
         $this->assertContains('...', ['...'], 'argument');

         $this->assertNotIsReadable('...');
-        $this->assertEmpty('...');
+        $this->assertTrue(empty('...'));
         $this->assertFileNotExists('...');
         $this->assertDirectoryExists('...');
         $this->assertFinite('...');

/home/runner/work/rector-phpunit/rector-phpunit/vendor/rector/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:245
/home/runner/work/rector-phpunit/rector-phpunit/vendor/rector/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:169
/home/runner/work/rector-phpunit/rector-phpunit/rules-tests/CodeQuality/Rector/MethodCall/AssertTrueFalseToSpecificMethodRector/AssertTrueFalseToSpecificMethodRectorTest.php:16

12) Rector\PHPUnit\Tests\PHPUnit60\Rector\ClassMethod\AddDoesNotPerformAssertionToNonAssertingTestRector\AddDoesNotPerformAssertionToNonAssertingTestRectorTest::test with data set #0
Failed on fixture file "fixture.php.inc"
Failed asserting that string matches format description.
--- Expected
+++ Actual
@@ @@

 class SomeClass extends \PHPUnit\Framework\TestCase
 {
-    /**
-     * @doesNotPerformAssertions
-     */
     public function test()
     {
         $nothing = 5;

/home/runner/work/rector-phpunit/rector-phpunit/vendor/rector/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:245
/home/runner/work/rector-phpunit/rector-phpunit/vendor/rector/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:169
/home/runner/work/rector-phpunit/rector-phpunit/rules-tests/PHPUnit60/Rector/ClassMethod/AddDoesNotPerformAssertionToNonAssertingTestRector/AddDoesNotPerformAssertionToNonAssertingTestRectorTest.php:16

13) Rector\PHPUnit\Tests\PHPUnit70\Rector\Class_\RemoveDataProviderTestPrefixRector\RemoveDataProviderTestPrefixRectorTest::test with data set #0
Failed on fixture file "fixture.php.inc"
Failed asserting that string matches format description.
--- Expected
+++ Actual
@@ @@
         $nothing = 5;
     }

-    public function provideData()
+    public function testProvideData()
     {
         return ['123'];
     }

/home/runner/work/rector-phpunit/rector-phpunit/vendor/rector/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:245
/home/runner/work/rector-phpunit/rector-phpunit/vendor/rector/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:169
/home/runner/work/rector-phpunit/rector-phpunit/rules-tests/PHPUnit70/Rector/Class_/RemoveDataProviderTestPrefixRector/RemoveDataProviderTestPrefixRectorTest.php:16

14) Rector\PHPUnit\Tests\PHPUnit70\Rector\Class_\RemoveDataProviderTestPrefixRector\RemoveDataProviderTestPrefixRectorTest::test with data set #1
Failed on fixture file "multiple_data_providers.php.inc"
Failed asserting that string matches format description.
--- Expected
+++ Actual
@@ @@
         $nothing = 5;
     }

-    public function provideData()
+    public function testProvideData()
     {
         return ['123'];
     }

-    public function nextProvideData2()
+    public function testNextProvideData2()
     {
         return ['123'];
     }

-    public function nextProvideData()
+    public function testNextProvideData()
     {
         return ['123'];
     }

/home/runner/work/rector-phpunit/rector-phpunit/vendor/rector/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:245
/home/runner/work/rector-phpunit/rector-phpunit/vendor/rector/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:169
/home/runner/work/rector-phpunit/rector-phpunit/rules-tests/PHPUnit70/Rector/Class_/RemoveDataProviderTestPrefixRector/RemoveDataProviderTestPrefixRectorTest.php:16

15) Rector\PHPUnit\Tests\PHPUnit70\Rector\Class_\RemoveDataProviderTestPrefixRector\RemoveDataProviderTestPrefixRectorTest::test with data set #2
Failed on fixture file "with_test_annotation.php.inc"
Failed asserting that string matches format description.
--- Expected
+++ Actual
@@ @@
         $nothing = 5;
     }

-    public function provideDataForWithATestAnnotation()
+    public function testProvideDataForWithATestAnnotation()
     {
         return ['123'];
     }

/home/runner/work/rector-phpunit/rector-phpunit/vendor/rector/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:245
/home/runner/work/rector-phpunit/rector-phpunit/vendor/rector/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:169
/home/runner/work/rector-phpunit/rector-phpunit/rules-tests/PHPUnit70/Rector/Class_/RemoveDataProviderTestPrefixRector/RemoveDataProviderTestPrefixRectorTest.php:16

FAILURES!
Tests: 195, Assertions: 261, Failures: 15, Warnings: 4.
samsonasik commented 10 months ago

@TomasVotruba @staabm the errors seems due to remove regex support:

which make the following code no longer works:

https://github.com/rectorphp/rector-phpunit/blob/2800f5c374cffc20cbdfb221ce033b46a3a8003d/rules/CodeQuality/Rector/Class_/PreferPHPUnitThisCallRector.php#L97-L99

The error detected after rules-tests registered to phpunit.xml above

https://github.com/rectorphp/rector-phpunit/pull/251#pullrequestreview-1618707993

samsonasik commented 10 months ago

Fixed 🎉

samsonasik commented 10 months ago

All checks have passed 🎉 @TomasVotruba I am merging it to have faster feedback to test ;)