Execute docker dependent tests with github actions #36647

jakzal · 2020-04-30T20:32:11Z

Q	A
Branch?	4.4
Bug fix?	no
New feature?	no
Deprecations?	no
Tickets	Fixes #36427
License	MIT
Doc PR	-

redis, memcached, rabbitmq and vulcain dependent tests moved to the github action
run on PHP 7.1 and 7.4 only
use the integration group for all tests that depend on docker services
do not exclude the integration group on Travis, but make sure tests that depend on docker services are skipped properly

.github/workflows/tests.yml

nicolas-grekas · 2020-05-01T08:31:13Z

Cool thanks.
I would suggest to target 4.4 for this. Docker on 3.4 is not really slowing us down. Then, on 4.4, I would suggest testing with only two versions: the lowest one (7.1) and the highest one. Adding more jobs wouldn't provide much extra safety, while it would slow the overall CI down. Let's save some CO² footprint.

jakzal · 2020-05-01T08:54:36Z

@nicolas-grekas sounds good. Regarding 7.1 the phpunit run fails with:

Remaining indirect deprecation notices (1)

  1x: The Doctrine\Common\Persistence\Mapping\ClassMetadata class is deprecated since doctrine/persistence 1.3 and will be removed in 2.0. Use \Doctrine\Persistence\Mapping\ClassMetadata instead.

    1x in DoctrineOrmTypeGuesserTest::requiredProvider from Symfony\Bridge\Doctrine\Tests\Form

nicolas-grekas · 2020-05-01T09:41:08Z

We skip a few deprecations by default:
https://github.com/symfony/symfony/blob/master/phpunit#L28

but the logic is this file doesn't reach this line currently.
We need to find a way to skip them in the new context.

(I wish Doctrine could fix its own deprecations, but that's another story...)

src/Symfony/Bundle/FrameworkBundle/Tests/Functional/CachePoolsTest.php

.travis.yml

jakzal · 2020-05-01T12:46:06Z

Rebased against 4.4 and applied feedback:

run on PHP 7.1 and 7.4 only
redis, memcached, rabbitmq moved to the github action
use the integration group for all tests that depend on docker services
do not exclude the integration group on Travis, but make sure tests that depend on docker services are skipped properly

Left to do:

vulcain?
higher branches once this is merged

Turns out that nothing really depends on mongodb running. The only test I could find actually mocks it (MongoDbSessionHandlerTest). There's quite a lot of code in .travis.yml that dealing with the mongodb extension. There's something using it in the master branch.

Nyholm

Im happy with this PR. I just had some minor comments

.github/workflows/tests.yml

src/Symfony/Component/Cache/Tests/Simple/AbstractRedisCacheTest.php

src/Symfony/Component/Cache/Tests/Adapter/AbstractRedisAdapterTest.php

jakzal · 2020-05-04T11:15:34Z

@Nyholm thank you so much for the review.

Vulcain tests are now migrated. They're skipped on 4.4 however.

This PR is now ready for a final review & merge.

jakzal · 2020-05-04T11:41:57Z

@nicolas-grekas it seems that some tests on Travis are failing, as they're not run against code in this branch? https://travis-ci.org/github/symfony/symfony/jobs/682876413#L8162

The same branch passes on my account: https://travis-ci.org/github/jakzal/symfony/builds/682876394

nicolas-grekas

some tests on Travis are failing, as they're not run against code in this branch

Correct. This will be fixed once we've merged up.

fabpot · 2020-05-04T12:47:24Z

Very impressive work @jakzal!

fabpot · 2020-05-04T12:48:59Z

Thank you @jakzal.

nicolas-grekas · 2020-05-04T14:16:50Z

@jakzal could you please have a look at master now?
I just merged 4.4 up to master and I removed integration tests from .travis.yml, but I did not add them back to the new system.

See this change:
1ae3e04#diff-354f30a63fb0907d4ad57269548329e3

We'd need to add tests for Messenger SQS, couchbase and kafka.

jakzal · 2020-05-04T14:40:26Z

on it

…ns (jakzal) This PR was merged into the 5.0 branch. Discussion ---------- [5.0] Use PHP 7.2 minimum in tests run with github actions | Q | A | ------------- | --- | Branch? | 5.0 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | Re #36647 | License | MIT | Doc PR | - Commits ------- 8b386f2 Use PHP 7.2 minimum in tests run with github actions

bendavies · 2020-05-05T14:56:44Z

.github/workflows/tests.yml

+                uses: actions/cache@v1
+                with:
+                    path: ${{ steps.composer-cache.outputs.directory }}
+                    key: ${{ matrix.php }}-composer-${{ hashFiles('**/composer.lock') }}


there is no composer.lock in this repo.

…ranch (jakzal) This PR was merged into the 5.1-dev branch. Discussion ---------- Configure services additionally required by the master branch | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | Re #36647 | License | MIT | Doc PR | - Additionally run tests for: * couchbase * sqs * kafka * mongodb (although currently skipped due to #36702) Before: Tests: 1893, Assertions: 5178, Skipped: 105. After: Tests: 2042, Assertions: 5407, Skipped: 120. Commits ------- dc7ac57 Configure services additionally required by the master branch

This PR was merged into the 3.4 branch. Discussion ---------- [Lock][Cache] Fix Redis tests | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | / | License | MIT | Doc PR | / In branch 3.4, travis started a redis-server container and tests run correctly. Starting from 4.4, the travis test suite don't start the server anymore, which is not an issue for 4.4's tests because tests are skiped. The issue is when branch 4.4 run the 3.4's test suite to check if component changes in 4.4 does not affect 3.4. in that case, the 3.4 tests suite is run without redise-server. see https://travis-ci.org/github/symfony/symfony/jobs/711062047 for example This PR replace the error handler (didn't worked) by catching the exception in a similar way than #36647 Commits ------- f524c85 Fix Redis tests

jakzal requested a review from jderusse as a code owner April 30, 2020 20:32

carsonbot added the Status: Needs Review label Apr 30, 2020

jakzal commented Apr 30, 2020

View reviewed changes

.github/workflows/tests.yml Outdated Show resolved Hide resolved

jakzal commented Apr 30, 2020

View reviewed changes

.github/workflows/tests.yml Outdated Show resolved Hide resolved

jakzal force-pushed the tests-on-github-actions branch 3 times, most recently from 0d0e2c1 to 52a84c5 Compare May 1, 2020 01:48

Plopix mentioned this pull request May 1, 2020

Move from Travis to Github Actions #36649

Closed

17 tasks

Nyholm reviewed May 1, 2020

View reviewed changes

.github/workflows/tests.yml Outdated Show resolved Hide resolved

nicolas-grekas reviewed May 1, 2020

View reviewed changes

src/Symfony/Bundle/FrameworkBundle/Tests/Functional/CachePoolsTest.php Outdated Show resolved Hide resolved

.travis.yml Outdated Show resolved Hide resolved

jakzal force-pushed the tests-on-github-actions branch from 6f0c3bd to 8258cf9 Compare May 1, 2020 12:37

jakzal requested review from dunglas, lyrixx, sroze and xabbuh as code owners May 1, 2020 12:37

jakzal changed the base branch from 3.4 to 4.4 May 1, 2020 12:37

jakzal changed the title ~~POC: Execute redis tests with github actions~~ Execute docker dependent tests with github actions May 1, 2020

nicolas-grekas added this to the 4.4 milestone May 1, 2020

Nyholm requested changes May 3, 2020

View reviewed changes

carsonbot added Status: Needs Work and removed Status: Needs Review labels May 3, 2020

Execute docker dependent tests with github actions

d710c1b

jakzal force-pushed the tests-on-github-actions branch from 4c1cdde to d710c1b Compare May 4, 2020 11:11

jakzal added Status: Needs Review and removed Status: Needs Work labels May 4, 2020

nicolas-grekas approved these changes May 4, 2020

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels May 4, 2020

fabpot approved these changes May 4, 2020

View reviewed changes

fabpot merged commit 0a7fa8f into symfony:4.4 May 4, 2020

jakzal deleted the tests-on-github-actions branch May 4, 2020 12:50

jakzal mentioned this pull request May 4, 2020

[5.0] Use PHP 7.2 minimum in tests run with github actions #36686

Merged

jakzal mentioned this pull request May 5, 2020

Configure services additionally required by the master branch #36704

Merged

bendavies reviewed May 5, 2020

View reviewed changes

jderusse mentioned this pull request Jul 23, 2020

[Lock][Cache] Fix Redis tests #37642

Merged

Uh oh!

Execute docker dependent tests with github actions #36647

Execute docker dependent tests with github actions #36647

Uh oh!

Conversation

jakzal commented Apr 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nicolas-grekas commented May 1, 2020

Uh oh!

jakzal commented May 1, 2020

Uh oh!

nicolas-grekas commented May 1, 2020

Uh oh!

Uh oh!

Uh oh!

jakzal commented May 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Nyholm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jakzal commented May 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jakzal commented May 4, 2020

Uh oh!

nicolas-grekas left a comment

Choose a reason for hiding this comment

Uh oh!

fabpot commented May 4, 2020

Uh oh!

fabpot commented May 4, 2020

Uh oh!

nicolas-grekas commented May 4, 2020

Uh oh!

jakzal commented May 4, 2020

Uh oh!

bendavies May 5, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jakzal commented Apr 30, 2020 •

edited

Loading

jakzal commented May 1, 2020 •

edited

Loading

jakzal commented May 4, 2020 •

edited

Loading