[ci] we do not need to install flex globally anymore #1146
Conversation
jrushlow
changed the title
| @@ -135,9 +135,6 @@ jobs: | |||
| - name: "Add PHPUnit matcher" | |||
| run: echo "::add-matcher::${{ runner.tool_cache }}/phpunit.json" | |||
|
|
|||
| - name: "Require symfony/flex" | |||
| run: composer global require --no-progress --no-scripts --no-plugins symfony/flex | |||
There was a problem hiding this comment.
How does the SYMFONY_VERSION env var work?
Wasn't the intend to name it SYMFONY_REQUIRE to let flex restrict the allowed range?
There was a problem hiding this comment.
We grab the version specified in SYMFONY_VERSION during each test when we are creating the isolated Symfony app for a particular test.
e.g. Run MakeEntityTest against Symfony 5.4
SYMFONY_VERSION=5.4.x-dev
-
the "global"
composer installwill install MakerBundle on PHP 8.1 against thehighestdepends (No flex needed). The installed depends for MakerBundle could be say Symfony 6.2.x-dev -
vendor/bin/simple-phpunit --filter MakeEntityTestcreates a "test project" using flex w/ 5.4 dependencies for each of the tests defined inMakeEntityTestundertests/tmp/cache/maker_app_HASH_5.4.x-dev. Then runs the test against that app and its depends (.../cache/maker_app_HASH_5.4.x-dev/vendor).
Wasn't the intend to name it SYMFONY_REQUIRE to let flex restrict the allowed range?
I honestly cant recall.
Back when we were using TravisCI, installing flex globally gave a perf boost in CI. Atleast that was the intent anyway. With Github Actions, the test suites run in about 9 minutes whether we have flex installed globally or not.
There was a problem hiding this comment.
As you can see in the CI, it's broken: "PHP 8.1 + @5.4.x-dev highest deps" does not run 5.4 but 6.2.
That is true with or without this PR.
I think that instead of removing flex, this PR should fix it's usage by renaming SYMFONY_VERSION to SYMFONY_REQUIRE
There was a problem hiding this comment.
Even when we do that, 8d8a414, our "global" depends are still installing at 6.2 for the PHP 8.1 + @5.4.x-dev highest deps run: https://github.com/symfony/maker-bundle/runs/7211621223?check_suite_focus=true
The versions used in ~/composer.lock ("global") are not so much important. It's the flex projects we create for each "Make*Test" that use their own composer installed dependencies that matter.
When I replace SYMFONY_VERSION w/ SYMFONY_REQUIRE locally, e.g. SYMFONY_REQUIRE=5.4.x-dev vendor/bin/simple-phpunit --filter it_creates_a_new_class_basic - the flex app is no longer created using the 5.4 dependencies.
SYMFONY_REQUIRE:
SYMFONY_VERSION:
There was a problem hiding this comment.
Oh, got it, sorry for the noise then!
There was a problem hiding this comment.
no worries - Ryan & I were trying to figure out the reasoning for having flex installed globally yesterday and couldnt come up with anything other than a perf boost from back in the day.
The only time this gets tricky - we have a couple depends (and i cannot think of them off the top of my head) that are indirectly used in the tests from the "global" app will sometimes have a conflict. We generally have to introduce "legacy checks" in those instances to circumvent that. Though if i recall, these instances are limited to 1 or 2 tests.
Anywho, if there are ideas how we can make the tests a bit more DX friendly, I'm all ears...
|
Thanks Jesse! |
This is a better solution than #1145