[go: nahoru, domu]

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

Update docker-compose test environment based on CI's tests #110

Closed
wants to merge 3 commits into from

Conversation

paulolobt
Copy link
Contributor

Simulate the CI's tests with the docker-compose environment running in local. Adding the changes:
- Add pip and curl Alpine packages on Dockerfile
- Add env variable to run or not the tests
- Create MariaDB and MySQL supervisor process
- Create supervisor test config file
- Run tests inside or not container
- Run tests automatically on docker provision

  - Add MariaDB 10.1 version
  - Add MySQL 5.7 version
  - Link databases to PHPMyAdmin container
  - Add pip and curl Alpine packages
  - Add env variable to run or not the tests
  - Create MariaDB and MySQL test process
  - Create supervisor test config file
  - Run tests inside or not container
  - Run tests automatically on docker provision
Copy link
Contributor
@nijel nijel left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution, but overall I'm not sure this is good approach.

It includes part of the test setup in the production Docker container which will get published on Docker hub. This will make the image bigger and contain not used software while the testing will still not work as it needs additional tweaking done in docker-compose.testing.yml.

What might be better approach to build the testing container on top of the production one adding all necessary files.

PS: Also when doing something like this, it would be great to test it in Travis to ensure we don't break it in the future.

@@ -1,7 +1,7 @@
FROM alpine:3.5

# Install dependencies
RUN apk add --no-cache php7-session php7-mysqli php7-mbstring php7-xml php7-gd php7-zlib php7-bz2 php7-zip php7-openssl php7-curl php7-opcache php7-json nginx php7-fpm supervisor
RUN apk add --no-cache curl php7-session php7-mysqli php7-mbstring php7-xml php7-gd php7-zlib php7-bz2 php7-zip php7-openssl php7-curl php7-opcache php7-json nginx php7-fpm py2-pip supervisor
Copy link
Contributor

Choose a reason for hiding this comment

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

This pulls whole python to the image, I'd prefer to avoid that.

; include files themselves.

[include]
files = /etc/supervisor.d/nginx.ini /etc/supervisor.d/php.ini
Copy link
Contributor

Choose a reason for hiding this comment

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

Couldn't this be handled somehow better? Having two almost same configuration files doesn't sound good for maintenance.

exit $ret
fi

# List processes
docker exec "$NAME" ps faux
echo -e "Result of ${PHPMYADMIN_DB_HOSTNAME} tests: ${GREEN}SUCCESS${NC}"
Copy link
Contributor

Choose a reason for hiding this comment

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

The echo -e does not work in many /bin/sh implementations, it's better to avoid it (see Travis log for example).

@paulolobt
Copy link
Contributor Author

Thanks your feedback @nijel. My mindset was only related to development mode and really the updates include unnecessary changes for production. So I will close this pull request and create a new with only docker-compose.testing.yml updates, because the compose test file not working. That's OK?

@nijel nijel mentioned this pull request Mar 28, 2017
@nijel
Copy link
Contributor
nijel commented Mar 28, 2017

I've tried to base on your changes in #111 (but it probably still needs some polishing).

@nijel nijel self-assigned this Mar 28, 2017
@nijel
Copy link
Contributor
nijel commented Mar 28, 2017

I've based on your changes and implemented solution with additional Dockerfile to overlay the production one for testing. Also the docker-compose.testing.yml is now used during CI to ensure it's kept consistent with future changes.

@nijel nijel closed this Mar 28, 2017
@paulolobt paulolobt deleted the docker_test_env branch March 28, 2017 14:57
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

Successfully merging this pull request may close these issues.

None yet

2 participants