-
Notifications
You must be signed in to change notification settings - Fork 11
Add GitHub Actions workflow for CI #223
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
Conversation
a0bf38e to
53cac28
Compare
This changes the permissions of isolock to match isolate, because the GitHub Actions runner user does not appear to be in their own group.
This fixes the following error during rake db:seed on Ubuntu 20.04: BCrypt::Errors::InvalidHash (invalid hash)
53cac28 to
cb30553
Compare
bagedevimo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great, I can't see any reason not to just merge it - low risk from my point of view.
.github/workflows/ci.yml
Outdated
| --health-cmd pg_isready | ||
| --health-interval 10s | ||
| --health-timeout 5s | ||
| --health-retries 5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know these health checks are the default from the sample, but I'd like to reduce the interval (maybe in a separate PR) because the 10s interval causes the "Initialize containers" steps to be needlessly slow (~20s). Reducing the interval to 1s (and increasing the retries to compensate) makes it faster (~10s).
(Ideally we would use start interval to set a short interval while waiting for the service to start and a longer interval afterwards to reduce CPU overhead, but --health-start-interval is relatively new and is not available here; need to wait for Docker v25.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure.
Since this code was originally written, v2 has been released. It natively supports the simplecov format, so it allows us to remove simplecov-lcov. This gives us nice HTML coverage reports when running locally. Also, with v2 we longer have to pass the GitHub token explicitly.
Settings RAILS_ENV=test for all steps is important, because script/install/config.bash only persists it for other install scripts, and failing to set RAILS_ENV can cause some subtle issues; e.g. Rails normally respects the environment variable DATABASE_URL (so we can remove "url" from config/database.yml), but things breaks if RAILS_ENV is not set to "test". This change also matches GitHub's Ruby on Rails CI template.
Replace $TRAVIS with $CI, and explicitly propagate the variable to debootstrap.bash via sudo (it wasn't necessary on Travis CI because Travis has a special sudo configuration that preserves certain environment variables[1]). [1] https://github.com/travis-ci/travis-cookbooks/blob/v7.0.0/ci_environment/travis_build_environment/files/default/etc/sudoers/env_keep Co-authored-by: Tom Levy <tomlevy93@gmail.com>
Switch from the deprecated coveralls gem to the Coveralls GitHub Action. The coveralls gem fails with SSLError [1] because it sets ssl_version to TLSv1, and its API endpoint is gone (404 Not Found). There is a community-maintained replacement called coveralls-ruby-reborn, but the official integration is through the Coveralls GitHub Action which is easy enough to use so we just switch to that. [1] https://www.github.com/lemurheavy/coveralls-ruby/issues/163 Co-authored-by: Tom Levy <tomlevy93@gmail.com>
Replace $TRAVIS with $CI, and explicitly propagate the variable to debootstrap.bash via sudo (it wasn't necessary on Travis CI because Travis has a special sudo configuration that preserves certain environment variables[1]). [1] https://github.com/travis-ci/travis-cookbooks/blob/v7.0.0/ci_environment/travis_build_environment/files/default/etc/sudoers/env_keep Co-authored-by: Tom Levy <tomlevy93@gmail.com>
Switch from the deprecated coveralls gem to the Coveralls GitHub Action. The coveralls gem fails with SSLError [1] because it sets ssl_version to TLSv1, and its API endpoint is gone (404 Not Found). There is a community-maintained replacement called coveralls-ruby-reborn, but the official integration is through the Coveralls GitHub Action which is easy enough to use so we just switch to that. [1] https://www.github.com/lemurheavy/coveralls-ruby/issues/163 Co-authored-by: Tom Levy <tomlevy93@gmail.com>
pg_isready works even if the username is invalid, however it causes the following error message to appear repeatedly in the postgres container log (which is displayed in the "Stop containers" step): ``` FATAL: role "root" does not exist ```
This option was inherited from .travis.yml and causes install.bash to skip the following steps (which are also done during update): - bundle install - script/install/migrate.bash - script/install/whenever.bash - (if in production) script/install/assets.bash The job currently works even without these steps, but it's fragile: the `bundle install` step is only safe to skip because we pass `bundler-cache: true` to ruby/setup-ruby; if we disable the cache then ruby/setup-ruby won't run `bundle install` and the build will fail. So don't use --skip-updates, in order to always run `bundle install` and ensure the build will work even without `bundler-cache: true`. (Running `bundle install` twice doesn't do any harm, it only takes ~0.27s the second time.) Not using --skip-updates has the a additional effect of running migrate.bash, which is probably a good thing because it exercises the migration code. The migrations take ~8s, which is a little slow but not a big deal. Note that the schema is dumped after migrating, so db:test:load will use the dumped schema rather than the one from the repository. We will add a check later on that reports an error if the dumped schema has any differences. Running whenever.bash is quick and doesn't do any harm.
Our install scripts create the database if it doesn't exist, but getting the postgres service to create the database is nicer and more consistent with the GitHub Actions Ruby on Rails CI template.
(Currently this doesn't do much because .standard.yml ignores all the files.)
The only semantic change is switching the postgres image from Debian-based to Alpine-based. There are a couple of differences compared to the GitHub Actions Ruby on Rails CI template: - The template only runs the job on push/pull-request for branch "master", but we run the job for all branches because we want to be able to push commits and have them tested without having to open a pull request. - The template runs on ubuntu-latest but we use ubuntu-20.04 because the build currently fails on ubuntu-latest. - The template doesn't have Docker health checks (for waiting until the services have started). - We have a "redis" service and a custom install script. - The template uses bin/rails etc., which we don't have yet. - The template has pretty step names, which we don't bother with. - The template's lint task uses bundler-audit, brakeman, and rubocop; we use standardrb.
Comment out the setting for now, so that we're actually testing the script that creates the database. Keeping it as a comment because it might be useful when we pare down the install scripts.
It's not required because we run db:migrate earlier. Also, db/schema.rb is slightly incomplete (it doesn't include the index index_users_on_username because of a Rails limitation).
|
Superseded by #246. |
The RDoc markup for the badges (as link images) wasn't rendered correctly on GitHub, and the file was using `code` for code snippets (Markdown syntax) rather than +code+ (RDoc syntax). Badge updates: - We now use GitHub Actions instead of Travis CI (#223) - Remove Waffle.io badge (the service has shut down)
MERGED as #246
This is an alternative to #82 that does not use a Dockerfile.
Note that the commands used are mostly copied from our previous CI setup in .travis.yml (which is no longer being used due to #147).