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

[Testing] Dockerized test suite #2430

Merged
merged 9 commits into from Dec 2, 2016

Conversation

jacobpenny
Copy link
Contributor

The README.md included gives an overview of the requirements/features, please have a look there first. Also note that the first time you run the tests will take a while because it needs to download a few docker images.

I tried not to make any changes that would affect existing workflows, however I did have to remove spaces from the test suite names in phpunit.xml in order to make some wrapper scripts I use work. Also, a dedicated test/config.xml file was added because it simplifies set up quite a bit.

This P/R also contains some general test fixes that are needed when running the tests with sandbox enabled (Travis has been running the tests with this disabled and was skipping some tests because of it).

@jacobpenny jacobpenny added the Testing PR contains test plan or automated test code (or config files for Travis) label Nov 21, 2016
@jacobpenny jacobpenny added this to the 17.1 milestone Nov 21, 2016
@jacobpenny jacobpenny changed the title Dockerized test suite [Testing] Dockerized test suite Nov 21, 2016
@kongtiaowang kongtiaowang self-assigned this Nov 22, 2016
@kongtiaowang kongtiaowang added Passed Manual Tests PR has undergone proper testing by at least one peer PassedCodeReview labels Nov 22, 2016
Copy link
Contributor

@kongtiaowang kongtiaowang left a comment

Choose a reason for hiding this comment

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

It is very cool!

Copy link
Collaborator

@driusan driusan left a comment

Choose a reason for hiding this comment

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

I just want to see what happens in the menu on GitHub if someone approves and someone else requests changes..

@driusan driusan dismissed their stale review November 22, 2016 18:14

This was a fake review.

Copy link
Collaborator

@driusan driusan left a comment

Choose a reason for hiding this comment

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

Some minor changes would help maintenance

@@ -0,0 +1,7 @@
FROM php:7.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you just update the main Dockerfile? The default one should probably be updated to PHP7, there's no reason to maintain one for PHP5 anymore.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you saying I can just overwrite the default Dockerfile and Dockerfile.MySQL with mine?

@@ -0,0 +1,27 @@
FROM mysql:5.7
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there any way for this to be combined with the existing Dockerfile.MySQL too?

@@ -0,0 +1,38 @@
#!/usr/bin/env bash
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you update Travis to use these scripts instead of duplicating it?

(You'd have to make sure it exits with a non-zero status for any of the lint runs so that Travis will fail..)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure I'll do that.
If we want to take the plunge we could use the Docker based testing in Travis too. I thought that might be too much change for one PR though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For what it's worth, a dockerized .travis.yml would look like:

language: php

sudo: required

services:
  - docker

cache:
    directories:
        - vendor

env:
  - TEST_SUITE=lint:javascript
  - TEST_SUITE=lint:php
  - TEST_SUITE=tests:unit
  - TEST_SUITE=tests:integration

before_install:
    # Ensure node version is up to date
    - nvm install node
    - npm install
install:
    - composer install
script:
    - npm run --silent $TEST_SUITE

This breaks the tests up into 4 builds: unit tests, integration tests, PHP linting, and JS linting.

@jacobpenny jacobpenny force-pushed the dockerized-test-suite branch 2 times, most recently from 95a5096 to 2025a05 Compare November 22, 2016 19:48
@jacobpenny
Copy link
Contributor Author

Requested changes made

@alisterdev
Copy link
Contributor

@driusan

@driusan driusan dismissed their stale review December 2, 2016 16:38

Changes were incorporated

@driusan driusan merged commit 706268f into aces:17.1-dev Dec 2, 2016
alisterdev pushed a commit to alisterdev/Loris that referenced this pull request Jan 20, 2017
* Removing spaces from test suite names

Having spaces in test suite names makes it difficult to call phpunit
from wrapper scripts, which are needed for the docker-based test suite.

* Skipping broken tests

* Replacing .click with .submit

.click was not executing form submit and was causing the test to fail

* Add missing xsi:nil property to fixture

* Allow a selenium host/port to be specified via config file

* Add docker-based test suite and required scripts

* Add note about creating Docker group

* Rename Dockerfiles

* Use linting scripts in .travis.yml
MounaSafiHarab pushed a commit to MounaSafiHarab/Loris that referenced this pull request Jan 20, 2017
* Removing spaces from test suite names

Having spaces in test suite names makes it difficult to call phpunit
from wrapper scripts, which are needed for the docker-based test suite.

* Skipping broken tests

* Replacing .click with .submit

.click was not executing form submit and was causing the test to fail

* Add missing xsi:nil property to fixture

* Allow a selenium host/port to be specified via config file

* Add docker-based test suite and required scripts

* Add note about creating Docker group

* Rename Dockerfiles

* Use linting scripts in .travis.yml
taracampbell pushed a commit to taracampbell/Loris that referenced this pull request Mar 9, 2017
* Removing spaces from test suite names

Having spaces in test suite names makes it difficult to call phpunit
from wrapper scripts, which are needed for the docker-based test suite.

* Skipping broken tests

* Replacing .click with .submit

.click was not executing form submit and was causing the test to fail

* Add missing xsi:nil property to fixture

* Allow a selenium host/port to be specified via config file

* Add docker-based test suite and required scripts

* Add note about creating Docker group

* Rename Dockerfiles

* Use linting scripts in .travis.yml
jstirling91 pushed a commit to jstirling91/Loris that referenced this pull request May 8, 2017
* Removing spaces from test suite names

Having spaces in test suite names makes it difficult to call phpunit
from wrapper scripts, which are needed for the docker-based test suite.

* Skipping broken tests

* Replacing .click with .submit

.click was not executing form submit and was causing the test to fail

* Add missing xsi:nil property to fixture

* Allow a selenium host/port to be specified via config file

* Add docker-based test suite and required scripts

* Add note about creating Docker group

* Rename Dockerfiles

* Use linting scripts in .travis.yml
kongtiaowang pushed a commit to kongtiaowang/Loris that referenced this pull request Jan 31, 2018
* Removing spaces from test suite names

Having spaces in test suite names makes it difficult to call phpunit
from wrapper scripts, which are needed for the docker-based test suite.

* Skipping broken tests

* Replacing .click with .submit

.click was not executing form submit and was causing the test to fail

* Add missing xsi:nil property to fixture

* Allow a selenium host/port to be specified via config file

* Add docker-based test suite and required scripts

* Add note about creating Docker group

* Rename Dockerfiles

* Use linting scripts in .travis.yml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Passed Manual Tests PR has undergone proper testing by at least one peer Testing PR contains test plan or automated test code (or config files for Travis)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants