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

Records created in before(:all) are cleared within examples. #564

Open
sevab opened this issue Jul 30, 2018 · 16 comments
Open

Records created in before(:all) are cleared within examples. #564

sevab opened this issue Jul 30, 2018 · 16 comments

Comments

@sevab
Copy link

sevab commented Jul 30, 2018

I recently switched our config from transactional to truncation strategy as shown in the Rspec + Capybara example in Readme.

However, this seems to affect the data created in the before(:all). With transaction strategy, records created in before(:all) were persisted and available between examples, as expected. With truncation strategy, however, records created within before(:all) are only available in the first test, and are then removed.

Is this an expected behaviour/side-effect of transactional strategy?

@cody-code-wy
Copy link

This also, perhaps unsupprisingly, also happens with the deletion strategy.

@ahorek
Copy link

ahorek commented Aug 31, 2018

hi, yes it's an expected behaviour, transaction strategy requires a block and it'll clean only records created within the block

DatabaseCleaner.strategy = :transaction
DatabaseCleaner.start
dirty_the_db
DatabaseCleaner.clean

on the other hand truncation and deletion strategy just clean the whole database except persistant tables

DatabaseCleaner.strategy = :deletion, {except: persistant_tables}
DatabaseCleaner.start
dirty_the_db
DatabaseCleaner.clean

it's basically the same as this

dirty_the_db
DatabaseCleaner.clean_with(:deletion, {except: persistant_tables})

@coding-bunny
Copy link

It's not working as described however.
We use the :transaction strategy like this in our RSpec suite, and this was working fine on 1.7.
However now with the 1.8 upgrade, the majority of our tests fail with errors like Cannot find object with ID xxxxxxxx because database_cleaner is wiping all the data between the tests.

RSpec.configure do |config|
  config.before(:suite) do
    # DatabaseCleaner configuration:  Clean the database, but run tests inside a transaction.
    DatabaseCleaner.clean_with(:truncation)
    DatabaseCleaner.strategy = :transaction

    NamedSeeds.load_seed
    ActiveJob::Base.queue_adapter.enqueued_jobs.clear
  end

  config.around do |example|
    non_transactional.each(&:delete_all)

    DatabaseCleaner.start
    example.run
    Temping.teardown
    NamedSeeds.reset_cache
    DatabaseCleaner.clean
    StuffedBunny.reset!

    # Reset Faker after each test to free up the unique values it generates.
    # Failure to do this will result in a Faker::UniqueGenerator::RetryLimitExceeded error otherwise, and values
    # only need to be unique during a test.
    ::Faker::UniqueGenerator.clear
  end
end

The output mostly is:

 ActiveRecord::RecordNotFound:
         Couldn't find Style with 'id'=416528006

So with the new version all data is being wiped constantly, and our entire seed that we setup when the test-suite starts is removed.

@botandrose
Copy link
Contributor

@coding-bunny Hmm, yes, it does sound like the 1.8 release introduced a regression. If I can reproduce it, I can fix it! I tried to reproduce the error you're experiencing by taking what you shared and chopping it down to just ActiveRecord and DatabaseCleaner here: https://gist.github.com/botandrose/5dc4c20e5f3d63cee191cbf336f3a542 , but the test passes.

Can you try to reduce your test suite down to a minimal failing example that you can share with me?

@coding-bunny
Copy link

Euh...I can try when i find the time, because this is a client's private source code, with a mySQL and Oracle database setup, so this won't be easy to get outside and visible.
I'll try to spend some time tomorrow to see if i can pin it down to mySQL or Oracle, perhaps it's database specific!?

@botandrose
Copy link
Contributor

Yes, perhaps you are right!

@coding-bunny
Copy link

I haven't been able to do much around this, but I can tell it's happening on our MySQL database, and all tests fail that depend on our named seeds, e.g data that's created during the test-suite initialization, at the beginning of the RSpec configuration block.

It seems that once we define the before/after configuration for database_cleaner, all data is being wiped.

@coding-bunny
Copy link

This is what we have inside the configuration:

# SimpleCov goes before anything being loaded, to ensure that all code paths are covered
require "simplecov"
SimpleCov.start("rails") do
  add_filter "/spec/"
  add_filter "/config/"
  add_filter "/vendor/"
  add_filter "Capfile"
  add_filter "config.ru"
  add_filter "Gemfile"
  add_filter "Guardfile"
  add_filter "Rakefile"

  add_group "Controllers", "app/controllers"
  add_group "Jobs", "app/jobs"
  add_group "Mappers", "app/mappers"
  add_group "Models", "app/models"
  add_group "Serializers", "app/serializers"
  add_group "Services", "app/services"
  add_group "Storages", "app/storages"
  add_group "Uploaders", "app/uploaders"
end

# This file is copied to spec/ when you run 'rails generate rspec:install'
ENV["RAILS_ENV"] ||= "test"
require File.expand_path("../config/environment", __dir__)
# Prevent database truncation if the environment is production
abort("The Rails environment is running in production mode!") if Rails.env.production?
require "spec_helper"
require "rspec/rails"
# Add additional requires below this line. Rails is not loaded until this point!
require "faker"
require "stuffed_bunny"
require "pry"

Dir[Rails.root.join("spec/support/**/*.rb")].each { |f| require f }

# Checks for pending migration and applies them before tests are run.
# If you are not using ActiveRecord, you can remove this line.
ActiveRecord::Migration.maintain_test_schema!

ActiveJob::Base.queue_adapter = :test

RSpec.configure do |config|
  # Remove this line if you're not using ActiveRecord or ActiveRecord fixtures
  config.fixture_path = "#{::Rails.root}/spec/fixtures"

  config.infer_spec_type_from_file_location!

  # We cannot use transactional fixtures due the way our Oracle callbacks work.
  # They require restore points to exist, which is not the case when this is enabled.
  config.use_transactional_fixtures = false

  config.infer_base_class_for_anonymous_controllers = false
  config.run_all_when_everything_filtered = true
  config.filter_run focus: true
  config.filter_run_excluding broken: true
  config.order = "random"
  config.tty = true

  include FactoryBot::Syntax::Methods
  RSpec::Core::ExampleGroup.include NamedSeeds::TestHelper
  RSpec::Core::ExampleGroup.named_seeds :anchor_complement_groups,
                                        :anchor_complements,
                                        :bulk_uploads,
                                        :bulk_upload_files,
                                        :categories,
                                        :category_filter_groups,
                                        :colors,
                                        :constraints,
                                        :decorator_metadata,
                                        :default_unit_prices,
                                        :filter_groups,
                                        :filter_tags,
                                        :guidelines,
                                        :pricing_groups,
                                        :rgbs,
                                        :color_sizes,
                                        :sizing_charts,
                                        :skus,
                                        :style_sizes,
                                        :styles,
                                        :supplier_unique_ids,
                                        :supporting_images,
                                        :turntimes,
                                        :view_images,
                                        :views

  ## This seeds the object space with legacy models so they can be iterated as 'truncation' models
  legacy_dir = "#{Rails.root}/app/models/legacy*/*.rb"
  Dir[legacy_dir].each { |f| require f }
  non_transactional = ObjectSpace.each_object(Class).select do |klass|
    # subclass of legacy model and no schema name
    klass < Legacy::Model && klass.table_name !~ /[\.]{1}/
  end

  config.before(:suite) do
    DatabaseCleaner.clean_with :truncation
    DatabaseCleaner.strategy = :transaction
    NamedSeeds.load_seed
    ActiveJob::Base.queue_adapter.enqueued_jobs.clear
  end

  config.around do |example|
    non_transactional.each(&:delete_all)
    DatabaseCleaner.cleaning do
      example.run
      Temping.teardown
      NamedSeeds.reset_cache
    end
    StuffedBunny.reset!

    # Reset Faker after each test to free up the unique values it generates.
    # Failure to do this will result in a Faker::UniqueGenerator::RetryLimitExceeded error otherwise, and values
    # only need to be unique during a test.
    ::Faker::UniqueGenerator.clear
  end
end

We basically generated data with the named_seeds gem for those specific classes.
And we tell database_cleaner then to run everything in a transaction with truncation, except for specific classes that are mapped to Oracle.

And all tests fail with the named seed usage.

@botandrose
Copy link
Contributor

Good job narrowing it down to MySQL and not Oracle! That's really good news because it makes it more likely that we'll be able to reduce this down to a test that fails on CI!

@Ex-Ark
Copy link

Ex-Ark commented Apr 14, 2020

Having kind of the same problem, I'm creating object in rspec shared_context, and this specific object is getting deleted by the time my test is run.

I have a minimal setup for now, so I don't need records to be deleted around examples, as per configuration :

# spec/spec_helper.rb
RSpec.configure do |config|
  config.before(:suite) do
    DatabaseCleaner.strategy = :transaction
    DatabaseCleaner.clean_with(:truncation)
  end
end

I have a shared_context, when included it creates a user. I added debug prompts :

# spec/support/shared_contexts/authorization_header.rb
require 'devise/jwt/test_helpers'

shared_context 'authorization_header_swagger' do
  byebug #  User.count => 6
  user = FactoryBot.create :user
  byebug #  User.count => 7
  let(:'Authorization') { Devise::JWT::TestHelpers.auth_headers({}, user)['Authorization'] }
end

And finally in the test where I need the user to exist, it is as if the user was never created (back to 6 entities instead of 7)

# spec/requests/some_test.rb
context 'with auth" do
        include_context 'authorization_header_swagger'

        it 'byebug' do
          byebug # User.count => 6
        end
end

gems:

  • database_cleaner (1.8.4)
  • database_cleaner-active_record (1.8.0)
  • mysql2 (0.5.3)

db:

  • 5.7.26 - MySQL Community Server (GPL)

Maybe, as the transaction mode was enabled, I need to flush or commit after creating any objects in contexts ?

@botandrose
Copy link
Contributor

@Ex-Ark thank you for sharing this example! It looks relatively simple, so I think we should be able to pare this down to a failing test. Is it possible for you to share this in a github repo that reproduces the problem? If you can do that, I'd be happy to take it from there!

@Ex-Ark
Copy link

Ex-Ark commented Apr 24, 2020

Sure thing, here is the most basic repo I could create to reproduce what I mentionned in my comment.
(rails api boilerplate with rspec and database_cleaner)
https://github.com/Ex-Ark/databasecleaner_issue_564

From there you can add before(:all) in tests to try OP's setup.

@botandrose
Copy link
Contributor

Hi @Ex-Ark! Thank you so much for sharing this repo. It's super helpful to be able to look at the same code to figure out what's going on!

After taking a look, I think that the bug is actually in how the test is using RSpec's shared context. Specifically, the user creation code is happening directly in the shared_context block, instead of in a before block inside of that block:

# incorrect
shared_context 'a_user_exist' do
  user = FactoryBot.create :user
end

# correct
shared_context 'a_user_exist' do
  before { user = FactoryBot.create :user }
end

When I change the code in the repo to the second version, the test passes. What do you think? Is this indeed the culprit, or maybe I've missed something? I wasn't able to modify the example with before(:all) and get it to fail, so if you know a way to do that, that would be something to look at, too.

Oh, and here are the RSpec docs for shared_context: https://relishapp.com/rspec/rspec-core/docs/example-groups/shared-context

Thoughts?

@Ex-Ark
Copy link

Ex-Ark commented Apr 27, 2020

Thanks for taking a look at this !
The code in the sample repo is of course not 100% what I have in my app, as I need to have the user in a memoized variable because of how swagger works.

Anyway I refactored my context using your approach, in fact I declared everything in let helper, and it worked !

Strange thing is that I used (wrongly ? 👎 ) rspec context until now and it always worked before I included database_cleaner into my test suite.

Not sure if it applies to OP's problem, but it is fixed for my case !

# what I had
shared_context 'authorization_header_swagger' do
  user = FactoryBot.create(:user)
  let(:'Authorization') { Devise::JWT::TestHelpers.auth_headers({}, user )['Authorization'] }
end

# what I have now
shared_context 'authorization_header_swagger' do
  let(:'Authorization') { Devise::JWT::TestHelpers.auth_headers({}, FactoryBot.create(:user))['Authorization'] }
end

I did not try using before(:all).

@botandrose
Copy link
Contributor

@Ex-Ark you're welcome! I'm glad we were able to resolve your issue. Since it didn't ultimately involve before(:all), though, I don't think I can close this issue, yet. @coding-bunny if you can Share a reproduction repo with me, or modify the one shared above to demonstrate your error, I'd be happy to take a look!

@Victorcorcos
Copy link

Victorcorcos commented Jul 1, 2020

@botandrose I am facing the same issue as the author of this PR @sevab

And I took a time to write a very simple way to reproduce the issue

The issue

I am facing a very anonying issue with the DatabaseCleaner gem when dealing with the before(:all) blocks.

When using the before(:all) block, the database items will be available on the next it block, but they will not be available on all subsequent it blocks.

Please take a look at the simple code snippet below...

require 'rails_helper'

RSpec.describe Record, type: :model do
  describe 'when creating a Project' do
    before(:all) do
      Project.delete_all
      @project = Project.create(name: 'Test')
    end

    it 'should have the Project created' do
      expect(Project.count).to be_eql(1)
    end

    it 'should still have the Project created' do
      expect(Project.count).to be_eql(1)
    end

    it 'should also still have the Project created' do
      expect(Project.count).to be_eql(1)
    end
  end
end

☝️
Before configuring the DatabaseCleaner on the system, this test will pass with success. However, after configuring the DatabaseCleaner, it will pass just on the first it block and fail on all the subsequent ones, on this case, it will fail on these ones...

  • should still have the Project created
  • should also still have the Project created

Before I applied the DatabaseCleaner gem, I was doing the job to clean the database by myself. I was trully believing the DatabaseCleaner would do something similar, but it is cleaning too much. A whole bunch of tests failed on my test suite.

I had this line on the version before applying the DatabaseCleaner gem... And this test was working correctly.

require 'rails_helper'

RSpec.describe Record, type: :model do
  describe 'when creating a Project' do
    before(:all) do
      Project.delete_all # Here is the change!
      @project = Project.create(name: 'Test')
    end

    it 'should have the Project created' do
      expect(Project.count).to be_eql(1)
    end

    it 'should still have the Project created' do
      expect(Project.count).to be_eql(1)
    end

    it 'should also still have the Project created' do
      expect(Project.count).to be_eql(1)
    end
  end
end

The configuration

I configured my spec_helper.rb file with this:

RSpec.configure do |config|
  # Configure DatabaseCleaner to automatically reset the database between tests
  config.before(:suite) do
    DatabaseCleaner[:mongoid].strategy = :truncation
  end

  config.around(:each) do |example|
    DatabaseCleaner.cleaning do
      example.run
    end
  end
end

How can I solve this?

If I change the before(:all) to simple before blocks, the problem is solved. However, the test suite become 1000x times slower, I would like to maintain the before(:all) blocks in tests that makes sense to use them.

DatabaseCleaner/database_cleaner-mongoid#15

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

No branches or pull requests

7 participants