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.

Sign up for GitHub

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

Jump to bottom

DX: PsrAutoloadingFixerTest - do not build mock in data provider #7491

Merged
merged 2 commits into from
Dec 2, 2023
Merged

DX: PsrAutoloadingFixerTest - do not build mock in data provider #7491

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
52a0a1c
DX: PsrAutoloadingFixerTest - do not build mock in data provider
kubawerlos Dec 1, 2023
7cce4f9
Fix AbstractFixerTestCase::getTestFile
kubawerlos Dec 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
37 changes: 16 additions & 21 deletions tests/Fixer/Basic/PsrAutoloadingFixerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
namespace PhpCsFixer\Tests\Fixer\Basic;

use PhpCsFixer\Tests\Test\AbstractFixerTestCase;
use Prophecy\Prophet;

/**
* @author Graham Campbell <hello@gjcampbell.co.uk>
Expand Down Expand Up @@ -163,11 +162,13 @@ public static function provideFixNewCases(): iterable
* @dataProvider provideIgnoredCases
* @dataProvider provideAnonymousClassCases
*/
public function testFix(string $expected, ?string $input = null, ?\SplFileInfo $file = null, ?string $dir = null): void
public function testFix(string $expected, ?string $input = null, ?string $filepath = null, ?string $dir = null): void
{
if (null === $file) {
$file = self::getTestFile(__FILE__);
if (null === $filepath) {
$filepath = __FILE__;
}
$file = self::getTestFile($filepath);

if (null !== $dir) {
$this->fixer->configure(['dir' => $dir]);
}
Expand All @@ -177,13 +178,7 @@ public function testFix(string $expected, ?string $input = null, ?\SplFileInfo $

public static function provideFixCases(): iterable
{
$prophet = new Prophet();
$fileProphecy = $prophet->prophesize(\SplFileInfo::class);
$fileProphecy->willBeConstructedWith(['']);
$fileProphecy->getBasename('.php')->willReturn('Bar');
$fileProphecy->getExtension()->willReturn('php');
$fileProphecy->getRealPath()->willReturn(__DIR__.\DIRECTORY_SEPARATOR.'Psr'.\DIRECTORY_SEPARATOR.'Foo'.\DIRECTORY_SEPARATOR.'Bar.php');
$file = $fileProphecy->reveal();
$filepath = __DIR__.\DIRECTORY_SEPARATOR.'Psr'.\DIRECTORY_SEPARATOR.'Foo'.\DIRECTORY_SEPARATOR.'Bar.php';

yield [ // namespace with wrong casing
'<?php
Expand All @@ -194,7 +189,7 @@ class Bar {}
namespace Psr\foo;
class bar {}
',
$file,
$filepath,
__DIR__,
];

Expand All @@ -205,7 +200,7 @@ class Psr_Foo_Bar {}
'<?php
class Psr_fOo_bAr {}
',
$file,
$filepath,
__DIR__,
];

Expand All @@ -218,7 +213,7 @@ class Bar {}
namespace Psr\foo;
class bar {}
',
$file,
$filepath,
__DIR__,
];

Expand Down Expand Up @@ -297,7 +292,7 @@ class /* hi there */ blah /* why hello */ {}
class PsrAutoloadingFixer {}
',
null,
self::getTestFile(__DIR__.'/../../../src/Fixer/Basic/PsrAutoloadingFixer.php'),
__DIR__.'/../../../src/Fixer/Basic/PsrAutoloadingFixer.php',
];

yield [ // namespace partially matching directory structure with comment
Expand All @@ -306,7 +301,7 @@ class PsrAutoloadingFixer {}
class /* hi there */ PsrAutoloadingFixer {}
',
null,
self::getTestFile(__DIR__.'/../../../src/Fixer/Basic/PsrAutoloadingFixer.php'),
__DIR__.'/../../../src/Fixer/Basic/PsrAutoloadingFixer.php',
];

yield [ // namespace not matching directory structure
Expand All @@ -315,7 +310,7 @@ class /* hi there */ PsrAutoloadingFixer {}
class PsrAutoloadingFixer {}
',
null,
self::getTestFile(__DIR__.'/../../../src/Fixer/Basic/PsrAutoloadingFixer.php'),
__DIR__.'/../../../src/Fixer/Basic/PsrAutoloadingFixer.php',
];

yield [ // namespace partially matching directory structure with configured directory
Expand All @@ -327,7 +322,7 @@ class PsrAutoloadingFixer {}
namespace Foo\Bar\Baz\FIXER\Basic;
class PsrAutoloadingFixer {}
',
self::getTestFile(__DIR__.'/../../../src/Fixer/Basic/PsrAutoloadingFixer.php'),
__DIR__.'/../../../src/Fixer/Basic/PsrAutoloadingFixer.php',
__DIR__.'/../../../src/',
];

Expand All @@ -340,7 +335,7 @@ class /* hi there */ PsrAutoloadingFixer {}
namespace /* hi there */ Foo\Bar\Baz\FIXER\Basic;
class /* hi there */ PsrAutoloadingFixer {}
',
self::getTestFile(__DIR__.'/../../../src/Fixer/Basic/PsrAutoloadingFixer.php'),
__DIR__.'/../../../src/Fixer/Basic/PsrAutoloadingFixer.php',
__DIR__.'/../../../src/',
];

Expand All @@ -350,7 +345,7 @@ class /* hi there */ PsrAutoloadingFixer {}
class PsrAutoloadingFixer {}
',
null,
self::getTestFile(__DIR__.'/../../../src/Fixer/Basic/PsrAutoloadingFixer.php'),
__DIR__.'/../../../src/Fixer/Basic/PsrAutoloadingFixer.php',
__DIR__.'/../../../src/Fixer/Basic',
];

Expand Down Expand Up @@ -396,7 +391,7 @@ public static function provideIgnoredCases(): iterable
namespace Aaa;
class Bar {}',
null,
self::getTestFile($case),
$case,
], $cases);
}

Expand Down
9 changes: 8 additions & 1 deletion tests/Test/AbstractFixerTestCase.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,14 @@ final protected static function getTestFile(string $filename = __FILE__): \SplFi
{
static $files = [];

return $files[$filename] ?? $files[$filename] = new \SplFileInfo($filename);
return $files[$filename] ?? $files[$filename] = new class($filename) extends \SplFileInfo {
public function getRealPath(): string
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why to override this method?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because of the failed tests in the 1st commit.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry but that is not the answer. It is basically "kerad, check it yourself" :(

if test from single test class is failing, because of custom logic that was originally isolated to given test class, i do not believe we should move that to affect test cases for all the rules

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i do not believe we should move that to affect test cases for all the rules

what if it affects it positive way?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's affect in surprising way. we effectively override logic of built-in method to not return false on non-existing file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to not return false on non-existing file

non-existing, like a mock file?

{
return false !== parent::getRealPath()
? parent::getRealPath()
: $this->getPathname();
}
};
}

/**
Expand Down