Formed in 2009, the Archive Team (not to be confused with the archive.org Archive-It Team) is a rogue archivist collective dedicated to saving copies of rapidly dying or deleted websites for the sake of history and digital heritage. The group is 100% composed of volunteers and interested parties, and has expanded into a large amount of related projects for saving online and digital history.
Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upAdd Docker testing to project #832
Conversation
|
@tombruijn Can you consider reviewing? |
|
@matkoniecz - This is not finished, but help is welcome! I have just updated the PR: the code now uses Docker Compose, and runs integration tests for MySQL and PostgreSQL. |
|
Woah.. This PR is a little big for me to review in a sane way. Can you split this up in several PRs? Added some random things as review comments I noticed along the way. |
| @@ -0,0 +1,317 @@ | |||
| # encoding: utf-8 | |||
tombruijn
Apr 16, 2017
Member
encoding magic comments shouldn't be necessary anymore
encoding magic comments shouldn't be necessary anymore
|
|
||
| Dir[File.expand_path("../support/**/*.rb", __FILE__)].each { |f| require f } | ||
|
|
||
| require "rspec/autorun" |
tombruijn
Apr 16, 2017
Member
If you rebase again master I've updated RSpec and this require won't work anymore
If you rebase again master I've updated RSpec and this require won't work anymore
| @@ -0,0 +1,317 @@ | |||
| # encoding: utf-8 | |||
|
|
|||
| require File.expand_path("../../spec_helper", __FILE__) | |||
tombruijn
Apr 16, 2017
Member
Wouldn't require "spec_helper" work here too?
Wouldn't require "spec_helper" work here too?
| ## 2. Add operating system packages ## | ||
|
|
||
| # Dependencies for developing and running Backup | ||
| # * The Nokogiri gem requires libxml2 |
tombruijn
Apr 16, 2017
Member
Doesn't nokogiri vendor libxml2 nowadays? Might no longer be needed with the upgrade to nokogiri 1.7 #845
Doesn't nokogiri vendor libxml2 nowadays? Might no longer be needed with the upgrade to nokogiri 1.7 #845
| rvm: | ||
| - 2.1.0 | ||
| - 2.2.0 | ||
| - 2.3.0 |
tombruijn
Apr 16, 2017
Member
why remove the run of the unit test suite from TravisCI? In the docker setup we don't test against other Ruby versions
why remove the run of the unit test suite from TravisCI? In the docker setup we don't test against other Ruby versions
| matrix: | ||
| include: | ||
| - rvm: "2.3.0" | ||
| script: "bundle exec rubocop" |
tombruijn
Apr 16, 2017
Member
I would keep rubocop here as well
I would keep rubocop here as well
| gem.add_development_dependency "mongo" | ||
| gem.add_development_dependency "bson_ext" | ||
| gem.add_development_dependency "redis" | ||
| gem.add_development_dependency "riak-client" |
tombruijn
Apr 16, 2017
Member
I would move these deps to a special gemfile for the docker container. Like the current setup for the Gemfile, but another one in integration/
I would move these deps to a special gemfile for the docker container. Like the current setup for the Gemfile, but another one in integration/
|
Oh and to clarify, I don't mean we need to drop running the unit test suite in a docker container entirely. I'd like to have that as well, but not as the only way to test it on travis. |
|
To be honest, I don't think that this kind of code review is the best next step for a couple of reasons. The first is that I have to change how things works as I go, so I can't provide clean PR steps until I reach a milestone and stop. The second is that just looking at individual files is probably not helpful and may be actively confusing (for example, the unit tests do run, they are called as a dependent task). Currently, I have been trying to get sudo to work in order to be able to finish a working first version that meets a reduced scope: running Docker locally and with Travis to run the unit tests, the integration tests for local files, plus MySQL and PostgreSQL. Unfortunately, using an unprivileged user by default forces me to rework how Docker runs (again), so it's currently broken. The idea would be to try to get that minimum scope merged somehow, partly because the size of this is just getting bigger, and partly for my own sanity: this is a big job, and it's become a real drain. |
|
I'm not quite sure what your goal you're trying to reach at this point. I don't know what exactly it is you're setting up, but I'd like to discuss it if you're having problems with getting it set up or want to discuss an approach for something easy to review and something we can split up in smaller pieces. (I did see that the unit tests are run in docker on travis, but my problem with it was that it's only being run in the docker container, under one ruby version.)
I understand it's becoming a drain. Not shown my face in this repo in quite a while, but I've been trying to do some more this weekend and feeling the burden again of making this work, realizing why I had kind of stepped back. There are real structural problems to Backup that make development on it a real pain. Tried to refactor some minor things today, just one spec, and pulling on that thread unraveled a whole mess. Anyway, I think we should approach this more structurally if we want to fix this any time soon. Seeing as the unit test suite's use is quite limited, as it mocks basically everything, it basically hinders refactoring every step of the way. We should put emphasis on this integration test suite going forward. If that's set up we can rework the unit test suite with more confidence. I've set up a #backup-dev room on Gitter if anyone wants to join. Going to put some thought in there for some refactoring. Let me know if anyone has any ideas too. https://gitter.im/backup/backup-dev |
|
This PR has been superseded by #857. |
I that case closing it would be a good idea. |
This PR implements support for running Backup inside Docker containers, as discussed in #807.
Once something like this gets to the master branch, I will write up docs to explain use of Docker in testing and development, per #812.
The Rake tasks provide facilities for:
In each case, the project directory on the host system is shared with the main container, and appears as the directory /usr/src/backup/ inside the running container.
Interactive Shell
The RUBYPATH environment variable is set for the interactive shell, so commands will work inside the container, if the Ruby interpreter is given the -S option, e.g.
Integration Tests
The docker:test:integration task uses the test suite in the integration/ directory, which currently contains copies of tests from the vagrant/ directory. All ported tests pass, but only the tests that uses local files have been ported so far.
Quirks
There are a couple of other quirks, due to using the gemspec file for dependencies:
The docker:build and docker:test:prepare tasks create images with different tags by design. Images created by "docker build" and docker-compose are not identical even with the same content, so if you use the same tag for images created by the two methods, Docker will remove the tag entirely from the first image that you created.
Known Issues
For Travis CI, we will need to consider if or how to support the current Rubocop test and running the RSpec suite on macOS.
The Dockerfile currently runs processes as root (the Docker default), which makes the sudo tests somewhat useless. I intend to change it to run commands as an unprivileged user in the container, and add a sudo configuration.
There is currently no tested process for using the image created with rake docker:build. To make Docker images as releasable artifacts, we would obviously need to inject the application code into the image, as well as solve practical issues about configuration, logging etc., so it's just an experimental thing right now.
The Docker Compose file specifies containers for network services, but the tests have not been ported yet.Tests for MySQL and PostgreSQL have been ported, along with local rsync tests, but tests for other network services still need to be ported or re-implemented.Similarly, the "live" tests that use services like S3 have yet to be ported across.