DatabaseCleaner / database_cleaner-mongoid

MIT License
9 stars 7 forks source link

DatabaseCleaner is removing items for subsequent it blocks when using before(:all) #15

Closed Victorcorcos closed 4 years ago

Victorcorcos commented 4 years ago

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 = 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...

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.

JonRowe commented 4 years ago

You are telling DatabaseCleaner to cleanup around every example and this is the expected outcome of that.

If you swap to this:

RSpec.configure do |config|
  # Use truncation and clean the database to start with.
  config.before(:suite) do
    DatabaseCleaner[:mongoid].strategy = :truncation
    DatabaseCleaner.clean
  end

  # Technically this is unneeded for truncation but is the equivalent of what you are doing
  config.before(:context) do |example|
    DatabaseCleaner.start
  end

  # Truncate the DB after each context.
  config.after(:context) do |example|
    DatabaseCleaner.clean
  end
end

You should find that DatabaseCleaner cleans after every describe or context block which might help you here, but it still won't survive between contexts.

Given you are sharing state between tests, (which is not a recommended practise) you may find it easier/more reliable to clean up test manually with something like:

RSpec.configure do |config|
  # Use truncation and clean the database to start with.
  config.before(:suite) do
    DatabaseCleaner[:mongoid].strategy = :truncation
    DatabaseCleaner.clean
  end

  config.before(:context, database_clean: true) do |example|
    DatabaseCleaner.clean
  end
end

RSpec.describe "My scenario inserting into the DB", database_clean: true do
  # clean runs before this but only before this
  before(:context) { insert_into_db }
  context do
    it {}
  end
  context do
    it {}
  end
end
Victorcorcos commented 4 years ago

Hi @JonRowe Thanks for the response!

I tried the first approach and this specific test with the before(:all) passed, but most part of my tests failed.

The issue is that when using before(:all), I want to remain with the database data for the its inside the context/describe block. (The example I just posted here). However, when using the before(:each) or just before, I want to clean the database data before each it inside the context/describe block.

I will show that illustratively

before(:all)

Screen Shot 2020-07-01 at 12 38 13

before

Screen Shot 2020-07-01 at 12 41 31

Victorcorcos commented 4 years ago

In other words..

This test should work (before(:all))

require 'rails_helper'

RSpec.describe Record, type: :model do
  describe 'when creating a Project' do
    before(:all) do
      @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

But this one also should work (before)

require 'rails_helper'

RSpec.describe Record, type: :model do
  describe 'when creating a Project' do
    before do
      @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
JonRowe commented 4 years ago

You will have to set this up manually so that cleaning occurs unless you mark that it shouldn't.

This is mostly a limitation of the truncation strategy, you simply can't have before every test database cleaning and have shared database setup.

Victorcorcos commented 4 years ago

@JonRowe 😢 Yep, looks like a limitation of the truncation strategy, because on the Active Record, I used the transaction strategy and everything worked perfectly fine.

If the mongoid enables the transaction strategy somehow, can be a nice solution

JonRowe commented 4 years ago

You basically want this:

RSpec.configure do |config|
  config.before(:suite) do
    DatabaseCleaner[:mongoid].strategy = :truncation
    DatabaseCleaner.clean
  end

  config.before(:example) do |example|
    unless example.metadata[:per_context_database_cleaning]
      DatabaseCleaner.clean
    end
  end

  config.before(:context, per_context_database_cleaning: true) do
    DatabaseCleaner.clean
  end
end

RSpec.describe do
  context "", per_context_database_cleaning: true do
    before(:context) { dirty_the_db } 
    it {}
    it {}
  end
  context do
    it {}
  end
end

Closing because this is a configuration issue really.

Victorcorcos commented 4 years ago

@JonRowe

Thank you very much for the response!

I really didn't understand why you closed this issue, since this still didn't work fully as expected here. 😢

I configured the spec_helper.rb with this

RSpec.configure do |config|
  config.before(:suite) do
    DatabaseCleaner[:mongoid].strategy = :truncation
    DatabaseCleaner.clean
  end

  config.before(:example) do |example|
    next if example.metadata[:context_cleaning]
    DatabaseCleaner.clean
  end

  config.before(:context, context_cleaning: true) do
    DatabaseCleaner.clean
  end
end

And these tests worked as expected:

require 'rails_helper'

RSpec.describe Record, type: :model do
  describe 'when creating a Project' do
    before do
      @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
require 'rails_helper'

RSpec.describe Record, type: :model do
  describe 'when creating a Project', context_cleaning: true do
    before(:all) do
      @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

However, many tests broke, because they can have nested describe/context blocks. For example, this test is not working...

require 'rails_helper'

RSpec.describe Record, type: :model do
  describe 'when creating a Project', context_cleaning: true do
    before(:all) do
      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

    describe 'the edge case' do
      before do
        Project.create(name: 'Test')
      end

      it 'should be equal 2' do
        expect(Project.count).to be_eql(2)
      end

      it 'should still be equal 2' do
        expect(Project.count).to be_eql(2) # Should be equal 3
      end
    end
  end
end

On the Active Record DatabaseCleaner, using the transaction strategy would avoid all of these contour solutions and would work fully as expected.

On this case, I should clean the database by myself?

If the mongoid enables the transaction strategy somehow, can be a really nice solution

JonRowe commented 4 years ago

I closed because this is not something database cleaner can fix, its just a configuration issue in your test suite. If you are sharing test state like this you will need to turn it off and on manually.

Victorcorcos commented 4 years ago

@JonRowe So why the transaction strategy on the Active Record worked? 🤔 For these examples that I showed.

JonRowe commented 4 years ago

Because transactions wrap examples and roll back the data afterwards, so there is a start and an end. Truncation just deletes everything.

Victorcorcos commented 4 years ago

@JonRowe Exactly!! As you said

this is mostly a limitation of the truncation strategy

Would be really nice if the mongoid database cleaner allows the transaction strategy. The new versions of mongoid allows transactions 🥇

Even after configuring all of these things, we still need to manually clean the database on the test suite. That's why I believe it is not just a configuration error, instead it is a limitation with the truncation strategy and it would be nice if mongoid database cleaner could allow the transaction strategy. 👍