-
-
Notifications
You must be signed in to change notification settings - Fork 21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/docker for testing #3
Comments
Coverage remained the same at 53.925% when pulling 05aa0002f15e8bf8204889f7e54a6a8bb04cf7ee on TomHAnderson:feature/docker-for-testing into 6d45064 on zfcampus:master. Originally posted by @coveralls at zfcampus/zf-apigility-doctrine#308 (comment) |
@TomHAnderson I like that idea, I'll have a look on it later on ! Thanks a lot! Originally posted by @michalbundyra at zfcampus/zf-apigility-doctrine#308 (comment) |
Before you approve this, if you choose, see @Ocramius comment on the linked doctrine PR. Originally posted by @TomHAnderson at zfcampus/zf-apigility-doctrine#308 (comment) |
If you plan to use docker in the examples, then make sure that the project eats its own dogfood and has a pipeline entry (in CI) that uses the advertised docker setup for testing too. It is extremely painful for users to jump into a project and find out-of-sync startup scripts and such 😱 Originally posted by @Ocramius at zfcampus/zf-apigility-doctrine#308 (comment) |
This has been rewritten to a minimal Docker footprint Originally posted by @TomHAnderson at zfcampus/zf-apigility-doctrine#308 (comment) |
Includes a fix to a unit test which changed order. Originally posted by @TomHAnderson at zfcampus/zf-apigility-doctrine#308 (comment) |
@Ocramius was right, as usual, so I added docker-compose to travis testing. I had to use a script because I didn't know how to escape a pipe to run all the commands on the same command line. Docker is php:latest so all 7.2 tests also test Docker with the same dependencies as the test. Originally posted by @TomHAnderson at zfcampus/zf-apigility-doctrine#308 (comment) |
Tested failed unit tests in docker fail the build tested with zfcampus/zf-apigility-doctrine#310 Originally posted by @TomHAnderson at zfcampus/zf-apigility-doctrine#308 (comment) |
Starting with @thomasvargiu suggestions I've engineered the tests to run Docker for every test. You can compose a docker container with
All travis tests use the current testing version of PHP in the Docker tests. Originally posted by @TomHAnderson at zfcampus/zf-apigility-doctrine#308 (comment) |
This comment has been buried under Show outdated so I've copied it here. I can create a pure language:none travis/docker build. But when language:php then I can run composer and it will validate the composer.json against the php binary. However the mapped directory is not mapped at the time a container (php) is built. So I can't run composer as part of the At this time I do not see another way around this. If I go the route of creating custom composer commands in entrypoint.sh it will complicate that section of the Dockerfile (not a bad thing) and the rest of the configuration will be clean. Looking for ideas etc. Thanks! Originally posted by @TomHAnderson at zfcampus/zf-apigility-doctrine#308 (comment) |
re: time spent vs future time I'm on nobody's clock but my own and I feel this work is bringing a new facet to travis builds of complex requirements which can be propagated through PHPdom and I'll be happy to provide that shine. Originally posted by @TomHAnderson at zfcampus/zf-apigility-doctrine#308 (comment) |
@TomHAnderson you should run every PHP command (even composer) inside the container, so you can do something like this (not tested): before_install:
# Passing a build argument will build the container enabling xdebug extension (handle it in Dockerfile)
- if [[ $TEST_COVERAGE == 'true' ]]; then DOCKER_BUILD_ARGS="$DOCKER_BUILD_ARGS --build-arg ENABLE_XDEBUG=true" ; fi
- docker-compose build --build-arg PHP_VERSION=$PHP_VERSION $DOCKER_BUILD_ARGS php
install:
# Run composer install from the container without "--ignore-platform-reqs" (container should be ok)
- travis_retry docker-compose run --rm php composer $COMPOSER_ARGS
- if [[ $LEGACY_DEPS != '' ]]; then travis_retry docker-compose run --rm php composer update $COMPOSER_ARGS --with-dependencies $LEGACY_DEPS ; fi
- if [[ $DEPS == 'latest' ]]; then travis_retry docker-compose run --rm php composer update $COMPOSER_ARGS ; fi
- if [[ $DEPS == 'lowest' ]]; then travis_retry docker-compose run --rm php composer update --prefer-lowest --prefer-stable $COMPOSER_ARGS ; fi
- if [[ $MONGO_DRIVER == 'mongodb' ]]; then travis_retry docker-compose run --rm php composer require --dev $COMPOSER_ARGS $ADAPTER_DEPS ; fi
- if [[ $TEST_COVERAGE == 'true' ]]; then travis_retry docker-compose run --rm php composer require --dev $COMPOSER_ARGS $COVERAGE_DEPS ; fi
script:
- if [[ $TEST_COVERAGE == 'true' ]]; then docker-compose run php composer test-coverage ; else docker-compose run php composer test ; fi
- if [[ $CS_CHECK == 'true' ]]; then docker-compose run php composer cs-check ; fi
after_script:
- if [[ $TEST_COVERAGE == 'true' ]]; then docker-compose run php vendor/bin/php-coveralls -v ; fi
The project path is always mounted and synchronized with the container, so after `composer install´ you can find the vendor directory in the host (outside the container), but every command that interact with the environment should be executed in the container. Let me know if this can help you. Originally posted by @thomasvargiu at zfcampus/zf-apigility-doctrine#308 (comment) |
... and we need install first composer in docker container. And just a thought: maybe we can create somehow alias in travis config to run just Originally posted by @michalbundyra at zfcampus/zf-apigility-doctrine#308 (comment) |
Ok, I see composer is already installed in docker container, so we can just create aliases:
and use See: https://github.com/webimpress/zf-apigility-doctrine/commit/d7a9b98fcfa337b1b481c6c99a66862688116607 Originally posted by @michalbundyra at zfcampus/zf-apigility-doctrine#308 (comment) |
The build works once again, this time with pure Docker unit testing and code coverage. I was unable to get alias working; could use some help there. I don't think naming an alias the same is a good idea though and would rather see an alias like xdebug support is in. For adding the .so for xdebug and mongo there seems to be some duplicated effort but it's all working. If someone wants to figure out how to get them both working without adding either to the php.ini twice and working for PHP 5.6 and 7+ that'd be great. I created a command to get the php.ini path
Better ideas are welcome; thanks! Originally posted by @TomHAnderson at zfcampus/zf-apigility-doctrine#308 (comment) |
And: here's the first draft of a presentation on how to do this: https://goo.gl/R4CQ9s This is intended as a code walk-through talking about the what and why of each line. Originally posted by @TomHAnderson at zfcampus/zf-apigility-doctrine#308 (comment) |
@TomHAnderson @weierophinney @thomasvargiu I've created PR TomHAnderson/zf-apigility-doctrine#1 with some simplification, more info there. I think we still have one problem. When using PHP 7 we requires Originally posted by @michalbundyra at zfcampus/zf-apigility-doctrine#308 (comment) |
@TomHAnderson Hm... I just noted that I've broken xdebug and coverage report... I've tried many thing and can't get it working back. See: TomHAnderson/zf-apigility-doctrine@feature/docker-for-testing...webimpress:feature/docker-for-testing Originally posted by @michalbundyra at zfcampus/zf-apigility-doctrine#308 (comment) |
For xdebug I had to spedify Originally posted by @TomHAnderson at zfcampus/zf-apigility-doctrine#308 (comment) |
I think the problem is with the condition in Dockerfile. I will have another try tomorrow... Any ideas what we can do with Originally posted by @michalbundyra at zfcampus/zf-apigility-doctrine#308 (comment) |
I fixed xdebug. You didn't read slide 11: https://docs.google.com/presentation/d/1KhcfyU4nKVUoaTczCuMPNVymw3GaTWrtfUTwB-TzJrs/edit#slide=id.g378fb9fb7b_0_99 Originally posted by @TomHAnderson at zfcampus/zf-apigility-doctrine#308 (comment) |
Nice work @TomHAnderson! Other proposals: Ignore the
|
Not sure about it. If you want to test with different PHP version you can do:
and then:
(or maybe it is even possible to do in one line?) But in general I can see your point, we just need to think what will be the most convenient for contributors.
Yes! This is a very good idea. The script will be clearer. Originally posted by @michalbundyra at zfcampus/zf-apigility-doctrine#308 (comment) |
Agree with @thomasvargiu that we should include as a dist allowing devs to have their own docker-compose.yml Originally posted by @TomHAnderson at zfcampus/zf-apigility-doctrine#308 (comment) |
I've used docker-compose.yml.dist now because @Ocramius thought (at one point) it was good to have my own composer environment to work on a repo and I agree. Using a .dist allows for custom environments. Now that entrypoint.sh is a script I've added a touch of ascii art to the shell when you login to the php container. I feel strongly this should stay as-is. This PR is now at RC Originally posted by @TomHAnderson at zfcampus/zf-apigility-doctrine#308 (comment) |
@TomHAnderson @thomasvargiu I was looking again on docker-compose.yml.dist file solution. I have found this in the docker documentation: https://docs.docker.com/compose/extends/ Originally posted by @michalbundyra at zfcampus/zf-apigility-doctrine#308 (comment) |
I like the docker supported method for this over the .dist approach. Originally posted by @TomHAnderson at zfcampus/zf-apigility-doctrine#308 (comment) |
New Doctrine modules are coming down the pipe. To avoid trying to manipulate my bare metal I wrote this Docker container set for use in development and testing.
This isn't normal for Zend packages so please let me know your thoughts on including Docker container with complex repositories and let's get this merged and ready for the upcoming Doctrine changes.
Originally posted by @TomHAnderson at zfcampus/zf-apigility-doctrine#308
The text was updated successfully, but these errors were encountered: