-
Notifications
You must be signed in to change notification settings - Fork 50
Feature/docker for testing #308
base: master
Are you sure you want to change the base?
Changes from 5 commits
2afe41b
857298a
e6c6465
97ca995
a6b1272
32cec04
c7b4d02
6894562
05aa000
a2c7bdb
50dfd85
08fd71a
26f53c0
e62c69a
91f7580
00faa66
6661ae8
0ee75b0
76bbabe
8c767ad
3a2ce6b
7fb7d96
79ae7f3
7954365
7a07548
b6f19ce
f91132a
8173b69
acf0cf0
3043a23
a7c0657
142233a
4469bfb
1090652
78cdf71
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,3 +4,4 @@ test/data/ | |
test/assets/module/*/config/module.config.php | ||
clover.xml | ||
coveralls-upload.json | ||
docker/data/ | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
cd /docker | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With previous changes, you don't need this anymore |
||
echo `host mongo` | awk '{print $4, "localhost"}' > /etc/hosts | ||
vendor/bin/phpunit |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ language: php | |
|
||
services: | ||
- mongodb | ||
- docker | ||
|
||
cache: | ||
directories: | ||
|
@@ -59,17 +60,21 @@ matrix: | |
env: | ||
- DEPS=lowest | ||
- MONGO_DRIVER=mongodb | ||
- DOCKER=true | ||
- php: 7.2 | ||
env: | ||
- DEPS=locked | ||
- MONGO_DRIVER=mongodb | ||
- DOCKER=true | ||
- php: 7.2 | ||
env: | ||
- DEPS=latest | ||
- MONGO_DRIVER=mongodb | ||
- DOCKER=true | ||
|
||
before_install: | ||
- if [[ $TEST_COVERAGE != 'true' ]]; then phpenv config-rm xdebug.ini || return 0 ; fi | ||
- if [[ $DOCKER == 'true' ]]; then docker-compose up -d; fi | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't need this |
||
|
||
install: | ||
- yes '' | pecl -q install -f $MONGO_DRIVER | ||
|
@@ -85,6 +90,7 @@ install: | |
script: | ||
- if [[ $TEST_COVERAGE == 'true' ]]; then composer test-coverage ; else composer test ; fi | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should use docker here too. We need to install xdebug on the docker image and then we can run tests with coverage:
or, for test only:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Coverage is only tested on one matrix element. The output of running coverage on Docker would not be used. I don't think Docker coverage is necessary. Is there a place the output would be used I'm not seeing? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @TomHAnderson We need to find a way to load xdebug extension only for coverage test. I think should be better to create a Then we could:
Then we can use a test-with-coverage script that will enable xdebug first: #!/bin/sh
docker-php-ext-enable xdebug \
&& ./vendor/bin/phpunit --colors=always --coverage-clover clover.xml What do you think? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't understand the need for coverage inside Docker. What benefit do we get? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @TomHAnderson My idea is that if we are implementing docker, we should use it for everything. It's not just a coverage, but are tests too. Right now tests are executed 2 times. First in the travis-ci environment (that it's different from the container) with coverage, then from the container. I think we should use every command from the container, even After all these changes, we don't need to do anything on travis-ci environment, and then we can use a simple image on travis. It's just my opinion and a suggestion :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I understand that, thanks. In this same thread @Ocramius suggests doing inside (Docker) and out (Travis) testing: #308 (comment) @webimpress I need sign-off from you too before I'm going to go with a Docker-only approach to unit tests. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I get the reasoning from @Ocramius , and generally agree. The idea should be to run the tests via docker always, for every environment, and not using the travis-ci environment; the rationale is to ensure that the CI environment mimics the environment maintainers use. That said: only do it if it can be feasibly setup. If this is going to take you days to accomplish, then I'm not 100% convinced it's worth it; in that case, the docker setup is simply so contributors do not need to have the required services (mongo, RDBMS systems, etc) running on their own machine, nor need to configure a If it is possible to do easily, let's do it. Use build arguments to do things like enable xdebug and indicate test coverage should be run. |
||
- if [[ $CS_CHECK == 'true' ]]; then composer cs-check ; fi | ||
- if [[ $DOCKER == 'true' ]]; then docker exec -it $(docker ps -f name=zfapigilitydoctrine_php_1 | awk '{print $1;}' | tail -n 1) bash /docker/.travis.docker; fi | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can directly run: |
||
|
||
after_script: | ||
- if [[ $TEST_COVERAGE == 'true' ]]; then travis_retry php vendor/bin/php-coveralls -v ; fi | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
# Using Docker for Development | ||
|
||
This library requires a running instance of Mongo in order to run and pass | ||
the unit tests. It is not expected for each developer to configure their | ||
individual machine to match this environment so Docker is provided. | ||
|
||
|
||
## Running docker-compose | ||
|
||
You will need docker-compose installed on your machine. | ||
|
||
From the root directory of this project run | ||
|
||
``` | ||
docker-compose build | ||
``` | ||
|
||
This will build the php container. Next run | ||
|
||
``` | ||
docker-compose up | ||
``` | ||
|
||
This will spin up the php container and a mongo container. | ||
|
||
To connect to the php container and run the unit tests | ||
run | ||
|
||
``` | ||
docker/connect | ||
``` | ||
|
||
You will connect to the php container a the root directory. | ||
`cd` to `docker` to work with the mapped local files. | ||
|
||
|
||
## Configuration | ||
|
||
Because you're in a Docker environment with a differnet IP address and name for each | ||
container you need to either change the config files in the project to point to `mongo` | ||
or with a simple hack map `localhost` to the `mongo` container. | ||
|
||
* To edit the config file to point to the `mongo` container instead of `localhost` edit | ||
`test/config/ODM/local.php` and change the configuration. | ||
|
||
* Optionally you can map localhost to mongo with | ||
``` | ||
echo `host mongo` | awk '{print $4, "localhost"}' > /etc/hosts | ||
``` | ||
|
||
## Unit Tests | ||
|
||
Having run `composer install` you may now run the unit tests | ||
|
||
``` | ||
vendor/bin/phpunit | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
version: "3" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. docker-compose cam be changed with: version: "3"
services:
php:
build:
context: .
args:
- PHP_VERSION=${PHP_VERSION:-7.2}
dockerfile: docker/Dockerfile
depends_on:
- mongo
volumes:
- ./:/docker
mongo:
image: mongo:latest You can optionally pass |
||
services: | ||
php: | ||
build: | ||
context: . | ||
dockerfile: docker/Dockerfile | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should have |
||
links: | ||
- mongo | ||
ports: | ||
- "9000:9000" | ||
volumes: | ||
- ./:/docker | ||
tty: true | ||
mongo: | ||
image: mongo:latest |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
FROM php:latest | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can use an alpine image to speed up tests, something like this: ARG PHP_VERSION=7.2
FROM php:${PHP_VERSION}-alpine
RUN apk add --no-cache \
autoconf \
make \
g++
RUN pecl install mongodb && docker-php-ext-enable mongodb
RUN set -o pipefail && curl -sS https://getcomposer.org/installer | php -- --install-dir=/usr/local/bin --filename=composer
RUN echo -e '#!/bin/sh' >> /usr/local/bin/entrypoint.sh \
&& echo -e 'while ! nc -z ${MONGO_HOST:-mongo} ${MONGO_PORT:-9000}; do sleep 1; done' >> /usr/local/bin/entrypoint.sh \
&& echo -e 'exec "$@"' >> /usr/local/bin/entrypoint.sh \
&& chmod +x /usr/local/bin/entrypoint.sh
WORKDIR /docker
ENTRYPOINT ["/usr/local/bin/entrypoint.sh"]
CMD ["./vendor/bin/phpunit"] You can specify a build argument to say which image to build. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Obviously you should change code where mongodb host is |
||
RUN apt-get update | ||
RUN apt-get --yes install vim | ||
RUN apt-get --yes install git | ||
RUN apt-get --yes install inetutils-ping | ||
RUN apt-get --yes install host | ||
RUN pecl install mongodb && docker-php-ext-enable mongodb | ||
RUN php -r "copy('https://getcomposer.org/installer', 'composer-setup.php');" | ||
RUN php composer-setup.php | ||
RUN php -r "unlink('composer-setup.php');" | ||
RUN mv composer.phar /bin/composer |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
docker exec -it $(docker ps -f name=zfapigilitydoctrine_php_1 | awk '{print $1;}' | tail -n 1) bash | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With previous changes, you don't need this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see any usage of this folder, can we remove it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was a remnant; removed