-
Notifications
You must be signed in to change notification settings - Fork 254
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
Replace Vagrant with Docker Compose for running functional tests #539
Conversation
runs-on: ubuntu-latest | ||
strategy: | ||
matrix: | ||
ruby: ["2.0", "ruby"] |
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.
🗒️ "ruby"
in this context means the latest stable version of Ruby. As of this writing, it is 3.3.3. Using this keyword means the functional tests will always run against the latest version of Ruby, without us having to manually update the workflow.
@@ -21,10 +21,6 @@ namespace :test do | |||
|
|||
end | |||
|
|||
Rake::Task["test:functional"].enhance do | |||
warn "Remember there are still some VMs running, kill them with `vagrant halt` if you are finished using them." |
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.
🗒️ DockerWrapper
will automatically start Docker Compose on demand and clean it up after tests are done, so this instruction is no longer relevant.
@@ -1,17 +0,0 @@ | |||
[ |
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 JSON file defined 3 different host VMs. However, in practice, only 1 of these was ever used in the functional tests. This extra complexity wasn't serving a purpose, so I removed the JSON in favor of just hardcoding a single host.
assert true | ||
end | ||
|
||
def test_creating_a_user_gives_us_back_his_private_key_as_a_string |
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 seemed to be testing the Vagrant infrastructure itself, as opposed to testing functionality of SSHKit. It doesn't serve a purpose with Docker, so I removed it.
Minitest.after_run do | ||
DockerWrapper.stop if DockerWrapper.running? | ||
end |
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 ensures that the container is cleaned up at exit.
DockerWrapper.stop if DockerWrapper.running? | ||
end | ||
|
||
module DockerWrapper |
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 code was mostly lifted from capistrano/capistrano#2159.
Our functional tests had been failing in CI for quite some time. They relied on VirtualBox (managed by Vagrant), which no longer works reliably on the standard GitHub Actions runners. We disabled functional tests in CI due to this problem (see #523).
This PR fixes the functional tests by swapping out Vagrant for Docker Compose, following the example of capistrano/capistrano#2159.
Docker Compose works fine with GitHub Actions, so I also reenabled functional tests in CI, borrowing from #522.
And they work now!