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

Add functional tests for routes #152

Merged
merged 1 commit into from Jul 6, 2017

Conversation

Projects
2 participants
@agathver
Copy link
Collaborator

agathver commented Jun 21, 2017

This PR sets up Travis CI for adds functional tests involving MySQL and RabbitMQ.

It adds a test for the DefaultController, which should be used as a template for writing further tests.

#148

@agathver agathver force-pushed the agathver:travis_webtest branch from c8985ff to b5cc40a Jun 21, 2017

@imphil

This comment has been minimized.

Copy link
Contributor

imphil commented Jun 22, 2017

This commit recreates part of the deployment rules we have in Ansible today inside the travis configuration. I'm not convinced this is a viable path in the longer term:

  • Deployment is tricky: getting all required components of the deployment right isn't easy.
  • We want to test what we actually use and not fight bugs caused by discrepancies between different testing environments.

In my opinion a more viable long-term approach would be:

  • Add tests and testing frameworks as needed and ensure that testing locally in the standard Vagrant environment works.
    For test automation, we then have two options:
  • To test in travis, wait for the docker deployments to be ready. #151
  • If that takes too long or doesn't work out, we can always use a baremetal machine with LibreCores CI (Jenkins) to run the tests in our own infrastructure using the already existing Vagrant setup.
@agathver

This comment has been minimized.

Copy link
Collaborator

agathver commented Jun 22, 2017

@imphil The main aim of these tests is to make sure that a PR accidentally does not break any feature due to modifying components which other parts are dependent on. This does not aim to be a full replacement for an "integration test" for which we will continue to need a staging box.

What it aims to do:

  1. Verifies that there is no errors in controllers and that they return a correct response in a mocked/controlled environment,
  2. Services work as expected.
  3. Database queries work as expected.

For testing all the parts correctly, we undoubtedly need Selenium or equivalent on a staging environment configured identical to the production environment.

Additionally, we can create a testing enviornment inside a sudo-enabled Travis CI by running the ansible provisioner inside Travis, and performing HTTP tests on it.

@imphil

This comment has been minimized.

Copy link
Contributor

imphil commented Jun 22, 2017

I agree with the goals, just the deployment part inside travis.yml replicating our setup is pretty fragile in my opinion. So let's do it this way:

  • First, split up this commit into a part adding the tests and a part adding the travis deployment.
  • The first part can go in immediately, no concerns here.
  • For the second part, the travis integration, I'm proposing two things:
    • We can add it as temporary stopgap measure until we have docker in travis running. Looking at the PR #151 it seems that's not in the far future. Also, don't put any more effort into it making the environment nicer.
    • Open a follow-up issue and to remove this stopgap again and replace it as soon as we either have docker in travis running, or we have another solution ready.

I hope this approach doesn't block your development process, but also makes it clear that this solution won't stay (and shouldn't stay).

* Date: 21/6/17
* Time: 3:05 PM
*/

This comment has been minimized.

@imphil

imphil Jun 22, 2017

Contributor

You can remove this header.

// since most of the content is static, extensive testing is not required, asserting presence of certain key elements
$this->assertEquals($crawler->filter('.librecores-home-banner-wordmark')->html(), 'LibreCores');
$this->assertEquals($crawler->filter('.librecores-home-banner-claim')->html(), 'We drive Free and Open Digital Hardware.'); //expect certain values

This comment has been minimized.

@imphil

imphil Jun 22, 2017

Contributor

Please keep the lines around 80 chars (not a strict requirement, but roughly).

@agathver agathver force-pushed the agathver:travis_webtest branch from b5cc40a to c5f5f50 Jul 4, 2017

@agathver agathver changed the title Configure Travis CI to run functional tests Add functional tests for routes Jul 4, 2017

@agathver

This comment has been minimized.

Copy link
Collaborator

agathver commented Jul 4, 2017

@imphil I have removed changes to Travis CI that dealt with setting up MySQL and RabbitMQ

@imphil imphil merged commit b3198d8 into librecores:master Jul 6, 2017

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
@imphil

This comment has been minimized.

Copy link
Contributor

imphil commented Jul 6, 2017

Merging this for now as it's mostly example code anyways. But please keep the lines around 80 characters. This makes reviewing and editing much easier.

@agathver agathver moved this from Review to Done in Code Quality Metrics Implementation Aug 9, 2017

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