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

[Skipper] Handle provide direct relative path in Skipper #2921

Merged
merged 21 commits into from
Sep 11, 2022

Conversation

samsonasik
Copy link
Member

@samsonasik samsonasik commented Sep 10, 2022

Fixes rectorphp/rector#7467

Step to reproduce:

cd e2e/skip-direct-file-path 
composer install
php ../e2eTestRunnerPath.php src/Test.php 

@samsonasik
Copy link
Member Author

Fixed 🎉 /cc @AlexOstrovsky

@samsonasik
Copy link
Member Author

All checks have passed 🎉 @TomasVotruba it is ready for review.

@samsonasik
Copy link
Member Author

I added another failing test case for skip with path list only 187d685 , not by rector rule -> path.

with config:

<?php

declare(strict_types=1);

use Rector\Config\RectorConfig;
use Rector\Naming\Rector\ClassMethod\RenameParamToMatchTypeRector;
use Rector\Set\ValueObject\SetList;

return static function (RectorConfig $rectorConfig): void {
    $rectorConfig->paths([__DIR__ . '/src']);

    $rectorConfig->skip([
        __DIR__ . '/src',
    ]);

    $rectorConfig->sets([SetList::NAMING]);
};

then use command:

php ../e2eTestRunnerPath.php -c rector_skip_path_list.php src/Test.php

@samsonasik
Copy link
Member Author

Fixed 🎉

@samsonasik
Copy link
Member Author

samsonasik commented Sep 10, 2022

To run test locally:

cd e2e/skip-direct-file-path
composer install --ansi

# run e2e test direct path
php ../e2eTestRunnerPath.php src/Test.php

# run e2e test only directory
php ../e2eTestRunnerPath.php src

# run e2e test no path, look by rector.php
php ../e2eTestRunnerPath.php

# run e2e test with config use path lists
php ../e2eTestRunnerPath.php -c rector_skip_path_list.php src/Test.php

@samsonasik
Copy link
Member Author

All checks have passed 🎉 🎉 🎉 @TomasVotruba I think it is ready.

@TomasVotruba
Copy link
Member

The e2e is too heavy test here. This should be unit tested only.

@samsonasik
Copy link
Member Author

samsonasik commented Sep 10, 2022

@TomasVotruba the test is for relative path file location to check itself provided by command:

vendor/bin/rector process src/Test.php

that only happen when direct relative path provided in command itself.

which config can be:

    $rectorConfig->skip([
        RenameParamToMatchTypeRector::class => [__DIR__ . '/src'],
    ]);

Or

    $rectorConfig->skip([
         __DIR__ . '/src',
    ]);

so I think the way to test it is by using e2e test, Do we have example for unit test with path in command test?

@TomasVotruba
Copy link
Member

Do we have example for unit test with path in command test?

This is not neccesary. The path is only argument, that will be passed in configuration.

What is the input value that goes to a skipper component in case of running Rector?

vendor/bin/rector process src/Test.php?

That's what should be tested only in Skipper unit test.

@samsonasik
Copy link
Member Author

@TomasVotruba Ok, moved to unit test with relative path provided in test provider eca1683 👍

@samsonasik samsonasik changed the title [e2e] Handle Skipper on Fnmatcher by direct path command [Skipper] Handle provide direct relative path in Skipper Sep 10, 2022
@samsonasik
Copy link
Member Author

All checks have passed 🎉 @TomasVotruba I think it is ready now.

@TomasVotruba TomasVotruba merged commit effe4d3 into main Sep 11, 2022
@TomasVotruba TomasVotruba deleted the e2e-handle-skipper-direct-path branch September 11, 2022 11:59
@TomasVotruba
Copy link
Member

Thank you 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants