Skip to content

[e2e][parallel] Allow set custom config on Parallel#1620

Merged
TomasVotruba merged 9 commits intomainfrom
parallel-custom-config
Jan 4, 2022
Merged

[e2e][parallel] Allow set custom config on Parallel#1620
TomasVotruba merged 9 commits intomainfrom
parallel-custom-config

Conversation

@samsonasik
Copy link
Member

@samsonasik samsonasik commented Jan 4, 2022

@TomasVotruba this seems a reason why downgrade scoper build can't work via Parallel, it because it can't read config via --c because the script is target with --config:

directories=$(php bin/rector downgrade-paths --config build/config/config-downgrade.php --working-dir $BUILD_DIRECTORY --ansi)

This PR apply:

1. updated WorkerCommandLineFactory to append config for it based on option 'config' https://github.com/rectorphp/rector-src/pull/1620/files#diff-1c7bf20096eedf4dac69e487b7d1e3ad3f9cc122b98d54e145c2f463a2facab9

        if ($input->hasOption(Option::CONFIG)) {
            $workerCommandArray[] = '--config';
            $workerCommandArray[] = $input->getOption(Option::CONFIG);
        }

If no --config input, it will got config from rector.php.

2. Add e2e tests for its demo with pass custom config into input.

@samsonasik samsonasik force-pushed the parallel-custom-config branch from a3174e6 to d2daa53 Compare January 4, 2022 04:04
@samsonasik samsonasik force-pushed the parallel-custom-config branch from 5b9c64d to a1c215f Compare January 4, 2022 04:06
@samsonasik samsonasik marked this pull request as draft January 4, 2022 04:24
@samsonasik samsonasik marked this pull request as ready for review January 4, 2022 04:31
@samsonasik
Copy link
Member Author

It seems -c with '--config` got different result:

$input->hasOption(Option::CONFIG)

return true on -c passed, while not for --config

@samsonasik
Copy link
Member Author

I updated e2e script to test passed --config as well

@samsonasik
Copy link
Member Author

Fixed 🎉

@samsonasik
Copy link
Member Author

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

Next PR is to apply Option::PARALLEL to downgrade config.

Comment on lines -48 to 60
# run e2e test
-
run: php ../e2eTestRunner.php
- run: php ../e2eTestRunner.php -c custom/config/rector.php
working-directory: ${{ matrix.directory }}
if: ${{ matrix.directory == 'e2e/parallel-custom-config' }}

- run: php ../e2eTestRunner.php --config custom/config/rector.php
working-directory: ${{ matrix.directory }}
if: ${{ matrix.directory == 'e2e/parallel-custom-config' }}

- run: php ../e2eTestRunner.php
working-directory: ${{ matrix.directory }}
if: ${{ matrix.directory != 'e2e/parallel-custom-config' }}

Copy link
Member

Choose a reason for hiding this comment

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

We could put the "if" right into the matrix:

   matrix:
                php_version: ['8.1']
                actions:
                     -
                          directory: e2e/template-extends
                          runner: php ../e2eTestRunner.php -c custom/config/rector.php

Copy link
Member

Choose a reason for hiding this comment

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

I'm merging this PR to keep original fix and this syntax improvement separated 👍

@TomasVotruba TomasVotruba merged commit 6390862 into main Jan 4, 2022
@TomasVotruba TomasVotruba deleted the parallel-custom-config branch January 4, 2022 09:20
@TomasVotruba
Copy link
Member

Thank you 👍

@samsonasik
Copy link
Member Author

@TomasVotruba here the original PR why -c vs --config is needed in e2e tests parallel

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants