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

A better CI setup #195

Closed
julien51 opened this issue Aug 23, 2018 · 8 comments
Closed

A better CI setup #195

julien51 opened this issue Aug 23, 2018 · 8 comments
Assignees

Comments

@julien51
Copy link
Member

We are using Travis-CI to run our tests on Pull Requests.
The current setup involves using docker-compose to build an setup in which we can do tests. This setup has 2 services:

  1. ganache-cli which runs an instance of ganache
  2. a container with all the Unlock code with a single command to run all the tests from tests.sh.

When adding new test suites (we currently only test smart-contracts and unlock-app) you need to update tests.sh.

The travis-ci config is "strange":

before_install:
- docker-compose -f docker/docker-compose.ci.yml build
- docker-compose -f docker/docker-compose.ci.yml up --abort-on-container-exit

We first build the cluster: so far so good, but then we also run it and ask it to terminate when a container exits. That second instruction should probably not in before_install. Also we should probably terminate properly.

This setup is probably what is causing #194? (the error does happen but for some reason test.sh continues it execution...

Finally, running CI already takes about 5 minutes while this is a very early project. There are ways to optimize that building process.

@benwerd
Copy link
Contributor

benwerd commented Aug 23, 2018

I think we'll gain a lot of time by caching Docker images. There's a good-looking tutorial here: http://www.rundef.com/fast-travis-ci-docker-build

We could also do more to run tests in parallel: https://docs.travis-ci.com/user/speeding-up-the-build/#parallelizing-your-builds-across-virtual-machines

@HardlyDifficult
Copy link
Contributor

To save on test run time, you could consider removing smart-contracts\test\Unlock\Unlock.js. That file demonstrates direct access to the Unlock contract vs going through a proxy. If you are moving forward with the proxy solution - there should be no benefit in maintaining both tests. smart-contracts\test\Unlock\UnlockProxy.js includes all the same tests running through a proxy.

@julien51
Copy link
Member Author

After meeting with @heewa this morning I start to have a better understanding of how our setup is not optimal. We probably have a decent approach but we need to improve further ;)

@julien51
Copy link
Member Author

So, in a nutshell, the feedback was this:

  1. we need to extract the stuff from test.sh and pull that as much as possible into the npm scripts (or else that we're running for each code component.
  2. we can and should run more containers... however they should probably of the same "kind" with just a different CMD. This way, the same image can be used to run the application, to run the test suite, or even be deployed!

@cellog
Copy link
Contributor

cellog commented Dec 24, 2018

@julien51
Copy link
Member Author

julien51 commented Dec 28, 2018

As of b31ab4a we now have both 1. and 2. implemented.

Next: speeding things up as latest build took 6 min 15 sec !

@julien51
Copy link
Member Author

Speeding up can be achieved in 2 ways:

  • caching the build phase (this takes about 4minutes empirically). However, we need to ensure that we have a way to expire these caches when dependencies are updated.
  • parallelizing. Right now we test each sub-directory/service sequentially. We could very well achieve that in parallel. We could go even further by allowing each service to define its own parallelization (for example linting can tests could be run in parallel for each project).

@julien51 julien51 self-assigned this Dec 29, 2018
@julien51
Copy link
Member Author

julien51 commented Jan 7, 2019

Closing as we now have caching and I am not sure we can make smart parallelization at this point...

@julien51 julien51 closed this as completed Jan 7, 2019
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

No branches or pull requests

4 participants