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

Test installer #553

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Test installer #553

wants to merge 4 commits into from

Conversation

zzak
Copy link
Contributor

@zzak zzak commented Jan 11, 2024

This commit adds test coverage for the installer Rake task and application template. The installer is run against a freshly generated Rails app using the version of Rails that is currently loaded. Thus the installer can be tested with different versions of Rails in CI.

Similar to hotwired/stimulus-rails#136

The motivation is to be able to move these tests from railties, see: rails/rails#49679

@zzak
Copy link
Contributor Author

zzak commented Jan 11, 2024

FWIW, this morning I also looked at adding a test for turbo_needs_redis, but couldn't find an easy way to mock whether or not redis was available on the system.

This method here determines that, but it's not easily stubbed since the tasks are run in a different process:

def redis_installed?
Gem.win_platform? ?
system('where redis-server > NUL 2>&1') :
system('which redis-server > /dev/null')
end

I still think this could happen after, if anyone comes up with a clever way to test it, and we've at least increased the test coverage here so that we can continue on the railties side.

@zzak
Copy link
Contributor Author

zzak commented Feb 16, 2024

@afcapel Sorry for the ping, would you be able to review this one and hotwired/stimulus-rails#136?

test/installer_test.rb Outdated Show resolved Hide resolved
test/app_helper.rb Outdated Show resolved Hide resolved
test/app_helper.rb Outdated Show resolved Hide resolved
test/app_helper.rb Outdated Show resolved Hide resolved
test/installer_test.rb Outdated Show resolved Hide resolved
test/installer_test.rb Outdated Show resolved Hide resolved
@zzak
Copy link
Contributor Author

zzak commented Apr 19, 2024

@afcapel Thanks for your review! I've addressed your comments. 🙇

I think Rails 6.1 is broken, and this test will always fail:

Error:
InstallerTest#test_installer:
Errno::ENOENT: No such file or directory @ rb_sysopen - app/javascript/application.js
    test/installer_test.rb:15:in `read'
    test/installer_test.rb:15:in `block (2 levels) in <class:InstallerTest>'
    test/installer_test.rb:8:in `block in <class:InstallerTest>'

This is because the check always assumes that app/javascript/application.js exists, but in Rails 6.1 it's using webpack, see: 5c5a85d

/tmp/d20240419-2363909-lm6npn/turbo_test_app_ba923836b46f828536c9eea7e80c44f44a9ee2cf => cat app/javascript/packs/application.js
// This file is automatically compiled by Webpack, along with any other files
// present in this directory. You're encouraged to place your actual application logic in
// a relevant structure within app/javascript and only use these pack files to reference
// that code so it'll be compiled.

import Rails from "@rails/ujs"
import Turbolinks from "turbolinks"
import * as ActiveStorage from "@rails/activestorage"
import "channels"

Rails.start()
Turbolinks.start()
ActiveStorage.start()

Unless I'm missing something? 🤔 I thought this test was passing before, but if you think of anything obvious LMK!

@zzak
Copy link
Contributor Author

zzak commented Apr 19, 2024

🤦 I've fixed the tests for Rails 6.1, the red text made me thing it was an error but the install does complete and yarn add ... still works, so I guess we're good!

zzak and others added 4 commits November 4, 2024 13:35
This commit adds test coverage for the installer Rake task and application template. The installer is run against a freshly generated Rails app using the version of Rails that is currently loaded. Thus the installer can be tested with different versions of Rails in CI.

Similar to hotwired/stimulus-rails#136.

The motivation is to be able to move these tests from railties, see: rails/rails#49679

Co-authored-by: Jonathan Hefner <[email protected]>
My hunch is that this was fixed in Rails 7.1 by rails/rails#46074, but I'm less concerned here because this test suite uses a Dummy app for all of the tests.

This is a problem I've been working on in rails/rails#50427, to remove the dummy applications and replace them with a generated app like we've done here with the installer test.

I'm happy to investigate replacing the dummy app here afterwards.
```
LoadError: Error loading the 'sqlite3' Active Record adapter. Missing a gem it depends on? can't activate sqlite3 (~> 1.4), already activated sqlite3-2.0.0-x86_64-linux-gnu. Make sure all dependencies are added to Gemfile. (LoadError)

Caused by:
Gem::LoadError: can't activate sqlite3 (~> 1.4), already activated sqlite3-2.0.0-x86_64-linux-gnu. Make sure all dependencies are added to Gemfile. (Gem::LoadError)

Tasks: TOP => db:test:prepare => db:load_config
(See full trace by running task with --trace)
/home/zzak/.rbenv/versions/3.2.3/lib/ruby/gems/3.2.0/gems/bundler-2.5.4/lib/bundler/rubygems_integration.rb:237:in `block (2 levels) in replace_gem': Error loading the 'sqlite3' Active Record adapter. Missing a gem it depends on? can't activate sqlite3 (~> 1.4), already activated sqlite3-2.0.0-x86_64-linux-gnu. Make sure all dependencies are added to Gemfile. (LoadError)
```
@zzak
Copy link
Contributor Author

zzak commented Nov 4, 2024

@afcapel Could I trouble you for another review? 🙇

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

Successfully merging this pull request may close these issues.

2 participants