[CI] Extend and enhance Travis build matrix #1035
Conversation
.travis.yml
Outdated
| env: | ||
| - SYMFONY_VERSION=4.0.* | ||
| - FLYSYSTEM_VERSION=1.0.* | ||
| - SIMPLE_PHPUNIT_PARAMETERS=" --stop-on-skipped" |
There was a problem hiding this comment.
i think this is not doing what you think it does. it stops execution when a skipped test is encountered, but not treat it as failure. see https://travis-ci.org/liip/LiipImagineBundle/jobs/326390446
it looks like a --fail-on-skipped is considered as unimportant by sebastian, so we could either maybe do a complicated thing with a test listener to detect skipped tests, or use an environment variable that we define on the line that runs phpunit that we look at in those tests and if its present fail instead of skip.
.travis.yml
Outdated
| - php: 7.2 | ||
| env: | ||
| - SYMFONY_VERSION=4.0.* | ||
| - FLYSYSTEM_VERSION=1.0.* |
There was a problem hiding this comment.
i like the approach we took here: https://github.com/php-http/HttplugBundle/blob/f034b168dde627de027c3d874410b10df2aa82af/.travis.yml#L56 - we define one dependencies variable and if any is set, have composer install those.
.travis.yml
Outdated
| - env: SYMFONY_VERSION=dev-master | ||
| - php: nightly | ||
|
|
||
|
|
||
| before_install: | ||
| - bash ./.travis/exec-before.bash | ||
|
|
||
| install: | ||
| - travis_retry composer require "symfony/symfony:${SYMFONY_VERSION}" $COMPOSER_FLAGS |
There was a problem hiding this comment.
while we are looking at these things, we could use dunglas/symfony-lock:^2 resp 3 and 4 to do this. the current code will hide missing dependencies on symfony components, as it installs all of symfony. the dunglas package conflicts with everything not in the right version instead, so the CI will show if there is a symfony component being used but not declared in require/require-dev.
|
@dbu yes, I hoped that --stop-on-skipped return an exit code greater than zero. At the moment I try to add the dependencies for the skipped tests locally. |
|
@dbu I fixed all tests. At the moment, Flysystem is included in composer.json so all are running. I now try to remove the dependency, what do you think? |
|
I found a lot of wrong namespaces like here @dbu what do you think? can we run php-cs-fixer on the 2.0 branch? |
There was a problem hiding this comment.
this looks good to me!
I now try to remove the dependency, what do you think?
the build looks great, no skipped tests in the build with flysystem 👍
you did not add it in composer.json if i see correctly.
php-cs-fixer
as far as i can see, Tests/ is included in styleci, and styleci should find such problems. is styleci not checking for namespace mismatches? i think tests should be correctly namespaced and autoloadable.
|
@dbu at the moment, it is included in require-dev see https://github.com/liip/LiipImagineBundle/blob/2.0/composer.json we have two possibilities:
what would you prefer? |
|
no @dbu, it is not ready at the moment. please give me one week more :) https://travis-ci.org/liip/LiipImagineBundle/jobs/327718645 |
|
@dbu now it looks better composer plugin prestissimo: build matrix:
In my opinion it is ready for feedback. |
|
@sebastianblum We intentionally limited the scope of the build matrix for This PR almost doubles the previous total time of Remember, there are lots more Symfony versions to come and we like to test all supported minor releases, so the matrix will naturally become larger later; we will need headroom so that can happen without getting to the 1 hour 16 minute total time it takes for the |
|
@robfrawley good point. I removed two tests, they are not necessary in my opinion. PHP 7.1 and PHP 7.2 without (symfony/symfony in a fixed version) are the most important tests. PHP coverage runs now only once, also php 7.3 check. Do you have more feedback? |
|
@sebastianblum Yeah; that looks better and brought us within two minutes of the current 2.x branch. As for any other feedback, I only really glanced at the build matrix and travis times; I'll take a broader look at the other changes later today and let you know if I notice anything. Thanks for the PR! |
robfrawley
added
State: Reviewing
|
@robfrawley After this pr is merged, I will add another check for php-cs-fixer to the Build Matrix in pr #1040 I can add it to an existing test or create a new one. What would you suggest? |
.travis.yml
Outdated
| - if [[ "$SYMFONY_VERSION" != "" ]]; then travis_retry composer require "symfony/symfony:${SYMFONY_VERSION}" --no-update $COMPOSER_FLAGS; fi; | ||
| - if [[ "$DEPENDENCIES" != "" ]]; then travis_retry composer require ${DEPENDENCIES} --no-update $COMPOSER_FLAGS; fi; | ||
| - if [[ "$STABILITY" != "" ]]; then composer config minimum-stability $STABILITY; fi | ||
| - if [[ "$ENABLE_CODE_COVERAGE" != "true" && "$TRAVIS_EVENT_TYPE" != "cron" ]]; then rm -f ~/.phpenv/versions/$(phpenv version-name)/etc/conf.d/xdebug.ini; fi; |
There was a problem hiding this comment.
Instead of rm -f ~/.phpenv/versions/$(phpenv version-name)/etc/conf.d/xdebug.ini you should use the phpenv configuration plugin instead; see "Preinstalled PHP extensions" (blog.travis-ci.com).
phpenv config-rm xdebug.ini
- fixed FlysystemTests - fix SymfonyFrameworkTest to use Kernel constants - use fly system depe in travis - test with --no-dev composer parameter - only install all symfony packages if version is specified - added matrix prefer-lowest - skip tests if browser-kit package is not available. - skip tests if package is not available. - skip doctrine/cache tests. - skip tests if symfony/form is missing - skip if twig/twig is missing. - added build to test dependencies - added missing use statement - skip if symfony/yaml is missing. - skip if doctrine/orm is missing. - Update .coveralls.yml - only 1 build with code coverage - removed hirak/prestissimo:^0.3 (no speed benefit) - running prefer-lowest also on the lowest php version - using phpenv to disable xdebug
robfrawley
changed the title
|
Thanks @sebastianblum! |
Try to extend the travis ci build matrix to fail on skipped tests