🔧 Make Suspenders set up the test environment #1261
Conversation
Similar to the last rewrite, we generate a new gem and do a directory swap, while making sure to preserve important files like NEWS.md. This branch will serve as a new base for us to merge into while we build up an [application template][template]. This approach is largely inspired by [staples][]. [template]: https://guides.rubyonrails.org/generators.html#application-templates [staples]: https://github.com/stevepolitodesign/staples
Introduces [Procfile][1] to automatically start the server and run migrations during the [release phase][2]. Additionally, we update our [database connection preference][3] to account for [nuance in connecting with Heroku][4]. [1]: https://devcenter.heroku.com/articles/getting-started-with-rails8#create-a-procfile [2]: https://devcenter.heroku.com/articles/release-phase [3]: https://guides.rubyonrails.org/configuring.html#connection-preference [4]: https://discuss.rubyonrails.org/t/brainstorming-approaches-to-reconcile-rails-8s-default-multi-db-setup-with-database-url/86769
Before, the gem generated the default configuration for the test environment. We wanted to raise errors when we missed a translation. We enabled this configuration in the gem for the test environment. [GitHub](#1259) Co-authored-by: Jose Blanco <jose.blanco@thoughtbot.com>
|
@stevepolitodesign, how might we change these tests so that we don't need to include Rails in the Gemfile? It seems unnecessary, and it also makes our test feedback loops too long |
Gemfile
Outdated
| gem "rails" | ||
| gem "rubocop-rails-omakase" |
There was a problem hiding this comment.
@purinkle we shouldn't need to add these, was there a particular error you ran into that forced you to add these?
There was a problem hiding this comment.
Without those lines, we get the following
bundle exec rspec spec/environments/test_spec.rb:15
Run options: include {:locations=>{"./spec/environments/test_spec.rb"=>[15]}}
Randomized with seed 35384
Test environment
/Users/purinkle/.asdf/installs/ruby/3.3.1/lib/ruby/gems/3.3.0/gems/bundler-2.4.19/lib/bundler/rubygems_integration.rb:308:in `block in replace_bin_path': can't find executable rails for gem railties. railties is not currently included in the bundle, perhaps you meant to add it to your Gemfile? (Gem::Exception)
from /Users/purinkle/.asdf/installs/ruby/3.3.1/lib/ruby/gems/3.3.0/gems/bundler-2.4.19/lib/bundler/rubygems_integration.rb:336:in `block in replace_bin_path'
from /Users/purinkle/.asdf/installs/ruby/3.3.1/bin/rails:25:in `<main>'
enables the raising of errors for missing translations (FAILED - 1)
Failures:
1) Test environment enables the raising of errors for missing translations
Failure/Error: raise Error, "Failed to create Rails app"
Suspenders::Error:
Failed to create Rails app
# ./lib/suspenders/cli.rb:41:in `generate_new_rails_app'
# ./lib/suspenders/cli.rb:21:in `run'
# ./lib/suspenders/cli.rb:16:in `run'
# ./spec/environments/test_spec.rb:29:in `block in generate'
# ./spec/environments/test_spec.rb:28:in `chdir'
# ./spec/environments/test_spec.rb:28:in `generate'
# ./spec/environments/test_spec.rb:7:in `block (3 levels) in <top (required)>'
# ./spec/environments/test_spec.rb:24:in `with_temp_directory'
# ./spec/environments/test_spec.rb:6:in `block (2 levels) in <top (required)>'
Finished in 0.12298 seconds (files took 0.08589 seconds to load)
1 example, 1 failure
Failed examples:
rspec ./spec/environments/test_spec.rb:5 # Test environment enables the raising of errors for missing translations
Randomized with seed 35384
shell returned 1
Press ENTER or type command to continue
There was a problem hiding this comment.
spec/environments/test_spec.rb
Outdated
There was a problem hiding this comment.
Ah, was this spec forcing you to update the Gemfile?
We actually don't need specs for any of the work we're doing, since the new CI script builds a new application with the Suspenders executable, and runs the test suite against the generated application (although I need to update that line to use RSpec).
I recognize this means a bit of manual testing is involved, but in general, I feel good about this approach, since CI has us covered.
So, for this commit (and future ones) you'd want to run...
bundle exec rake install
suspenders demo
...and check that the affected files were updated.
There was a problem hiding this comment.
Jose and I spent the end of the day playing around with introducing a dummy app
It is much faster and allows us to TDD our changes
What do you think?
I want to introduce a Rake task to keep the dummy app updated, but I can add that later
There was a problem hiding this comment.
Ohh, this is interesting. I think in this case we'd want the dummy app to have been generated with the same flags: rails new dummy -d=postgresql --skip-test --skip-solid.
There was a problem hiding this comment.
I want to introduce a Rake task to keep the dummy app updated, but I can add that later
We can probably just do this every time there's a new Rails release.
There was a problem hiding this comment.
As for the CI failures, we can update Standard to ignore the dummy app.
There was a problem hiding this comment.
I created #1263 as another approach. If we're happy with that approach, I'll close that pull request and introduce the changes here.
There was a problem hiding this comment.
I think one "risk" to adding a dummy app that is it adds to the overall size of the library. I don't think this is a necessarily a problem, but wanted to point it out.
That being said, I'm still on the fence about adding a dummy app for TDD purposes. This is because I personally think Suspenders does not benefit from TDD. This new release is really just an application template, which uses Thor under the hood. Writing tests feels a bit like we're testing Thor than anything else.
I feel like the most valuable test can really only be done manually, which is generating a new app, pushing it to GitHub and ensuring CI passes, and deploying to Heroku. This is where the current version of Suspenders fails. The new CI script gets us pretty close for now.
I recognize it feels cumbersome right now because we're re-implementing all the features. But, once we're done, there likely will not be a lot of churn in the codebase.
Before, the gem generated the default configuration for the test environment. We wanted to disable the show dispatch exception. We enabled this configuration in the gem for the test environment. Co-authored-by: Rob Whittaker <rob@thoughtbot.com>
laicuRoot
force-pushed
the
make-test-translations
branch
from
b361381 to
290dad7
Compare
Lifts from the existing [testing][], [factories][] and [ci][] generators, while also relying on the fact that some of these files will already exist. For example, `.github/workflows/ci.yml` will be generated by Rails, we just need to modify the file. The previous version of Suspenders could be invoked on an **existing** application, but this new version is intended to be used when creating **new** applications. Since we're calling `rails new` with `--skip-test` we know that the `.github/workflows/ci.yml` will be missing the test steps. Also, this commit enables all the "recommended" settings in `spec_helper`, which is a change from the current version. This commit also updates the library's CI script to account for the fact that the generated applicaiton uses RSpec. [testing]: https://github.com/thoughtbot/suspenders/blob/main/lib/generators/suspenders/ci_generator.rb [factories]: https://github.com/thoughtbot/suspenders/blob/main/lib/generators/suspenders/ci_generator.rb [ci]: https://github.com/thoughtbot/suspenders/blob/main/lib/generators/suspenders/ci_generator.rb
Lifted from the current [jobs generator][]. The only new addition is we enable the web UI for local development, and configure for [Heroku][]. Since this commit adds a new process, we introduce `Procfile.dev`, and update `bin/dev` based on [prior art][]. [jobs generator]: https://github.com/thoughtbot/suspenders/blob/main/lib/generators/suspenders/jobs_generator.rb [Heroku]: https://github.com/sidekiq/sidekiq/wiki/Heroku [prior art]: https://github.com/rails/jsbundling-rails/blob/main/lib/install/dev
Lifted from existing [views generator][]. Note that we add a new `app/views/application/_form_errors.html.erb` partial, and no longer depend on the `title` gem, since Rails ships with a similar solution. We also do not concern ourselves with any CSS framework (for now). [views generator]: https://github.com/thoughtbot/suspenders/blob/main/lib/generators/suspenders/views_generator.rb
Lifted from the existing [accessibility generator][]. [accessibility generator]: https://github.com/thoughtbot/suspenders/blob/main/lib/generators/suspenders/accessibility_generator.rb
Lifted from [email generator][]. However, we simplify the implementation by removing the `include ActiveSupport::Configurable`. Instead, we just create a simple PORO and place it in `lib`. [email generator]: https://github.com/thoughtbot/suspenders/blob/main/lib/generators/suspenders/email_generator.rb
Lifted from the existing [inline_svg generator][]. [inline_svg generator]: https://github.com/thoughtbot/suspenders/blob/main/lib/generators/suspenders/inline_svg_generator.rb
Lifted from [development_generator][]. However, we do not need to enable `annotate_rendered_view_with_filenames` or `query_log_tags_enabled`, since those are now enabled by default. Also enables `apply_rubocop_autocorrect_after_generate!` since this is a new option since the last release. [development_generator]: https://github.com/thoughtbot/suspenders/blob/main/lib/generators/suspenders/environments/development_generator.rb
…1270) Lifted from [production generator][]. Also adds the following new settings: - Enables [strict_loading_by_default][]. - Sets [strict_loading_mode][] to `:n_plus_one`. - Enables [sandbox_by_default][]. - Sets [action_on_strict_loading_violation][] to `:log`. This commit also introduces `ASSET_HOST` and `APPLICATION_HOST` environment variables. Finally, since we require the Rails Master Key, we update CI. [production generator]: https://github.com/thoughtbot/suspenders/blob/main/lib/generators/suspenders/environments/production_generator.rb [strict_loading_by_default]: https://guides.rubyonrails.org/configuring.html#config-active-record-strict-loading-by-default [strict_loading_mode]: https://guides.rubyonrails.org/configuring.html#config-active-record-strict-loading-mode [sandbox_by_default]: https://guides.rubyonrails.org/configuring.html#config-sandbox-by-default [action_on_strict_loading_violation]: https://guides.rubyonrails.org/configuring.html#config-active-record-action-on-strict-loading-violation
Prior to this commit, we were getting errors when running `bundle exec rake install`.
laicuRoot
changed the title
laicuRoot
changed the title
Alternative to #1261, lifted from the existing [generator][]. However, since the authoring of that generator, the structure of `config/environments/test.rb` has changed, so we no longer need conditionals. Additionally, we set `action_dispatch.show_exceptions` to `:none` rather than comment it out to be explicit about the value. [generator]: https://github.com/thoughtbot/suspenders/blob/main/lib/generators/suspenders/environments/test_generator.rb Co-authored-by: Rob Whittaker <rob@thoughtbot.com> Co-authored-by: Jose Blanco <53355525+laicuRoot@users.noreply.github.com>
Alternative to #1261, lifted from the existing [generator][]. However, since the authoring of that generator, the structure of `config/environments/test.rb` has changed, so we no longer need conditionals. Additionally, we set `action_dispatch.show_exceptions` to `:none` rather than comment it out to be explicit about the value. [generator]: https://github.com/thoughtbot/suspenders/blob/main/lib/generators/suspenders/environments/test_generator.rb Co-authored-by: Rob Whittaker <rob@thoughtbot.com> Co-authored-by: Jose Blanco <53355525+laicuRoot@users.noreply.github.com>
Alternative to #1261, lifted from the existing [generator][]. However, since the authoring of that generator, the structure of `config/environments/test.rb` has changed, so we no longer need conditionals. Additionally, we set `action_dispatch.show_exceptions` to `:none` rather than comment it out to be explicit about the value. [generator]: https://github.com/thoughtbot/suspenders/blob/main/lib/generators/suspenders/environments/test_generator.rb Co-authored-by: Rob Whittaker <rob@thoughtbot.com> Co-authored-by: Jose Blanco <53355525+laicuRoot@users.noreply.github.com>
|
Closed by #1282 |
Before, the gem generated the default configuration for the test environment. We wanted to raise errors when we missed a translation and disable action dispatch show exception. We enabled this configuration in Suspenders for the test environment.
GitHub