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

Fix exception raised by ActiveSupport Object#in? #1405

Merged
merged 1 commit into from Feb 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -327,8 +327,8 @@ def association?
end

def collection_association?
association? && association_reflection.macro.in?(
[:has_many, :has_and_belongs_to_many],
association? && [:has_many, :has_and_belongs_to_many].include?(
association_reflection.macro,
)
end

Expand Down
89 changes: 89 additions & 0 deletions spec/acceptance/active_record_integration_spec.rb
@@ -0,0 +1,89 @@
require 'acceptance_spec_helper'

describe 'shoulda-matchers integrates with active record' do
before do
create_active_record_project

write_file 'Rakefile', <<-FILE
require 'active_record'
require 'sqlite3'

namespace :db do
desc 'Create the database'
task :create do
File.unlink 'test.sqlite3' if File.exist?('test.sqlite3')
db = SQLite3::Database.new('test.sqlite3')
db.execute("CREATE TABLE users (id integer)")
db.execute("CREATE TABLE profiles (id integer, user_id integer)")
end
end
FILE

run_rake_tasks!('db:create')

write_file 'lib/user.rb', <<-FILE
require 'active_record'

class User < ActiveRecord::Base
end
FILE

write_file 'lib/profile.rb', <<-FILE
require 'active_record'
require 'user'

class Profile < ActiveRecord::Base
belongs_to :user
validates_presence_of :user
end
FILE

write_file 'spec/profile_spec.rb', <<-FILE
require 'spec_helper'
require 'profile'

describe Profile, type: :model do
it { should validate_presence_of(:user) }
end
FILE

updating_bundle do
add_rspec_to_project
add_shoulda_matchers_to_project(
manually: true,
with_configuration: false,
)

write_file 'spec/spec_helper.rb', <<-FILE
require 'active_record'
require 'shoulda-matchers'

RSpec.configure do |config|
Copy link
Collaborator

Choose a reason for hiding this comment

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

I feel like if people are wanting to integrate with ActiveRecord, that we should support/document using Shoulda::Matchers.configure (which will add includes for the right libraries) to set this up. What do you think about changing this to:

Shoulda::Matchers.configure do |config|
  config.integrate do |with|
    with.library :active_record
    with.test_framework :rspec
  end
end

Alternatively, when you call add_shoulda_matchers_to_project you can specify the configuration which will add the Shoulda::Matchers.configure line automatically, so what do you think about:

add_shoulda_matchers_to_project(
  libraries: [:active_record]
)

Copy link
Collaborator Author

@vsppedro vsppedro Jan 30, 2021

Choose a reason for hiding this comment

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

I agree.

I tried to make it work with the second option, but the problem is that the shoulda matches configuration is being added to the rails_helper.rb instead of spec_helper.rb.

I think I found the problem, but I'm not sure how to fix it. I'll keep trying.

FWIW, the problem is that is finding rspec-rails in the Gemfile because it's checking on the wrong Gemfile - gemfiles/rails_6_0.

if integrates_with_rspec?(test_framework)
files <<
if bundle.includes?('rspec-rails')
'spec/rails_helper.rb'
else
'spec/spec_helper.rb'
end
end

Values obtained in the file above using pry-byebug:

[1] pry(#<AcceptanceTests::AddsShouldaMatchersToProject>)> bundle
=> #<Tests::Bundle:0x0000555b8b6bf800 @already_updating=false, @fs=#<Tests::Filesystem:0x0000555b8b6bf7d8>>
[2] pry(#<AcceptanceTests::AddsShouldaMatchersToProject>)> bundle.includes?('rspec-rails')
=> true
[3] pry(#<AcceptanceTests::AddsShouldaMatchersToProject>)> ENV['BUNDLE_GEMFILE']
=> "<path-to>/shoulda-matchers/gemfiles/rails_6_0.gemfile"
[4] pry(#<AcceptanceTests::AddsShouldaMatchersToProject>)> 

Correct me if I'm wrong, please, does the value of BUNDLE_GEMFILE need to be equal to / tmp / shoulda-matchers-accept / test-project / Gemfile at that point in the code, or not?

EDIT: I tried to change the environment variable, but it did not work. I'll keep looking.

Copy link
Collaborator

Choose a reason for hiding this comment

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

In a "normal" app, yes, it would make sense for BUNDLE_GEMFILE to be the same as the application's Gemfile. For the tests we do a sort of hack where we force it to be the Appraisal gemfile instead. The reason we do that is because we want the Appraisal to dictate the list of gems that are loaded, as that is something we can control. As far as how BUNDLE_GEMFILE gets set, that comes from Appraisal. The 6.0 gemfile is the default because that's the latest one, but you You can override the Gemfile being used by using the appraisal command:

bundle exec appraisal rails_5_2 rspec ...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I get it. Thanks!

I am thinking of using the first approach you suggested. I couldn't get it to work with the second.

What do you think?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh that's fine to use the first option! I didn't realize it only added to rails_helper.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks! Merging!

config.before(:suite) do
ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: 'test.sqlite3')
end
end

Shoulda::Matchers.configure do |config|
config.integrate do |with|
with.test_framework :rspec

with.library :active_record
with.library :active_model
end
end
FILE
end
end

context 'when using both active_record and active_model libraries' do
it 'allows the use of matchers from both libraries' do
result = run_rspec_tests('spec/profile_spec.rb')

expect(result).to have_output('1 example, 0 failures')
expect(result).to have_output(
'is expected to validate that :user cannot be empty/falsy',
)
end
end
end
2 changes: 2 additions & 0 deletions spec/support/acceptance/helpers.rb
@@ -1,4 +1,5 @@
require_relative 'helpers/active_model_helpers'
require_relative 'helpers/active_record_helpers'
require_relative 'helpers/base_helpers'
require_relative 'helpers/command_helpers'
require_relative 'helpers/gem_helpers'
Expand All @@ -20,6 +21,7 @@ def self.configure_example_group(example_group)
end

include ActiveModelHelpers
include ActiveRecordHelpers
include BaseHelpers
include CommandHelpers
include GemHelpers
Expand Down
11 changes: 11 additions & 0 deletions spec/support/acceptance/helpers/active_record_helpers.rb
@@ -0,0 +1,11 @@
require_relative 'gem_helpers'

module AcceptanceTests
module ActiveRecordHelpers
include GemHelpers

def active_record_version
bundle_version_of('activerecord')
end
end
end
13 changes: 13 additions & 0 deletions spec/support/acceptance/helpers/step_helpers.rb
Expand Up @@ -17,6 +17,19 @@ def create_active_model_project
add_gem 'activemodel', active_model_version
end

def create_active_record_project
create_generic_bundler_project
add_gem 'activemodel', active_model_version
add_gem 'activerecord', active_record_version
add_gem 'rake'

if rails_version =~ '~> 6.0'
add_gem 'sqlite3', '~>1.4'
else
add_gem 'sqlite3', '~>1.3.6'
end
end

def create_generic_bundler_project
fs.clean
fs.create
Expand Down