Skip to content
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

Open
weierophinney opened this issue Dec 31, 2019 · 27 comments
Open

Feature/docker for testing #3

weierophinney opened this issue Dec 31, 2019 · 27 comments

Comments

@weierophinney
Copy link
Contributor

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

@weierophinney
Copy link
Contributor Author

Coverage Status

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)

@weierophinney
Copy link
Contributor Author

@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)

@weierophinney
Copy link
Contributor Author

Before you approve this, if you choose, see @Ocramius comment on the linked doctrine PR.
If you think this should be a part of this module I think the mongodb should be part of the php build and not its own container thereby simulating (more closely) the Travis environment.


Originally posted by @TomHAnderson at zfcampus/zf-apigility-doctrine#308 (comment)

@weierophinney
Copy link
Contributor Author

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)

@weierophinney
Copy link
Contributor Author

This has been rewritten to a minimal Docker footprint


Originally posted by @TomHAnderson at zfcampus/zf-apigility-doctrine#308 (comment)

@weierophinney
Copy link
Contributor Author

Includes a fix to a unit test which changed order.


Originally posted by @TomHAnderson at zfcampus/zf-apigility-doctrine#308 (comment)

@weierophinney
Copy link
Contributor Author

@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)

@weierophinney
Copy link
Contributor Author

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)

@weierophinney
Copy link
Contributor Author

Starting with @thomasvargiu suggestions I've engineered the tests to run Docker for every test. You can compose a docker container with

docker-compose build --build-arg PHP_VERSION=5.6

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)

@weierophinney
Copy link
Contributor Author

The point that @thomasvargiu is making is that we can provide this as a flag instead of a version number via a build argument in docker-compose.yml (as he demonstrates in another comment). If the initial docker-compose (build|up --build) operation includes appropriate ENV variables, these can be referenced via docker-compose.yml build arguments and referenced in the Dockerfile. That would arguably be better than parsing versions in the Dockerfile, and make it more explicit the scenarios we are testing. [mwop]

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 docker build. Before with language:php it would install composer as part of the install: script of travis. Now I think the work-around is to create a more elaborate entrypoint.sh file which runs composer according to the build environment parameters. This means every time you connect to the container it will re-compose. That's a strain on the developer using docker to develop against the repository. Maybe an environment variable passed into run which toggles running composer?

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)

@weierophinney
Copy link
Contributor Author

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)

@weierophinney
Copy link
Contributor Author

@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)

@weierophinney
Copy link
Contributor Author

... 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 composer which points to composer on docker?


Originally posted by @michalbundyra at zfcampus/zf-apigility-doctrine#308 (comment)

@weierophinney
Copy link
Contributor Author

Ok, I see composer is already installed in docker container, so we can just create aliases:

    - alias composer="docker-compose run --rm php composer"
    - alias php="docker-compose run --rm php php"

and use composer and php as before.

See: https://github.com/webimpress/zf-apigility-doctrine/commit/d7a9b98fcfa337b1b481c6c99a66862688116607


Originally posted by @michalbundyra at zfcampus/zf-apigility-doctrine#308 (comment)

@weierophinney
Copy link
Contributor Author

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 run_composer. That uncomplicates reading the scripts.

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

`php --ini | sed -n -e 's/^.*Path: //p'`/php.ini

Better ideas are welcome; thanks!


Originally posted by @TomHAnderson at zfcampus/zf-apigility-doctrine#308 (comment)

@weierophinney
Copy link
Contributor Author

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)

@weierophinney
Copy link
Contributor Author

@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 alcaeus/mongo-php-adapter to run tests... Not sure how we can resolve it nicely? Any ideas?


Originally posted by @michalbundyra at zfcampus/zf-apigility-doctrine#308 (comment)

@weierophinney
Copy link
Contributor Author

@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
on my branch I have enabled travis and there are build, but it is still not working correctly...


Originally posted by @michalbundyra at zfcampus/zf-apigility-doctrine#308 (comment)

@weierophinney
Copy link
Contributor Author

For xdebug I had to spedify zend_extension= in the ini. Is it possible the docker-php-ext-enable xdebug isn't enough? I had to do a lot of trial and error to get it working where it was in my last commit.


Originally posted by @TomHAnderson at zfcampus/zf-apigility-doctrine#308 (comment)

@weierophinney
Copy link
Contributor Author

I think the problem is with the condition in Dockerfile. I will have another try tomorrow...

Any ideas what we can do with alcaeus/mongo-php-adapter in default installation?


Originally posted by @michalbundyra at zfcampus/zf-apigility-doctrine#308 (comment)

@weierophinney
Copy link
Contributor Author

@weierophinney
Copy link
Contributor Author

Nice work @TomHAnderson!

Other proposals:

Ignore the docker-compose.yml file FROM VCS

Usually in projects the docker-compose.yml is ignored from VCS to let the user to change it. We could rename it in docker-compose.yml.dist and add docker-compose.yml to .gitignore. In travis-ci we can copy it before to execute commands and tests.

We let the user to use the default image with the "latest" PHP version but if he wants to change it in different environment can easily change it. An example could be:

A contributor wants to test it against different PHP versions:

version: "3"
services:
  php71:
    build:
# .....

  php72:
    build:

And the user could easily run docker-compose run --rm php71 on his host.

I'm just suggesting to ignore the  docker-compose.yml file, not to change it.

Move the entrypoint to a file

Since we have a .docker directory, we can move the entrypoint script to an external file and change the Dockerfile with:

# ...
COPY ./entrypoint.sh /usr/local/bin/entrypoint.sh

You can chmod +x it on VCS or building the image.
The Dockerfile will result more clear.

@TomHAnderson @webimpress what do you think?


Originally posted by @thomasvargiu at zfcampus/zf-apigility-doctrine#308 (comment)

@weierophinney
Copy link
Contributor Author

@thomasvargiu

Ignore the docker-compose.yml file FROM VCS

Not sure about it. If you want to test with different PHP version you can do:

$ docker-compose build --build-arg PHP_VERSION=7.1

and then:

$ docker-compose run --rm php composer test

(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.

Move the entrypoint to a file

Yes! This is a very good idea. The script will be clearer.


Originally posted by @michalbundyra at zfcampus/zf-apigility-doctrine#308 (comment)

@weierophinney
Copy link
Contributor Author

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)

@weierophinney
Copy link
Contributor Author

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)

@weierophinney
Copy link
Contributor Author

@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/
So we have file docker-compose.yml as it is, and in local if we want to have different env we use docker-compose.override.yml, these by default are red by docker, and I would prefer to go this way instead of requiring one more step do contributors - copying .dist file... It's harder then to make some changes in this file, contributors need to check it everytime, and basically I can see many more issues... and not sure what are the advantages?


Originally posted by @michalbundyra at zfcampus/zf-apigility-doctrine#308 (comment)

@weierophinney
Copy link
Contributor Author

I like the docker supported method for this over the .dist approach.


Originally posted by @TomHAnderson at zfcampus/zf-apigility-doctrine#308 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant