Added a good example of a travis file #8701

Nyholm · 2017-11-19T12:49:18Z

After a lot of discussions with people at Symfony con I came up with a "perfect" travis file.

See php-http/client-common#90 (comment) for info.
Btw, should I make a table explaining what we really test? (Like on the client-common PR)

javiereguiluz

Thanks for contributing this. I have two quick questions:

Shouldn't we recommend to use PHPUnit Bridge here instead of just PHPUnit?
Is it really necessary to create so many test jobs? Specifically, this looks like too much:

             - php: 5.5
             - php: 5.6
             - php: 7.0
             - php: 7.1
             - php: 7.2

Thanks!

weaverryan · 2017-11-20T15:00:59Z

bundles/best_practices.rst

+        - $TEST_COMMAND
+
+
+


few extra lines here

dunglas · 2017-11-20T15:06:23Z

bundles/best_practices.rst

+            - php: 7.2
+              env: DEPENDENCIES="symfony/lts:v2"
+            - php: 7.2
+              env: DEPENDENCIES="symfony/lts:v3"


Shouldn't we also add "symfony/lts": "^4@dev" to ensure that it works when using only 4.x packages (it's the default when using symfony/skeleton)?

normal jobs will already cover this, as 4.x will be the latest version anyway.

"symfony/lts": "^4@dev" will not forbid using a 3.x component anyway. It will only forbid using a 5.x one. The version 4 of the LTS package will be useless until we reach the 4.4 era (which is why it won't be stable until that time either)

dunglas · 2017-11-20T15:09:59Z

bundles/best_practices.rst

+        include:
+              # Minimum supported PHP and Symfony version
+            - php: 5.5
+              env: DEPENDENCIES="minimum" COVERAGE=true TEST_COMMAND="phpunit --coverage-text"


--prefer-stable --prefer-lowest should be tested with the latest version of PHP instead of the oldest (as done in Symfony), because most problems occur especially in this case (running old code on a cutting edge runtime).

stof · 2017-11-21T14:42:33Z

bundles/best_practices.rst

+
+              # Latest beta release
+            - php: 7.2
+              env: DEPENDENCIES="beta"


people should rather test latest dev than latest beta. Most libraries don't release beta releases (Twig for instance will not have a beta release before the stable one). The goal of this job is to catch incompatibilities with future releases as early as possible.
This job is not only about Symfony itself, but also about other libraries they depend on.

stof · 2017-11-21T14:42:41Z

bundles/best_practices.rst

@@ -476,3 +539,4 @@ Learn more
 .. _`Packagist`: https://packagist.org/
 .. _`choose any license`: http://choosealicense.com/
 .. _`valid license identifier`: https://spdx.org/licenses/
+.. _`Travis CI`: travis-ci.org


use https://travis-ci.org (btw, I think the current link would be broken, as it would be a relative link, and so would stay on symfony.com)

stof · 2017-11-21T14:44:28Z

bundles/best_practices.rst

+    install:
+        # To be removed when this issue will be resolved: https://github.com/composer/composer/issues/5355
+        - if [[ "$COMPOSER_FLAGS" == *"--prefer-lowest"* ]]; then travis_retry composer update --prefer-dist --no-interaction --prefer-stable --quiet; fi
+        - travis_retry composer update ${COMPOSER_FLAGS} --prefer-dist --no-interaction


travis_retry is useless IMO:

the download endpoints of Github are reliable now (4 years ago, they would fail randomly)

if the dependency resolution itself fails, retrying will only make the output less readable (due to displaying the failure 3 times). It will not allow it to succeed.

stof · 2017-11-21T14:50:06Z

bundles/best_practices.rst

+        - travis_retry composer update ${COMPOSER_FLAGS} --prefer-dist --no-interaction
+
+    script:
+        - $TEST_COMMAND


for shared bundle, I would recommend another test command as well, which I started adding recently (FOSUserBundle does not have it yet, but KnpMenuBundle does. See KnpLabs/KnpMenuBundle#351 for the reasoning):

composer validate --strict --no-check-lock

stof · 2017-11-21T14:52:34Z

bundles/best_practices.rst

+            - $HOME/.composer/cache/files
+    env:
+        global:
+            - TEST_COMMAND="phpunit"


Using phpunit as test command on Travis tends to create issue depending on your supported PHP versions: PHP 7 comes with PHPUnit 6, while PHP 5.3 images (for bundles still supporting it) come with a 4.8 version before the introduction of the namespaced classes.

stof · 2017-11-21T14:58:22Z

bundles/best_practices.rst

+
+A popular way to test open source bundles is by using `Travis CI`_. A good practice
+is to support at least the two latest LTS versions of symfony. One should also test
+test latest beta release. Here is a recommended configuration file (``.travis.yml``).


I would put a few other recommendations for their CI setup:

have a dev requirement on the symfony/phpunit-bridge "^3.3 || ^4" and run it in strict mode (which is the default), so that any usage of a deprecated API is catched. The only job where I'm allowing weak mode is the lowest job (because then, a version of your dependency may trigger a deprecation just because it went older than the release using the new API, which might be very hard to avoid in this job if it is a transitive dependency)

recommend enabling a Travis cron if your repository is not very active, so that failures on the unstable jobs due to an upstream change have a chance to occur before this change becomes stable. If the repo is very active, this is useless (because some activity-related build will also catch it), but most bundles are far less active than Symfony and will benefit from it (I generally use a monthly cron, but you could make it a weekly one instead)

sounds like an excellent idea to me (and was not aware thats possible). often, i notice such problems when somebody does a small contribution or even provides a trivial typo fix, and then wonder why that makes the build break. so far i manually restarted master to be sure, but that would automate that away and we would be aware of problems earlier.

weekly sounds like a good rate to me.

stof · 2017-11-21T15:01:21Z

bundles/best_practices.rst

+            - php: 7.2
+              env: DEPENDENCIES="symfony/lts:v2"
+            - php: 7.2
+              env: DEPENDENCIES="symfony/lts:v3"


normal jobs will already cover this, as 4.x will be the latest version anyway.

"symfony/lts": "^4@dev" will not forbid using a 3.x component anyway. It will only forbid using a 5.x one. The version 4 of the LTS package will be useless until we reach the 4.4 era (which is why it won't be stable until that time either)

stof · 2017-11-21T15:06:39Z

@javiereguiluz testing all PHP versions supported by your bundle is a must-have IMO (PHP is bad at following semver, so you cannot only test the low and high ones).

Btw, this gives me another recommendation: always have a job for the lowest PHP minor version you support. If you cannot have CI for it, you must remove it from your composer.json instead (so that Composer installs an older tested release instead of thinking the new untested release is fine).

stof · 2017-11-24T11:49:07Z

@Nyholm can you finish this PR ?

stof · 2017-11-24T11:49:29Z

Status: needs work

Nyholm · 2017-11-24T12:31:53Z

Yes. I will finish it this weekend. Thank you for all the feedback!

theofidry

👍 for it

theofidry · 2017-11-24T18:13:35Z

bundles/best_practices.rst

+.. code-block:: yaml
+
+    language: php
+    sudo: false


IIRC sudo is no longer required

I did some searching just now. I cannot find any references to this.. Can you provide a link?

Before sudo was enabled by default and there was a deprecation for a while stating if you want sudo you need to explicitly require it. You can see here that if you do need sudo, you should put sudo: required. Might be worth to double check what the actual default is though

Hm, Same page says:

Container-based with sudo: false

When specifying sudo: false, Travis CI runs each build in a container on a shared host via Docker. The container contents are a pristine copy of the Docker image, as guaranteed by Docker itself.

The advantage of running with sudo: false is dramatically reduced time between commit push to GitHub and the start of a job on Travis CI, which works especially well when one’s average job duration is under 3 minutes.

NOTE: The allowed list of packages that may be installed varies between Precise and Trusty, with the Precise list being considerably larger at the time of this writing. If the packages you need are not yet available on Trusty, it is recommended that you either target dist: precise or sudo: required depending on your needs.

It depends on the build distribution, see here. Newer builds, or builds explicitly using dist: trusty will ignore the sudo: setting.

theofidry · 2017-11-24T18:14:17Z

bundles/best_practices.rst

+            - php: 5.5
+            - php: 5.6
+            - php: 7.0
+            - php: 7.1


can't we drop 5.5-7.0?

Nyholm · 2017-11-25T08:51:58Z

Thank you for all the feedback. I've made some updates accordingly.

I will test this travis.yaml file in a php-http/client-common to see the changes "in the wild"

dbu

great idea!

is it correct to make the example with php 7.0, 7.1 and 7.2? or should we add 5.6 to the example because of the older symfony LTS versions?

dbu · 2017-11-28T07:35:38Z

bundles/best_practices.rst

+        fast_finish: true
+        include:
+              # Minimum supported PHP and Symfony version
+            - php: 7.2


should this be 7.0 ? or is the comment wrong?

Wrong comment. Thank you

dbu · 2017-11-28T07:36:36Z

bundles/best_practices.rst

+
+              # Test LTS versions
+            - php: 7.2
+              env: DEPENDENCIES="symfony/lts:^2"


is there somewhere a bit more documentation about the effect of the lts package so that we could link to that? people might be confused by this when there is no explanation.

Added a comment

dbu · 2017-11-28T07:37:21Z

bundles/best_practices.rst

+        - if [ "$DEPENDENCIES" = "dev" ]; then composer config minimum-stability dev; fi;
+        - if [[ $DEPENDENCIES == *"/"* ]]; then composer require --no-update $DEPENDENCIES; fi;
+
+    install:


don't forget to add vendor/bin/simple-phpunit install here (as you did on httplug) to have the installation output be collapsed by default.

xabbuh · 2017-11-29T09:35:27Z

bundles/best_practices.rst

+------
+
+A popular way to test open source bundles is by using `Travis CI`_. A good practice
+is to support at least the two latest LTS versions of symfony. One should also test


xabbuh · 2017-11-29T09:36:38Z

bundles/best_practices.rst

+        include:
+              # Minimum supported Symfony version
+            - php: 7.2
+              env: DEPENDENCIES="minimum" COVERAGE=true TEST_COMMAND="phpunit --coverage-text" SYMFONY_DEPRECATIONS_HELPER="weak"


Why phpunit here and not ./vendor/bin/simple-phpunit?

xabbuh · 2017-11-29T09:38:01Z

bundles/best_practices.rst

+            - php: 7.1
+            - php: 7.2
+
+              # Test LTS versions. This makes sure we do not use symfony packages with version greater


xabbuh · 2017-11-29T09:38:23Z

bundles/best_practices.rst

+
+        allow_failures:
+              # Latest beta is allowed to fail.
+            - php: 7.2


I would omit the PHP version to not have to change two places when a new PHP version is tested (which is likely to be forgotten)

xabbuh · 2017-11-29T09:39:06Z

bundles/best_practices.rst

+    before_install:
+        - composer require --no-update "symfony/phpunit-bridge:^3.3 || ^4"
+        - if [[ $COVERAGE != true ]]; then phpenv config-rm xdebug.ini || true; fi
+        - if [ "$DEPENDENCIES" = "minimum" ]; then COMPOSER_FLAGS="--prefer-stable --prefer-lowest"; fi;


Why not set $COMPOSER_FLAGS directly above instead?

xabbuh · 2017-11-29T09:39:28Z

bundles/best_practices.rst

+        - if [[ $COVERAGE != true ]]; then phpenv config-rm xdebug.ini || true; fi
+        - if [ "$DEPENDENCIES" = "minimum" ]; then COMPOSER_FLAGS="--prefer-stable --prefer-lowest"; fi;
+        - if [ "$DEPENDENCIES" = "dev" ]; then composer config minimum-stability dev; fi;
+        - if [[ $DEPENDENCIES == *"/"* ]]; then composer require --no-update $DEPENDENCIES; fi;


Maybe let's not use $DEPENDENCIES for two different things, but use two variables instead.

xabbuh · 2017-11-29T09:40:12Z

bundles/best_practices.rst

+        - composer validate --strict --no-check-lock
+        - $TEST_COMMAND
+
+When configuring travis you should also enable `Travis cron`_ to make sure your


"Travis CI" instead of "travis"?

Nyholm · 2017-11-29T10:01:39Z

I've made some updates, I need to check if this works properly.

Does it look good at least?

xabbuh · 2017-11-29T10:10:28Z

bundles/best_practices.rst

+
+        allow_failures:
+              # Latest beta is allowed to fail.
+            - php: 7.2


I would still remove the PHP version here

Sorry, I missed that.

xabbuh · 2017-11-29T10:10:29Z

bundles/best_practices.rst

+              env: STABILITY="dev"
+
+        allow_failures:
+              # Latest beta is allowed to fail.


in fact, we are not testing beta versions

stof · 2017-11-29T15:22:21Z

bundles/best_practices.rst

+        fast_finish: true
+        include:
+              # Minimum supported Symfony version with the latest PHP version
+            - env: COMPOSER_FLAGS="--prefer-stable --prefer-lowest" COVERAGE=true TEST_COMMAND="./vendor/bin/simple-phpunit --coverage-text" SYMFONY_DEPRECATIONS_HELPER="weak"


I recommand to put the coverage on a recent version instead of the lowest one.

Nyholm · 2018-01-13T11:19:53Z

@javiereguiluz and @weaverryan. Im done with this PR now (as I can tell from the reviews). I think it is ready to merge.

javiereguiluz

@Nyholm thanks! This is really nice and definitely useful for many people.

I made some rewords to focus on the general idea ("continuous integration") instead of the specific service used today to achieve that (Travis).

Nyholm · 2018-01-13T16:24:08Z

Thank you!

javiereguiluz · 2018-01-15T07:41:52Z

This is finally merged! Thanks Tobias.

…luz) This PR was submitted for the master branch but it was merged into the 4.0 branch instead (closes #8701). Discussion ---------- Added a good example of a travis file After a lot of discussions with people at Symfony con I came up with a "perfect" travis file. See php-http/client-common#90 (comment) for info. Btw, should I make a table explaining what we really test? (Like on the client-common PR) Commits ------- 93bdb86 Rewords 4bd8e65 Minor updates bc9a43f use ./ before vendor ae322b7 typo 1017897 Using better cache syntax 36ca90b Using PHPUNIT_FLAGS 18e4a18 Added back php-versions 05bf09d Moved coverage to PHP 7.2 with latest deps bfdd064 Fixed minor things 3e2b876 Fixed typos and created more variables a62bcb7 Added comment and install simple-phpunit deps 65f91df Use tilde or composer validate will never pass 179abb0 Updated according to feedback a281604 Typo 2418834 Added a good example travis file

dbu · 2018-01-15T07:46:28Z

awesome!

how to get to this page in the navigation? search finds it, but the "Bundles" point in the navigation does not find it: https://symfony.com/doc/bundles/ . and its not in https://symfony.com/doc/current/best_practices/index.html ...

javiereguiluz · 2018-01-15T08:08:01Z

@dbu in the sidebar navigation menu, click on the "Bundles" option (https://symfony.com/doc/current/bundles.html) and at the bottom of it you'll find all the related articles, including this one: https://symfony.com/doc/current/bundles/best_practices.html.

And the contents of this pull request are already available: https://symfony.com/doc/current/bundles/best_practices.html#continuous-integration

dbu · 2018-01-15T09:11:45Z

yeah i noticed its available :-)

i did not click on the guide for bundles, but on "Symfony Bundles" which is a list of existing bundles. maybe that list should have an intro that says this is about existing bundles you can use in your projects, and link to the guide for when you want to write your own bundle.

i don't find the source for https://symfony.com/doc/bundles/ in the symfony-docs repository however, otherwise i would have proposed something.

- class alias must be defined after the class is defined - explicit public declaration on services for compatibility between sf 3 and 4 - use conditional config deprecation for api secret - better travis according to symfony/symfony-docs#8701

carsonbot added the Status: Needs Review label Nov 19, 2017

javiereguiluz reviewed Nov 20, 2017

View reviewed changes

weaverryan requested changes Nov 20, 2017

View reviewed changes

bundles/best_practices.rst Outdated

- $TEST_COMMAND

Copy link

Member

weaverryan Nov 20, 2017

Choose a reason for hiding this comment

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

few extra lines here

dunglas reviewed Nov 20, 2017

View reviewed changes

stof suggested changes Nov 21, 2017

View reviewed changes

carsonbot added Status: Needs Work and removed Status: Needs Review labels Nov 24, 2017

theofidry reviewed Nov 24, 2017

View reviewed changes

carsonbot added Status: Needs Review and removed Status: Needs Work labels Nov 25, 2017

Nyholm mentioned this pull request Nov 26, 2017

Updated travis yml to properly test different symfony versions php-http/HttplugBundle#221

Merged

dbu reviewed Nov 28, 2017

View reviewed changes

dbu mentioned this pull request Nov 29, 2017

Update dependency for Symfony 4. php-http/stopwatch-plugin#4

Merged

xabbuh reviewed Nov 29, 2017

View reviewed changes

stof reviewed Nov 29, 2017

View reviewed changes

Nyholm added 14 commits January 13, 2018 12:18

Added a good example travis file

465d702

Typo

c990fea

Updated according to feedback

aa3283a

Use tilde or composer validate will never pass

6b805cf

Added comment and install simple-phpunit deps

5803fd0

Fixed typos and created more variables

5848ff6

Fixed minor things

f1a0439

Moved coverage to PHP 7.2 with latest deps

83daf5d

Added back php-versions

33aa6a7

Using PHPUNIT_FLAGS

3671cbd

Using better cache syntax

8613f1d

typo

e11b677

use ./ before vendor

5bacbd5

Minor updates

14f96a9

Nyholm force-pushed the travis branch from 8d33c15 to 14f96a9 Compare January 13, 2018 11:18

javiereguiluz added Status: Reviewed and removed Status: Needs Review labels Jan 13, 2018

Rewords

165bcf7

javiereguiluz approved these changes Jan 13, 2018

View reviewed changes

javiereguiluz added this to the 4.0 milestone Jan 13, 2018

javiereguiluz closed this Jan 15, 2018

Nyholm deleted the travis branch January 15, 2018 09:21

Uh oh!

Added a good example of a travis file #8701

Added a good example of a travis file #8701

Uh oh!

Conversation

Nyholm commented Nov 19, 2017

Uh oh!

javiereguiluz left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dunglas Nov 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dunglas Nov 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stof commented Nov 21, 2017

Uh oh!

stof commented Nov 24, 2017

Uh oh!

stof commented Nov 24, 2017

Uh oh!

Nyholm commented Nov 24, 2017

Uh oh!

theofidry left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Container-based with sudo: false

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Nyholm commented Nov 25, 2017

Uh oh!

dbu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

javiereguiluz left a comment •

edited

Loading

dunglas Nov 20, 2017 •

edited

Loading

dunglas Nov 20, 2017 •

edited

Loading