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.

Sign up for GitHub

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

Make sure annotate_rendered_view_with_filenames is configured #130

Merged
merged 3 commits into from
Mar 19, 2024
Merged

Make sure annotate_rendered_view_with_filenames is configured #130

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e050e08
Make the dummy app bootable in test
etiennebarrie Mar 19, 2024
ea19da1
Make sure annotate_rendered_view_with_filenames is configured
etiennebarrie Mar 19, 2024
bcc021f
Support actionview 6.0
etiennebarrie Mar 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,8 @@
*.rbc
capybara-*.html
.rspec
/log
/tmp
/db/*.sqlite3
/db/*.sqlite3-journal
test/dummy/log
/public/system
/coverage/
/spec/tmp
Expand Down
1 change: 1 addition & 0 deletions Gemfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ source "https://rubygems.org"
gemspec

gem "actionview"
gem "railties"
gem "minitest"
gem "mocha"
gem "rake"
Expand Down
41 changes: 41 additions & 0 deletions Gemfile.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,16 @@ PATH
GEM
remote: https://rubygems.org/
specs:
actionpack (7.1.3.2)
actionview (= 7.1.3.2)
activesupport (= 7.1.3.2)
nokogiri (>= 1.8.5)
racc
rack (>= 2.2.4)
rack-session (>= 1.0.1)
rack-test (>= 0.6.3)
rails-dom-testing (~> 2.2)
rails-html-sanitizer (~> 1.6)
actionview (7.1.3.2)
activesupport (= 7.1.3.2)
builder (~> 3.1)
Expand Down Expand Up @@ -41,6 +51,10 @@ GEM
erubi (1.12.0)
i18n (1.14.4)
concurrent-ruby (~> 1.0)
io-console (0.7.2)
irb (1.12.0)
rdoc
reline (>= 0.4.2)
json (2.7.1)
language_server-protocol (3.17.0.3)
loofah (2.22.0)
Expand All @@ -65,19 +79,41 @@ GEM
pry-byebug (3.10.1)
byebug (~> 11.0)
pry (>= 0.13, < 0.15)
psych (5.1.2)
stringio
racc (1.7.3)
rack (3.0.9.1)
rack-session (2.0.0)
rack (>= 3.0.0)
rack-test (2.1.0)
rack (>= 1.3)
rackup (2.1.0)
rack (>= 3)
webrick (~> 1.8)
rails-dom-testing (2.2.0)
activesupport (>= 5.0.0)
minitest
nokogiri (>= 1.6)
rails-html-sanitizer (1.6.0)
loofah (~> 2.21)
nokogiri (~> 1.14)
railties (7.1.3.2)
actionpack (= 7.1.3.2)
activesupport (= 7.1.3.2)
irb
rackup (>= 1.0.0)
rake (>= 12.2)
thor (~> 1.0, >= 1.2.2)
zeitwerk (~> 2.6)
rainbow (3.1.1)
rake (13.1.0)
rake-compiler (1.2.7)
rake
rdoc (6.6.2)
psych (>= 4.0.0)
regexp_parser (2.9.0)
reline (0.4.3)
io-console (~> 0.5)
rexml (3.2.6)
rubocop (1.62.1)
json (~> 2.3)
Expand All @@ -99,9 +135,13 @@ GEM
ruby_memcheck (2.3.0)
nokogiri
smart_properties (1.17.0)
stringio (3.1.0)
thor (1.3.1)
tzinfo (2.0.6)
concurrent-ruby (~> 1.0)
unicode-display_width (2.5.0)
webrick (1.8.1)
zeitwerk (2.6.13)

PLATFORMS
ruby
Expand All @@ -112,6 +152,7 @@ DEPENDENCIES
minitest
mocha
pry-byebug
railties
rake
rake-compiler
rubocop-shopify
Expand Down
2 changes: 1 addition & 1 deletion gemfiles/Gemfile-rails-6-0
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ source "https://rubygems.org"
gemspec path: ".."

gem "actionview", "~> 6.0.0"
gem "rake"
gem "railties"
gem "rake-compiler"
gem "minitest"
gem "mocha"
Expand Down
2 changes: 1 addition & 1 deletion gemfiles/Gemfile-rails-6-1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ source "https://rubygems.org"
gemspec path: ".."

gem "actionview", "~> 6.1.0"
gem "rake"
gem "railties"
gem "rake-compiler"
gem "minitest"
gem "mocha"
Expand Down
4 changes: 4 additions & 0 deletions lib/better_html/railtie.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,12 @@ module BetterHtml
class Railtie < Rails::Railtie
initializer "better_html.better_erb.initialization" do
BetterHtml::BetterErb.prepend!
end

config.after_initialize do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverting this change I still get the test passing

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fixed the test.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test was working fine. When running test_helper, the first time Rails wasn't defined and lib/better_html.rb wouldn't require the railtie:

better-html/lib/better_html.rb

Line 27 in d2e71f5

require "better_html/railtie" if defined?(Rails)

But then when the specific test file would be loaded, we would make sure the railtie is loaded on its own. Since the actual config-copying code only runs when ActionView::Base is loaded, and that only happens later in the test with this hack, the initializer code copying the config executed and the test was checking that the config was indeed copied

better-html/test/better_html/railtie_test.rb

Line 14 in 72e7bc8

_ = ActionView::Base

Still your commit makes it better because the railtie will always be loaded before the dummy application is initialized, the way it's done in a real Rails app. In other words the tests don't rely on the fact that it all happens in an on_load(:action_view). 👍

ActiveSupport.on_load(:action_view) do
next unless ActionView::Base.respond_to?(:annotate_rendered_view_with_filenames)

BetterHtml.config.annotate_rendered_view_with_filenames = ActionView::Base.annotate_rendered_view_with_filenames
end
end
Expand Down
19 changes: 19 additions & 0 deletions test/better_html/railtie_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# frozen_string_literal: true

require "test_helper"
# setup dummy app
ENV["RAILS_ENV"] ||= "test"
require_relative "../dummy/config/environment"
# load railtie
require "better_html/railtie"

module BetterHtml
class RailtieTest < ActiveSupport::TestCase
if Rails::VERSION::STRING >= "6.1"
test "configuration is copied from ActionView" do
_ = ActionView::Base
assert BetterHtml.config.annotate_rendered_view_with_filenames
end
end
end
end
5 changes: 3 additions & 2 deletions test/dummy/config/application.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ class Application < Rails::Application
# config.i18n.load_path += Dir[Rails.root.join('my', 'locales', '*.{rb,yml}').to_s]
# config.i18n.default_locale = :de

# Do not swallow errors in after_commit/after_rollback callbacks.
config.active_record.raise_in_transactional_callbacks = true
if Rails::VERSION::STRING >= "6.1"
config.action_view.annotate_rendered_view_with_filenames = true
end
end
end
5 changes: 0 additions & 5 deletions test/dummy/config/environments/test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,6 @@
# Disable request forgery protection in test environment.
config.action_controller.allow_forgery_protection = false

# Tell Action Mailer not to deliver emails to the real world.
# The :test delivery method accumulates sent emails in the
# ActionMailer::Base.deliveries array.
config.action_mailer.delivery_method = :test

# Randomize the order test cases are executed.
config.active_support.test_order = :random

Expand Down
13 changes: 0 additions & 13 deletions test/dummy/config/initializers/assets.rb
View file
Open in desktop

This file was deleted.

Loading