Skip to content

Commit

Permalink
[Windows] Rework add windows support on tests CI (#5504)
Browse files Browse the repository at this point in the history
* [Windows] Rework add windows support on tests CI

* normalize file paths

* normalize file paths

* compare to false

* utilize PHP_EOL on FixtureSplitter for cross platform check

* fix WorkerCommandLineFactoryTest

* enable NewlineBeforeNewAssignSetRector

* normalize use \n for ending on DefaultDiffer

* [ci-review] Rector Rectify

* normalize \r\n

* normalize in ConsoleDiffer as well

* [ci-review] Rector Rectify

* normalize \n on FixtureSplitter

* fix replace

* fix FIlesFinderTest

* fix test on FilePathHelperTest

* fix test on SkippedPathsResolverTest

* udpat regex uri

* normalize PhpDocInfoTest

* check line ending PHP_EOL on BetterStandardPrinter

* use PHP_EOL on BetterPhpDocParser

* normalize PHP_EOL on PhpDocInfoPrinter

* normalize PHP_EOL on Differ

* normalize print docblock of InlineHTML

* normalize output result on print on PhpDocInfoPrinter

* normalize child node output

* normalize new line on BetterPhpDocParser

* normalize new line on BetterPhpDocParser

* [ci-review] Rector Rectify

* normalize path on FilePathHelper

* verify \n on PhpDocInfoPrinter

* comment

* fix unnecessary cast

* end with verify no need

* normalize content on FixtureFileUpdater

* fix path resolve

* remove cheap check

* fix BetterStandardPrinterTest

* Fix PhpDocInfoTest

* Revert "Fix PhpDocInfoTest"

This reverts commit 2622eb5.

* normalize file content

* split fix

* replace \r\n with \n of file content

* clean up normalize content

* clean up normalize content

* [ci-review] Rector Rectify

* clean up normalize content

* clean up normalize content

* Fix Differ

* roll

* normalize \n on ColorConsoleDiffFormatter

* normalize \n on diff

* normalize \n on diff

* display as original PHP_EOL on format final result

* normalize new line on DoctrineAnnotationDecorator

* split fix

* final touch: typo fix

* Final touch: normalize printFormatPreserving()

* final touch: normalize PhpDocInfoPrinter as well

* Revert "final touch: normalize PhpDocInfoPrinter as well"

This reverts commit be34a68.

* final touch: comment

* final touch: clarify comment

* try windows on e2e.yaml

* normalize diff on ConsoleDiffer and DefaultDiffer

* try normalize in diff

* trim

* trim

* flip

* roll

* roll

* try StrictUnifiedDiffOutputBuilder

* use \n on ColorConsoleDiffFormatter

* do not normalize too early on FixtureSplitter, let differ handle it

* do not normalize too early on FixtureSplitter, let differ handle it

* back DoctrineAnnotationDecorator

* back ConsoleDiffer

* back BetterStandardPrinter

* back FixtureFileUpdater

* back FixtureSplitter

* normalize new line on differ

* normalize new line on differ

* use PHP_EOL

* normalize both \r\n and \r to \n

* use DiffNormalizer on AbstractRectorTestCase

* use PHP_EOL

* Revert "use PHP_EOL"

This reverts commit ca05b97.

* Revert "use DiffNormalizer on AbstractRectorTestCase"

This reverts commit 9b74148.

* handle new line replace with space multi platform on PhpDocInfoPrinter

* multi platform new line replace keep definition on BetterPhpDocParser

* [ci-review] Rector Rectify

* use PHP_EOL on ColorConsoleDiffFormatter

* Fix TestModifyReprintTest and TagValueNodeReprintTest

* fix TestModifyReprintTest

* Fix CommentRemoverTest

* normalize FixtureSplitter

* rooll

* fix TagvalueNodeReprintTest

* use \n on ColorConsoleDiffFormatter

* use \n on ColorConsoleDiffFormatter

* rollback differ

* replace \r\n on diff output

* fix ColorConsoleDiffFormatterTest

* use "\r\n"

* refactor PhpDocInfoTest

* fix

* fix

* Revert "fix"

This reverts commit 21f106e.

* Revert "fix"

This reverts commit 3bf0992.

* Revert "refactor PhpDocInfoTest"

This reverts commit f08165f.

* fixing e2eTestRunner

* rollback e2e windows tset

---------

Co-authored-by: GitHub Action <actions@github.com>
  • Loading branch information
samsonasik and actions-user committed Jan 28, 2024
1 parent b107a16 commit 040b839
Show file tree
Hide file tree
Showing 21 changed files with 104 additions and 57 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ jobs:
strategy:
fail-fast: false
matrix:
os: [ubuntu-latest, macos-latest]
os: [ubuntu-latest, macos-latest, windows-latest]

runs-on: ${{ matrix.os }}
timeout-minutes: 3
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,6 @@ final class NewlineBeforeNewAssignSetRectorTest extends AbstractRectorTestCase
#[DataProvider('provideData')]
public function test(string $filePath): void
{
if ($this->isWindows()) {
$this->markTestSkipped('doesnt work on windows, see https://github.com/rectorphp/rector/issues/6572');
}

$this->doTestFile($filePath);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,6 @@ final class SensitiveHereNowDocRectorTest extends AbstractRectorTestCase
#[DataProvider('provideData')]
public function test(string $filePath): void
{
if ($this->isWindows()) {
$this->markTestSkipped('minor differences on windows, see https://github.com/rectorphp/rector/issues/6571');
}

$this->doTestFile($filePath);
}

Expand Down
18 changes: 14 additions & 4 deletions src/BetterPhpDocParser/PhpDocParser/BetterPhpDocParser.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,15 @@ final class BetterPhpDocParser extends PhpDocParser
{
/**
* @var string
* @see https://regex101.com/r/JOKSmr/1
* @see https://regex101.com/r/JDzr0c/1
*/
private const MULTI_NEW_LINES_REGEX = '#(\n\r|\n){2,}#';
private const NEW_LINE_REGEX = "#(?<new_line>\r\n|\n)#";

/**
* @var string
* @see https://regex101.com/r/JOKSmr/5
*/
private const MULTI_NEW_LINES_REGEX = '#(?<new_line>\r\n|\n){2,}#';

/**
* @param PhpDocNodeDecoratorInterface[] $phpDocNodeDecorators
Expand Down Expand Up @@ -120,7 +126,11 @@ public function parseTagValue(TokenIterator $tokenIterator, string $tag): PhpDoc
$endPosition = $tokenIterator->currentPosition();

if ($isPrecededByHorizontalWhitespace && property_exists($phpDocTagValueNode, 'description')) {
$phpDocTagValueNode->description = str_replace("\n", "\n * ", (string) $phpDocTagValueNode->description);
$phpDocTagValueNode->description = Strings::replace(
(string) $phpDocTagValueNode->description,
self::NEW_LINE_REGEX,
static fn (array $match): string => $match['new_line'] . ' * '
);
}

$startAndEnd = new StartAndEnd($startPosition, $endPosition);
Expand All @@ -130,7 +140,7 @@ public function parseTagValue(TokenIterator $tokenIterator, string $tag): PhpDoc
$phpDocTagValueNode->value = Strings::replace(
$phpDocTagValueNode->value,
self::MULTI_NEW_LINES_REGEX,
"\n"
static fn (array $match) => $match['new_line']
);
}

Expand Down
12 changes: 9 additions & 3 deletions src/BetterPhpDocParser/Printer/PhpDocInfoPrinter.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,12 @@ final class PhpDocInfoPrinter
*/
private const NEWLINE_WITH_ASTERISK = "\n" . ' *';

/**
* @var string
* @see https://regex101.com/r/ME5Fcn/1
*/
private const NEW_LINE_WITH_SPACE_REGEX = "# (?<new_line>\r\n|\n)#";

private int $tokenCount = 0;

private int $currentTokenPosition = 0;
Expand Down Expand Up @@ -175,7 +181,7 @@ private function printPhpDocNode(PhpDocNode $phpDocNode): string
$output .= ' */';
}

return str_replace(" \n", "\n", $output);
return Strings::replace($output, self::NEW_LINE_WITH_SPACE_REGEX, static fn (array $match) => $match['new_line']);
}

private function hasDocblockStart(string $output): bool
Expand Down Expand Up @@ -287,8 +293,8 @@ private function addTokensFromTo(string $output, int $from, int $to, bool $shoul

// skip extra empty lines above if this is the last one
if ($shouldSkipEmptyLinesAbove &&
\str_contains((string) $this->tokens[$from][0], PHP_EOL) &&
\str_contains((string) $this->tokens[$from + 1][0], PHP_EOL)
\str_contains((string) $this->tokens[$from][0], "\n") &&
\str_contains((string) $this->tokens[$from + 1][0], "\n")
) {
++$from;
}
Expand Down
2 changes: 1 addition & 1 deletion src/FileSystem/FilePathHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public function normalizePathAndSchema(string $originalPath): string
$normalizedPathParts = $this->normalizePathParts($pathParts, $scheme);

$pathStart = ($scheme !== self::SCHEME_UNDEFINED ? $scheme . '://' : '');
return $pathStart . $pathRoot . implode($directorySeparator, $normalizedPathParts);
return $this->normalizePath($pathStart . $pathRoot . implode($directorySeparator, $normalizedPathParts));
}

private function relativeFilePathFromDirectory(string $fileRealPath, string $directory): string
Expand Down
8 changes: 5 additions & 3 deletions src/Skipper/FileSystem/FnMatchPathNormalizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,13 @@ public function normalizeForFnmatch(string $path): string
}

if (\str_contains($path, '..')) {
/** @var string|false $path */
$path = realpath($path);
if ($path === false) {
/** @var string|false $realPath */
$realPath = realpath($path);
if ($realPath === false) {
return '';
}

return PathNormalizer::normalize($realPath);
}

return $path;
Expand Down
13 changes: 13 additions & 0 deletions src/Skipper/FileSystem/PathNormalizer.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<?php

declare(strict_types=1);

namespace Rector\Skipper\FileSystem;

final class PathNormalizer
{
public static function normalize(string $path): string
{
return str_replace('\\', '/', $path);
}
}
11 changes: 2 additions & 9 deletions src/Skipper/Fnmatcher.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,11 @@ final class Fnmatcher
{
public function match(string $matchingPath, string $filePath): bool
{
$normalizedMatchingPath = $this->normalizePath($matchingPath);
$normalizedFilePath = $this->normalizePath($filePath);
if (\fnmatch($normalizedMatchingPath, $normalizedFilePath)) {
if (\fnmatch($matchingPath, $filePath)) {
return \true;
}

// in case of relative compare
return \fnmatch('*/' . $normalizedMatchingPath, $normalizedFilePath);
}

private function normalizePath(string $path): string
{
return \str_replace('\\', '/', $path);
return \fnmatch('*/' . $matchingPath, $filePath);
}
}
5 changes: 4 additions & 1 deletion src/Skipper/Matcher/FileInfoMatcher.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
namespace Rector\Skipper\Matcher;

use Rector\Skipper\FileSystem\FnMatchPathNormalizer;
use Rector\Skipper\FileSystem\PathNormalizer;
use Rector\Skipper\Fnmatcher;
use Rector\Skipper\RealpathMatcher;

Expand All @@ -22,7 +23,9 @@ public function __construct(
*/
public function doesFileInfoMatchPatterns(string $filePath, array $filePatterns): bool
{
$filePath = PathNormalizer::normalize($filePath);
foreach ($filePatterns as $filePattern) {
$filePattern = PathNormalizer::normalize($filePattern);
if ($this->doesFileMatchPattern($filePath, $filePattern)) {
return true;
}
Expand All @@ -36,7 +39,7 @@ public function doesFileInfoMatchPatterns(string $filePath, array $filePatterns)
*/
private function doesFileMatchPattern(string $filePath, string $ignoredPath): bool
{
// in ecs.php, the path can be absolute
// in rector.php, the path can be absolute
if ($filePath === $ignoredPath) {
return true;
}
Expand Down
15 changes: 6 additions & 9 deletions src/Skipper/RealpathMatcher.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,24 +4,26 @@

namespace Rector\Skipper;

use Rector\Skipper\FileSystem\PathNormalizer;

final class RealpathMatcher
{
public function match(string $matchingPath, string $filePath): bool
{
/** @var string|false $realPathMatchingPath */
$realPathMatchingPath = realpath($matchingPath);
if (! is_string($realPathMatchingPath)) {
if ($realPathMatchingPath === false) {
return false;
}

/** @var string|false $realpathFilePath */
$realpathFilePath = realpath($filePath);
if (! is_string($realpathFilePath)) {
if ($realpathFilePath === false) {
return false;
}

$normalizedMatchingPath = $this->normalizePath($realPathMatchingPath);
$normalizedFilePath = $this->normalizePath($realpathFilePath);
$normalizedMatchingPath = PathNormalizer::normalize($realPathMatchingPath);
$normalizedFilePath = PathNormalizer::normalize($realpathFilePath);

// skip define direct path
if (is_file($normalizedMatchingPath)) {
Expand All @@ -35,9 +37,4 @@ public function match(string $matchingPath, string $filePath): bool

return str_starts_with($normalizedFilePath, $normalizedMatchingPath);
}

private function normalizePath(string $path): string
{
return \str_replace('\\', '/', $path);
}
}
1 change: 0 additions & 1 deletion src/Skipper/Skipper/PathSkipper.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ public function __construct(

public function shouldSkip(string $filePath): bool
{
$filePath = str_replace('\\', '/', $filePath);
$skippedPaths = $this->skippedPathsResolver->resolve();
return $this->fileInfoMatcher->doesFileInfoMatchPatterns($filePath, $skippedPaths);
}
Expand Down
8 changes: 2 additions & 6 deletions src/Testing/Fixture/FixtureSplitter.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,7 @@ public static function split(string $filePath): array
*/
public static function splitFixtureFileContents(string $fixtureFileContents): array
{
$posixContents = explode("-----\n", $fixtureFileContents);
if (isset($posixContents[1])) {
return $posixContents;
}

return explode("-----\r\n", $fixtureFileContents);
$fixtureFileContents = str_replace("\r\n", "\n", $fixtureFileContents);
return explode("-----\n", $fixtureFileContents);
}
}
25 changes: 22 additions & 3 deletions tests/BetterPhpDocParser/PhpDocInfo/PhpDocInfo/PhpDocInfoTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,18 @@ public function testReplaceTagByAnother(): void
$this->docBlockTagReplacer->replaceTagByAnother($phpDocInfo, 'test', 'flow');

$printedPhpDocInfo = $this->phpDocInfoPrinter->printFormatPreserving($phpDocInfo);
$this->assertStringEqualsFile(__DIR__ . '/Source/expected-replaced-tag.txt', $printedPhpDocInfo);
$printedPhpDocInfo = str_replace("\r\n", "\n", $printedPhpDocInfo);

$fileContent = str_replace(
"\r\n",
"\n",
FileSystem::read(__DIR__ . '/Source/expected-replaced-tag.txt')
);

$this->assertSame(
$fileContent,
$printedPhpDocInfo
);
}

public function testDoNotAddSpaseWhenAddEmptyString(): void
Expand All @@ -74,8 +85,16 @@ public function testDoNotAddSpaseWhenAddEmptyString(): void
$this->phpDocInfo->addPhpDocTagNode(new PhpDocTextNode('Some text'));

$printedPhpDocInfo = $this->phpDocInfoPrinter->printFormatPreserving($this->phpDocInfo);
$this->assertStringEqualsFile(
__DIR__ . '/Source/expected-without-space-when-add-empty-string.txt',
$printedPhpDocInfo = str_replace("\r\n", "\n", $printedPhpDocInfo);

$fileContent = str_replace(
"\r\n",
"\n",
FileSystem::read(__DIR__ . '/Source/expected-without-space-when-add-empty-string.txt')
);

$this->assertSame(
$fileContent,
$printedPhpDocInfo
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ private function doTestPrintedPhpDocInfo(string $filePath, string $annotationCla
private function splitListByEOL(string $content): array
{
$trimmedContent = trim($content);
return explode(PHP_EOL, $trimmedContent);
return explode("\n", $trimmedContent);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion tests/Console/Formatter/ColorConsoleDiffFormatterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public static function provideData(): Iterator
yield ["-old\n+new", __DIR__ . '/Source/expected/expected_old_new.txt'];

yield [
FileSystem::read(__DIR__ . '/Fixture/with_full_diff_by_phpunit.diff'),
str_replace("\r\n", "\n", FileSystem::read(__DIR__ . '/Fixture/with_full_diff_by_phpunit.diff')),
__DIR__ . '/Fixture/expected_with_full_diff_by_phpunit.diff',
];
}
Expand Down
5 changes: 4 additions & 1 deletion tests/FileSystem/FilesFinder/FilesFinderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,10 @@ public function testMultipleSuffixes(): void
$foundFilePaths = $this->filesFinder->findInDirectoriesAndFiles([__DIR__ . '/Source'], ['yaml', 'yml']);
$this->assertCount(2, $foundFilePaths);

$expectedFoundFilePath = [__DIR__ . '/Source/some_config.yml', __DIR__ . '/Source/other_config.yaml'];
$expectedFoundFilePath = [
__DIR__ . DIRECTORY_SEPARATOR . 'Source' . DIRECTORY_SEPARATOR . 'some_config.yml',
__DIR__ . DIRECTORY_SEPARATOR . 'Source' . DIRECTORY_SEPARATOR . 'other_config.yaml',
];

sort($foundFilePaths);
sort($expectedFoundFilePath);
Expand Down
12 changes: 12 additions & 0 deletions tests/Parallel/Command/WorkerCommandLineFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,12 @@ public function testSpacedMainScript(array $inputParameters, string $expectedCom
// running on macOS cause empty string after SPACED_DUMMY_MAIN_SCRIPT constant value removed on run whole unit test
// this ensure it works
$workerCommandLine = str_replace("rector' worker", "rector' '' worker", $workerCommandLine);

if (strncasecmp(PHP_OS, 'WIN', 3) === 0) {
$expectedCommand = str_replace("'", '"', $expectedCommand);
$workerCommandLine = str_replace("'", '"', $workerCommandLine);
}

$this->assertSame($expectedCommand, $workerCommandLine);
}

Expand Down Expand Up @@ -134,6 +140,12 @@ public function test(array $inputParameters, string $expectedCommand): void
// running on macOS cause empty string after main_script removed on run whole unit test
// this ensure it works
$workerCommandLine = str_replace("'main_script' worker", "'main_script' '' worker", $workerCommandLine);

if (strncasecmp(PHP_OS, 'WIN', 3) === 0) {
$expectedCommand = str_replace("'", '"', $expectedCommand);
$workerCommandLine = str_replace("'", '"', $workerCommandLine);
}

$this->assertSame($expectedCommand, $workerCommandLine);
}

Expand Down
4 changes: 2 additions & 2 deletions tests/PhpParser/Printer/BetterStandardPrinterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public function testAddingCommentOnSomeNodesFail(): void
$classMethod = new ClassMethod('run');
$classMethod->stmts = [$methodCallExpression];

$printed = $this->betterStandardPrinter->print($classMethod) . PHP_EOL;
$printed = str_replace("\n", PHP_EOL, $this->betterStandardPrinter->print($classMethod) . "\n");
$this->assertStringEqualsFile(
__DIR__ . '/Source/expected_code_with_non_stmt_placed_nested_comment.php.inc',
$printed
Expand All @@ -50,7 +50,7 @@ public function testStringWithAddedComment(): void
$string = new String_('hey');
$string->setAttribute(AttributeKey::COMMENTS, [new Comment('// todo: fix')]);

$printed = $this->betterStandardPrinter->print($string) . PHP_EOL;
$printed = str_replace("\n", PHP_EOL, $this->betterStandardPrinter->print($string) . "\n");
$this->assertStringEqualsFile(__DIR__ . '/Source/expected_code_with_comment.php.inc', $printed);
}

Expand Down
5 changes: 3 additions & 2 deletions tests/Skipper/FileSystem/FnMatchPathNormalizerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use Iterator;
use PHPUnit\Framework\Attributes\DataProvider;
use Rector\Skipper\FileSystem\FnMatchPathNormalizer;
use Rector\Skipper\FileSystem\PathNormalizer;
use Rector\Testing\PHPUnit\AbstractLazyTestCase;

final class FnMatchPathNormalizerTest extends AbstractLazyTestCase
Expand All @@ -31,7 +32,7 @@ public static function providePaths(): Iterator
yield ['*path/with/asterisk/begin', '*path/with/asterisk/begin*'];
yield ['path/with/asterisk/end*', '*path/with/asterisk/end*'];
yield ['*path/with/asterisk/begin/and/end*', '*path/with/asterisk/begin/and/end*'];
yield [__DIR__ . '/Fixture/path/with/../in/it', __DIR__ . '/Fixture/path/in/it'];
yield [__DIR__ . '/Fixture/path/with/../../in/it', __DIR__ . '/Fixture/in/it'];
yield [__DIR__ . '/Fixture/path/with/../in/it', PathNormalizer::normalize(__DIR__ . '/Fixture/path/in/it')];
yield [__DIR__ . '/Fixture/path/with/../../in/it', PathNormalizer::normalize(__DIR__ . '/Fixture/in/it')];
}
}

0 comments on commit 040b839

Please sign in to comment.