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

Add :on_rails hook to rspec #15

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ group :development do
end

group :test do
gem 'rails', '~> 7.1.1'
gem 'rspec', '~> 3.2'
gem 'simplecov', '~> 0.21'
end
Expand Down
38 changes: 33 additions & 5 deletions Rakefile
Original file line number Diff line number Diff line change
@@ -1,10 +1,38 @@
# frozen_string_literal: true

require 'bundler/gem_tasks'
require 'rubocop/rake_task'
require 'rspec/core/rake_task'

RSpec::Core::RakeTask.new(:spec)
RuboCop::RakeTask.new
task default: %i[rubocop spec]

task default: %i[spec rubocop]
desc 'Run RuboCop'
task :rubocop do
require 'rubocop/rake_task'

RuboCop::RakeTask.new
end

desc 'Run tests'
task :spec do
require 'rspec/core/rake_task'

RSpec::Core::RakeTask.new(:spec)
end

RAILS_DUMMY_DIR = 'tmp/test/dummy/'

namespace :rails do
desc 'Generate dummy rails application'
task :generate_dummy_app, [:dir] do |_t, args|
dummy_app_dir = args.fetch(:dir, RAILS_DUMMY_DIR)
exit(0) if Dir.exist?(dummy_app_dir)

system(
"rails new #{dummy_app_dir} " \
'--skip-git --skip-asset-pipeline --skip-action-cable ' \
'--skip-action-mailer --skip-action-mailbox --skip-action-text ' \
'--skip-active-record --skip-active-job --skip-active-storage ' \
'--skip-javascript --skip-hotwire --skip-jbuilder ' \
'--skip-test --skip-system-test --skip-bootsnap ',
)
end
end
7 changes: 7 additions & 0 deletions spec/rspec_settings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,20 @@
require 'simplecov'
SimpleCov.start do
add_filter '/spec/'
add_filter %r{^/tmp/}
end

require 'uber_task'
require 'colorize'

require_relative 'support/rails_manager'

RSpec.configure do |config|
config.expect_with :rspec do |c|
c.syntax = :expect
end

config.around(:example, :on_rails) do |example|
RailsManager.new(config).on_rails { example.run }
end
end
47 changes: 47 additions & 0 deletions spec/support/rails_manager.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
# frozen_string_literal: true

require 'rake'

class RailsManager
APP_PATH = '../../tmp/test/dummy/'

attr_reader :config, :app_path

def initialize(config, app_path: APP_PATH)
@config = config
@app_path = app_path
end

def on_rails
load_rails!
yield
unload_rails!
end
Comment on lines +15 to +19
Copy link

Choose a reason for hiding this comment

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

Consider adding error handling.

Ensure that load_rails! and unload_rails! handle potential errors gracefully.

-  load_rails!
-  yield
-  unload_rails!
+  begin
+    load_rails!
+    yield
+  ensure
+    unload_rails!
+  end
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def on_rails
load_rails!
yield
unload_rails!
end
def on_rails
begin
load_rails!
yield
ensure
unload_rails!
end
end


private

def load_rails!
return if defined?(Rails)

if config.instance_variable_defined?(:@rails)
Object.const_set(:Rails, config.instance_variable_get(:@rails))
return
end

absolute_app_path = File.expand_path(app_path, __dir__)
system("rake rails:generate_dummy_app[#{absolute_app_path}]")

require File.expand_path(
File.join(absolute_app_path, '/config/environment.rb'),
__dir__,
)

ENV['RAILS_ROOT'] = absolute_app_path
config.instance_variable_set(:@rails, Object.const_get(:Rails))
end
Comment on lines +23 to +41
Copy link

Choose a reason for hiding this comment

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

Ensure the system call succeeds.

Add error handling for the system call that generates the dummy Rails app.

-  system("rake rails:generate_dummy_app[#{absolute_app_path}]")
+  unless system("rake rails:generate_dummy_app[#{absolute_app_path}]")
+    raise "Failed to generate dummy Rails app at #{absolute_app_path}"
+  end
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def load_rails!
return if defined?(Rails)
if config.instance_variable_defined?(:@rails)
Object.const_set(:Rails, config.instance_variable_get(:@rails))
return
end
absolute_app_path = File.expand_path(app_path, __dir__)
system("rake rails:generate_dummy_app[#{absolute_app_path}]")
require File.expand_path(
File.join(absolute_app_path, '/config/environment.rb'),
__dir__,
)
ENV['RAILS_ROOT'] = absolute_app_path
config.instance_variable_set(:@rails, Object.const_get(:Rails))
end
def load_rails!
return if defined?(Rails)
if config.instance_variable_defined?(:@rails)
Object.const_set(:Rails, config.instance_variable_get(:@rails))
return
end
absolute_app_path = File.expand_path(app_path, __dir__)
unless system("rake rails:generate_dummy_app[#{absolute_app_path}]")
raise "Failed to generate dummy Rails app at #{absolute_app_path}"
end
require File.expand_path(
File.join(absolute_app_path, '/config/environment.rb'),
__dir__,
)
ENV['RAILS_ROOT'] = absolute_app_path
config.instance_variable_set(:@rails, Object.const_get(:Rails))
end


def unload_rails!
ENV.delete('RAILS_ROOT')
Object.send(:remove_const, :Rails)
end
Comment on lines +43 to +46
Copy link

Choose a reason for hiding this comment

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

Check if Rails is defined before removing the constant.

Add a check to ensure Rails is defined before attempting to remove the constant.

-  Object.send(:remove_const, :Rails)
+  Object.send(:remove_const, :Rails) if defined?(Rails)
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def unload_rails!
ENV.delete('RAILS_ROOT')
Object.send(:remove_const, :Rails)
end
def unload_rails!
ENV.delete('RAILS_ROOT')
Object.send(:remove_const, :Rails) if defined?(Rails)
end

end
47 changes: 47 additions & 0 deletions spec/support_specs/rails_manager_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
# frozen_string_literal: true

require 'tmpdir'

describe RailsManager do
let(:manager) { described_class.new(config, app_path: app_path) }

let(:config) { double }
let(:app_path) { File.join(tmp_dir, 'test/dummy') }

after do
FileUtils.remove_entry(tmp_dir)
end

describe '#on_rails', skip: 'Conflicts with manager in rspec_settings' do
Copy link
Collaborator Author

@zzaakiirr zzaakiirr Jul 9, 2024

Choose a reason for hiding this comment

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

It seems that RailsManager#unload_rails! isn't fully unloads rails application. Running this test separately works fine, but running all tests at once raises issue

If we run bundle exec rspec 2 rails applications will be initialized (first one when this test is run, second one is when test with :on_rails hook is executed), and error will be raised.

I couldn't figure out how to fix this issue, maybe i/o creating hook we should just run tests on rails and non-rails application environment 🤔 @borela WDYT?

it 'runs isolated code as if on Rails application' do
### 1st run
#
expect(Dir.exist?(app_path)).to eq false
expect(defined?(Rails)).to be_nil
expect(config.instance_variable_defined?(:@rails)).to eq false

manager.on_rails do
expect(defined?(Rails)).not_to be_nil
end

expect(Dir.exist?(app_path)).to eq true
expect(defined?(Rails)).to be_nil
expect(config.instance_variable_defined?(:@rails)).to eq true

### 2nd run
#
manager.on_rails do
expect(defined?(Rails)).not_to be_nil
expect(Rails).to eq config.instance_variable_get(:@rails)
end

expect(Dir.exist?(app_path)).to eq true
expect(defined?(Rails)).to be_nil
expect(config.instance_variable_defined?(:@rails)).to eq true
end
end

def tmp_dir
@tmp_dir ||= Dir.mktmpdir
end
end
9 changes: 2 additions & 7 deletions spec/uber_task/internal/path_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,20 +25,15 @@
end
end

context 'in rails' do
let(:rails_root) { Pathname.new(Dir.pwd) }
context 'on rails', :on_rails do
let(:paths) do
[
rails_root.join('abc').to_s,
Rails.root.join('abc').to_s,
'/foo/bar/baz/gems/bar',
'/foo/bar/rubygems/baz',
]
end

before do
stub_const('Rails', double(root: rails_root))
end

it 'shortens paths' do
shortened = paths.map do |path|
described_class.shorten(path)
Expand Down
8 changes: 8 additions & 0 deletions spec/uber_task/logger_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,14 @@
end.to output(/some message/).to_stdout
end
end

context 'when Rails logger is defined', :on_rails do
it 'uses Rails logger by default' do
expect(Rails.logger).to receive(:info).with('some message')

described_class.logger.info('some message')
end
end
end

describe '.logger=' do
Expand Down