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

Docker setup for running tests #374

Merged
merged 4 commits into from
Oct 30, 2018
Merged

Conversation

LeadTechVisas
Copy link
Contributor

No description provided.

Copy link
Collaborator

@Slamdunk Slamdunk left a comment

Choose a reason for hiding this comment

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

That's just awesome, thanks!
A few changes and this gets merged 👍

Dockerfile Outdated
@@ -0,0 +1,6 @@
FROM php:7
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
FROM php:7
FROM php:7.0

We shall ensure the library runs on the lowest PHP version supported

container_name: php-imap
volumes:
- .:/project
command: bash -c "sleep 1s && /project/vendor/bin/phpunit /project/tests"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
command: bash -c "sleep 1s && /project/vendor/bin/phpunit /project/tests"
command: bash -c "sleep 1s && cd /project && vendor/bin/phpunit"

CDing into the directory gives us the benefit of reading the committed phpunit.xml.dist

Is the sleep 1s really necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the sleep is necessary because the imap server doesn't respond immediately.

Using php 7.0 I get the following error:

This version of PHPUnit is supported on PHP 7.1 and PHP 7.2. You are using PHP 7.0.32 (/usr/local/bin/php).

Would it be a problem to just lower the phpunit version?

Copy link
Collaborator

Choose a reason for hiding this comment

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

The problem is not PHPUnit, but the fact that composer install was run into an environment different from docker, and thus a different PHPUnit version was chosen.

The correct solution is copying current directory into the docker image, and then running composer install at every run, to ensure a proper image initialization without touching the host non-committed files.

Could you give it a try?

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 see that as a practical solution. The goal of this setup is to have a quick and easy way of running the tests on your local machine while developing. If every time I want to run the tests I have to rebuild the docker image and run composer install, the whole thing just becomes slow and clumsy.

Maybe we can just install the right phpunit version globally inside the container?

Copy link
Collaborator

Choose a reason for hiding this comment

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

If every time I want to run the tests I have to rebuild the docker image and run composer install, the whole thing just becomes slow and clumsy.

The docker image wouldn't need a rebuild.

I've done some tests: if a developer uses Docker to test this library, it is very unlikely that tests are run outside Docker.

So I tried to add composer to the Dockerfile, and run composer install at every docker-compose run tests without protecting the hosts for changes. The dependancies are updated the first run, and then the other runs are quick since no update is needed, vendor/ stay aligned with Docker environments requirements.

To me this is practical enough. These are the files I've used:

Dockerfile

FROM php:7.0

RUN runtimeDeps=" \
        curl \
        git \
        zip \
        libc-client-dev \
        libkrb5-dev \
        libxml2-dev \
    " \
    && apt-get update && DEBIAN_FRONTEND=noninteractive apt-get install -y $runtimeDeps \
    && docker-php-ext-configure imap --with-kerberos --with-imap-ssl \
    && docker-php-ext-install iconv mbstring imap \
    && rm -r /var/lib/apt/lists/*

RUN curl -sS https://getcomposer.org/installer | php -- --install-dir=/usr/local/bin --filename=composer

docker-compose.yml

version: '3.0'

services:
  tests:
    build: .
    container_name: php-imap
    volumes:
      - .:/project
    command: bash -c "cd /project && rm composer.lock && composer install && vendor/bin/phpunit"
    environment:
      - IMAP_SERVER_NAME=imap.server
      - IMAP_SERVER_PORT=993
      - [email protected]
      - IMAP_PASSWORD=p4ssword
    depends_on:
      - imap
  imap:
    image: antespi/docker-imap-devel:latest
    container_name: imap-server
    environment:
    - MAILNAME=test.test
    - [email protected]
    - MAIL_PASS=p4ssword
    networks:
      default:
        aliases:
        - 'imap.server'

container_name: php-imap
volumes:
- .:/project_orig
command: bash -c "wait-for-it.sh imap.server:993 && cp -dpr /project_orig /project && cd /project && composer install && vendor/bin/phpunit"
Copy link
Collaborator

Choose a reason for hiding this comment

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

@LeadTechVisas I've replaced sleep with wait-for-it, maybe it's helpful for you to know this

@Slamdunk Slamdunk merged commit 89e0ea5 into ddeboer:master Oct 30, 2018
@Slamdunk
Copy link
Collaborator

@LeadTechVisas thank you very much 🚢

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

Successfully merging this pull request may close these issues.

2 participants