Skip to content
This repository has been archived by the owner on Jan 21, 2020. It is now read-only.

Feature/docker for testing #308

Open
wants to merge 35 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 32 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
2afe41b
Added docker-compose for development
TomHAnderson Apr 20, 2018
857298a
Instructions for Docker
TomHAnderson Apr 20, 2018
e6c6465
proofreading DOCKER.md
TomHAnderson Apr 20, 2018
97ca995
Added docker-compose and unit tests to travis
TomHAnderson Apr 22, 2018
a6b1272
The results in this test have changed order but are still valid. Rew…
TomHAnderson Apr 2, 2018
32cec04
Incorporated suggestions from @thomasvargiu for Docker
TomHAnderson Apr 22, 2018
c7b4d02
Added docker tests to travis
TomHAnderson Apr 22, 2018
6894562
Organized .travis.yml
TomHAnderson Apr 23, 2018
05aa000
Made changes based on feedback on github
TomHAnderson Apr 23, 2018
a2c7bdb
First test run of non-php specific travis test
TomHAnderson Apr 24, 2018
50dfd85
none = language?
TomHAnderson Apr 24, 2018
08fd71a
Added all tests back in
TomHAnderson Apr 24, 2018
26f53c0
Added build to install script
TomHAnderson Apr 24, 2018
e62c69a
composer-build works; extenting to .travis.yml
TomHAnderson Apr 24, 2018
91f7580
Added xdebug
TomHAnderson Apr 25, 2018
00faa66
Contributing Docker docs
TomHAnderson Apr 25, 2018
6661ae8
Tidy contributing
TomHAnderson Apr 25, 2018
0ee75b0
Removed DOCKER.md
TomHAnderson Apr 26, 2018
76bbabe
Removed ini debugging line
TomHAnderson Apr 26, 2018
8c767ad
Use aliases and simplify installing php extensions
michalbundyra Apr 26, 2018
3a2ce6b
Merge pull request #1 from webimpress/feature/docker-for-testing
TomHAnderson Apr 26, 2018
7fb7d96
Minor tweeks and documentation for Docker
TomHAnderson Apr 26, 2018
79ae7f3
Moved XDEBUG arg back below FROM
TomHAnderson Apr 26, 2018
7954365
Re-added COMPOSER_ARGS
TomHAnderson Apr 26, 2018
7a07548
Implemented all ideas from @webimpress
TomHAnderson Apr 27, 2018
b6f19ce
Put brackets back around variables
TomHAnderson Apr 27, 2018
f91132a
Put brackets back around variables
TomHAnderson Apr 27, 2018
8173b69
Move docker-compose to dist; move entrypoint to file; add ascii art
TomHAnderson Apr 27, 2018
acf0cf0
Made entrypoint.sh executable
TomHAnderson Apr 27, 2018
3043a23
Added ascii art
TomHAnderson Apr 27, 2018
a7c0657
Updated docs and ascii art
TomHAnderson Apr 27, 2018
142233a
Removed .dist in favor of Docker override
TomHAnderson Apr 28, 2018
4469bfb
Add override to .gitignore
TomHAnderson Apr 30, 2018
1090652
Added install and update hooks for comopser for MongoDB Shim
TomHAnderson Apr 30, 2018
78cdf71
Added JSON_UNESCAPED_SLASHES
TomHAnderson Apr 30, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions .docker/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
ARG PHP_VERSION=7.2
FROM php:${PHP_VERSION}-alpine

ARG XDEBUG=0

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

RUN apk add --no-cache \
autoconf \
make \
g++ \
bash \
git \
openssl-dev

RUN echo -e 'memory_limit=2G' > /usr/local/etc/php/conf.d/memory.ini
RUN set -o pipefail && curl -sS https://getcomposer.org/installer | php -- --install-dir=/usr/local/bin --filename=composer
RUN if [ ${PHP_VERSION:0:3} == "5.6" ] ; then \
pecl install mongo && docker-php-ext-enable mongo ; \
else \
pecl install mongodb && docker-php-ext-enable mongodb ; \
fi
RUN if [ ${XDEBUG} == "1" ] ; then pecl install xdebug && docker-php-ext-enable xdebug ; fi
RUN composer config --global "platform.ext-mongo" "1.999" ;

WORKDIR /docker
ENTRYPOINT ["/usr/local/bin/entrypoint.sh"]
20 changes: 20 additions & 0 deletions .docker/ascii.art
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
+-------------------------+ && %&&(
|ooooo:. .:ooooooooooooo| &&&&&&&&&&&&
|ooooo. ::ooooooooooo| ,& &&&
|ooooo:. .oooooooooo| ,//&& (&&&&&
|ooooooo:. .:ooooooo| / ////&&%&&% #(&&&&
|ooooo:... :ooooo| ,*////######&&& &&&&&
|ooo:. .. .:ooo| //////########&&&&&&&&&
|o:. :ooo. :o| ///////#########&&&&&&&
|o. .:oooo. o| ////////&######&&&&&&&
|. . ..ooooo.. .| &%(/////&&&&&####&&&&&&&
| :oooooooooooooo. | &&& &&&&&&##&&&&&&&&
| .:::::::oooooo: | / * & (&&&&&&&%&&&&&&&&&(
|. .:oooo:. .| & % / &&&&&&&&& &&&
|o. :oooo:. .o| & ( & ( . &&&&& %&&
|oo. :o::. .oo| ( & &&&&
|ooo:.. .oooo| ** * &&&&
|oooooo:.. ..:oooooo| /
+-------------------------+ / /

Doctrine in Apigility
9 changes: 9 additions & 0 deletions .docker/entrypoint.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#!/bin/sh

while ! nc -z ${MONGO_HOST:-mongo} ${MONGO_PORT:-27017};
do sleep 1;
done

cat ./.docker/ascii.art

exec "$@"
2 changes: 2 additions & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,5 @@
.travis.yml export-ignore
phpcs.xml export-ignore
phpunit.xml.dist export-ignore
docker-compose.yml export-ignore
/.docker export-ignore
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ test/data/
test/assets/module/*/config/module.config.php
clover.xml
coveralls-upload.json
docker-compose.yml
Copy link
Member

Choose a reason for hiding this comment

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

We shouldn't ignore that file, but docker-compose.override.yml.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've changed this around to override.

45 changes: 16 additions & 29 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -1,20 +1,11 @@
sudo: false

language: php

services:
- mongodb

cache:
directories:
- $HOME/.composer/cache

env:
global:
- COMPOSER_ARGS="--no-interaction"
Copy link
Member

Choose a reason for hiding this comment

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

@TomHAnderson For me it is not simplification, I think we should revert it back. Sometimes we uses here --no-scripts or --no-plugins as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've restored the COMPOSER_ARGS

- ADAPTER_DEPS="alcaeus/mongo-php-adapter"
- COVERAGE_DEPS="php-coveralls/php-coveralls"
- MONGO_DRIVER=mongo
- COMPOSER_ARGS="--no-interaction"

matrix:
include:
Expand All @@ -31,63 +22,59 @@ matrix:
- php: 7
env:
- DEPS=lowest
- MONGO_DRIVER=mongodb
- php: 7
env:
- DEPS=locked
- LEGACY_DEPS="doctrine/doctrine-module doctrine/doctrine-orm-module phpunit/phpunit zendframework/zend-code"
- MONGO_DRIVER=mongodb
- php: 7
env:
- DEPS=latest
- MONGO_DRIVER=mongodb
- php: 7.1
env:
- DEPS=lowest
- MONGO_DRIVER=mongodb
- php: 7.1
env:
- DEPS=locked
- CS_CHECK=true
- TEST_COVERAGE=true
- MONGO_DRIVER=mongodb
- php: 7.1
env:
- DEPS=latest
- MONGO_DRIVER=mongodb
- php: 7.2
env:
- DEPS=lowest
- MONGO_DRIVER=mongodb
- php: 7.2
env:
- DEPS=locked
- MONGO_DRIVER=mongodb
- php: 7.2
env:
- DEPS=latest
- MONGO_DRIVER=mongodb

before_install:
- if [[ $TEST_COVERAGE != 'true' ]]; then phpenv config-rm xdebug.ini || return 0 ; fi
- shopt -s expand_aliases
- alias composer="travis_retry docker-compose run --rm php composer"
- alias php="travis_retry docker-compose run --rm php php"
- docker-compose build --build-arg PHP_VERSION=${TRAVIS_PHP_VERSION} --build-arg XDEBUG=${TEST_COVERAGE:+1} --no-cache php
- composer --version
- php -v
- php -m

install:
- yes '' | pecl -q install -f $MONGO_DRIVER
- composer config platform.ext-mongo '1.999'
- travis_retry composer install $COMPOSER_ARGS --ignore-platform-reqs
- if [[ $LEGACY_DEPS != '' ]]; then travis_retry composer update $COMPOSER_ARGS --with-dependencies $LEGACY_DEPS ; fi
- if [[ $DEPS == 'latest' ]]; then travis_retry composer update $COMPOSER_ARGS ; fi
- if [[ $DEPS == 'lowest' ]]; then travis_retry composer update --prefer-lowest --prefer-stable $COMPOSER_ARGS ; fi
- if [[ $MONGO_DRIVER == 'mongodb' ]]; then travis_retry composer require --dev $COMPOSER_ARGS $ADAPTER_DEPS ; fi
- if [[ $TEST_COVERAGE == 'true' ]]; then travis_retry composer require --dev $COMPOSER_ARGS $COVERAGE_DEPS ; fi
- composer install $COMPOSER_ARGS --ignore-platform-reqs
- if [[ $LEGACY_DEPS != '' ]]; then composer update $COMPOSER_ARGS --with-dependencies $LEGACY_DEPS ; fi
- if [[ $DEPS == 'latest' ]]; then composer update $COMPOSER_ARGS ; fi
- if [[ $DEPS == 'lowest' ]]; then composer update --prefer-lowest --prefer-stable $COMPOSER_ARGS ; fi
- if [[ $TRAVIS_PHP_VERSION != "5.6" ]] ; then composer require --dev $COMPOSER_ARGS $ADAPTER_DEPS ; fi
- if [[ $TEST_COVERAGE == 'true' ]]; then composer require --dev $COMPOSER_ARGS $COVERAGE_DEPS ; fi
- stty cols 120 && composer show
- composer show

script:
- if [[ $TEST_COVERAGE == 'true' ]]; then composer test-coverage ; else composer test ; fi
Copy link
Contributor

Choose a reason for hiding this comment

The 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:

$ docker-compose run --rm php ./vendor/bin/phpunit --colors=always --coverage-clover clover.xml

or, for test only:

$ docker-compose run --rm php ./vendor/bin/phpunit --colors=always

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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?

Copy link
Contributor

Choose a reason for hiding this comment

The 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 .docker directory to put there docker files as @webimpress suggested.

Then we could:

  • Write the entry point file and use COPY or ADD in the Dockerfile, without to write it "on the fly" building the image
  • Install xdebug extension, without enabling it
  • Create sh commands and copy them in the Dockerfile image

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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?

Copy link
Contributor

Choose a reason for hiding this comment

The 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 composer install and php-coveralls.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

The 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 phpunit.xml with the specifics of their own services.

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

after_script:
- if [[ $TEST_COVERAGE == 'true' ]]; then travis_retry php vendor/bin/php-coveralls -v ; fi
- if [[ $TEST_COVERAGE == 'true' ]]; then php vendor/bin/php-coveralls -v ; fi

notifications:
Copy link
Member

Choose a reason for hiding this comment

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

Not sure why it is removed, we need it back.

email: false
55 changes: 53 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,41 @@ contributors a chance to resolve the vulnerability and issue a new release prior
to any public exposure; this helps protect Apigility users, and provides them
with a chance to upgrade and/or update in order to protect their applications.

For sensitive email communications, please use
For sensitive email communications, please use
[our PGP key](http://framework.zend.com/zf-security-pgp-key.asc).

## RUNNING TESTS
## 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.
Docker supports many PHP versions. You may specify the version of PHP you
want (default:7.2) to build against when running `docker-compose` and flag XDEBUG to
install (1) or not (default:0).

Next, from the root directory of this project, run

```
$ docker-compose build
```

To build a custom container use the flags

```
$ docker-compose build --build-arg PHP_VERSION=7.1 --build-arg XDEBUG=1
```

To connect to the php container for development run

```
$ docker-compose run --rm php bash
```

### RUNNING UNIT TESTS IN THE CONTAINER

First, use [Composer](https://getcomposer.org) to install all dependencies:
Copy link
Member

Choose a reason for hiding this comment

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

We don't need that anymore as it is installed on docker. Just we need to note that following commands have to be run in docker container (so after connection as described in previous section).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But it isn't installed in docker. Travis runs composer. A fresh copy of the module and running Docker will not run composer.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Hah, sorry, of course, composer is installed but dependencies are not :) I red this paragraph wrong.


Expand All @@ -56,6 +87,26 @@ To run tests:
$ composer test
```

### RUNNING UNIT TESTS ON DOCKER

You may run the unit tests through the container without running bash via

```
$ docker-compose run --rm php composer test
```

### DOCKER AND CONTINUOUS INTEGRATION

When you have made a change and created a pull request for it Travis-CI is used
to validate the build. For this a Docker container is created on travis for each
build matrix and the unit tests are ran identical to running unit tests on Docker locally.

### OVERRIDING DOCKER FOR LOCAL DEVELOPMENT

Docker supports overriding the docker-compose.yml file. See
[https://docs.docker.com/compose/extends/#multiple-compose-files](https://docs.docker.com/compose/extends/#multiple-compose-files)
for more information.

## CODING STANDARDS

While Apigility uses Zend Framework 2 coding standards, in practice, we check
Expand Down
19 changes: 19 additions & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
version: "3"
Copy link
Contributor

Choose a reason for hiding this comment

The 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 PHP_VERSION as environment variable in .travis.yml, using it for different PHP versions

services:
php:
build:
context: .
dockerfile: .docker/Dockerfile
args:
- PHP_VERSION=${PHP_VERSION:-7.2}
- XDEBUG=${XDEBUG:-0}
depends_on:
- mongo
volumes:
- ./:/docker
environment:
- MONGO_HOST=mongo
Copy link
Contributor

Choose a reason for hiding this comment

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

These are the default one, you can avoid these environment variables, because you can override it in travis.yml

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Running locally the environment variables are correct. The "default" ones are localhost/27017. We cannot move these to .travis.yml only because the local Docker needs them too.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Additionally this Docker configuration is correct for the docker-compose and the current project. Moving the docker connect information to environment variables means we're encouraging the user to setup their own hardware to run the repository which is fine if they don't want to use Docker.

- MONGO_PORT=27017

mongo:
image: mongo:latest
4 changes: 2 additions & 2 deletions test/config/ODM/local.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
'doctrine' => [
'connection' => [
'odm_default' => [
'server' => 'localhost',
'port' => '27017',
'server' => getenv('MONGO_HOST') ?: 'localhost',
'port' => getenv('MONGO_PORT') ?: '27017',
'user' => '',
'password' => '',
'dbname' => 'zf_apigility_doctrine_server_test',
Expand Down
35 changes: 26 additions & 9 deletions test/src/Admin/Model/DoctrineAutodiscoveryModelTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,34 @@ public function testORMAutodiscoveryEntitiesWithFields()
$this->assertInternalType('array', $result);
$this->assertCount(3, $result);

$this->assertEquals(Album::class, $result[0]['entity_class']);
$this->assertEquals('Album', $result[0]['service_name']);
$this->assertCount(2, $result[0]['fields']);
$resultProcessed = [];
foreach ($result as $row) {
switch ($row['entity_class']) {
case Album::class:
$this->assertEquals('Album', $row['service_name']);
$this->assertCount(2, $row['fields']);
break;
case Artist::class:
$this->assertEquals('Artist', $row['service_name']);
$this->assertCount(2, $row['fields']);
break;
case Product::class:
$this->assertEquals('Product', $row['service_name']);
$this->assertCount(1, $row['fields']);
break;
default:
throw new \Exception("Unexpected result: " . $row['entity_class']);
}

$this->assertEquals(Artist::class, $result[1]['entity_class']);
$this->assertEquals('Artist', $result[1]['service_name']);
$this->assertCount(2, $result[1]['fields']);
if (isset($resultProcessed[$row['entity_class']])) {
$resultProcessed[$row['entity_class']] ++;
} else {
$resultProcessed[$row['entity_class']] = 1;
}
}

$this->assertEquals(Product::class, $result[2]['entity_class']);
$this->assertEquals('Product', $result[2]['service_name']);
$this->assertCount(1, $result[2]['fields']);
$this->assertEquals(3, sizeof($resultProcessed));
$this->assertEquals(3, array_sum($resultProcessed));
}

public function testODMAutodiscoveryEntitiesWithFields()
Expand Down