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

before_all in minitest leaks data across test classes #266

Open
quorak opened this issue Apr 24, 2023 · 6 comments
Open

before_all in minitest leaks data across test classes #266

quorak opened this issue Apr 24, 2023 · 6 comments
Labels

Comments

@quorak
Copy link

quorak commented Apr 24, 2023

Tell us about your environment

Ruby Version:
2.7.3
Framework Version (RSpec, Minitest, FactoryGirl, Rails, whatever):
minitest (5.18.0)
factory_bot (5.2.0)
rails (6.1.7.3)

TestProf Version:
test-prof (1.2.1)

What did you do?

I have 2 test classes, one of those with before_all where I create some data.

class IndexControllerTest < ActionDispatch::IntegrationTest
        before_all do
                create(:record)
        end
       ....[many tests].....
end

class SomeOtherControllerTest < ActionDispatch::IntegrationTest
        test 'I would not expect to see any reords from another test class' do
             assert_equal 0, Record.count
        end
end

What did you expect to happen?

I would expect not to see data from IndexControllerTest.before_all while running tests in SomeOtherControllerTest

What actually happened?

I saw data from another test classes before_all data.

This only happens during parallel test execution. It looks like, tests are running randomly and the test from SomeOtherControllerTest is using a open transaction from IndexControllerTest that has been filled with data from before_all and has not been rolled back yet.


Needless to day, that this is a great idea and a awesome gem!
Thanks for helping!

@palkan
Copy link
Collaborator

palkan commented May 18, 2023

This only happens during parallel test execution

Do you use threads for parallelization (parallelize(with: :threads))? Also, do you parallelize all tests or only some of them?

As I see, we use a class instance variable here, which is not thread-safe:

def run_one_method(klass, method_name)
return super unless klass.respond_to?(:parallelized) && klass.parallelized
if @previous_klass && @previous_klass != klass
@previous_klass.before_all_executor&.deactivate!
end
@previous_klass = klass
super
end

@palkan palkan added awaiting response Awaiting reporter's response investigation required labels May 18, 2023
@quorak
Copy link
Author

quorak commented Jun 29, 2023

thanks @palkan for looking into this. It happens when executing it with workers, so no threads.

@palkan
Copy link
Collaborator

palkan commented Jul 25, 2023

Oh, that's interesting. That means that our activation code is not triggered and data is created outside of transactions.

Do you use fixtures or multiple databases or anything else dealing with DBs in your tests? Any details would be appreciated, so we can reproduce it ourselves.

@quorak
Copy link
Author

quorak commented Aug 6, 2023

We are using factory_bot (no fictures) and the standard rails 6 parallel test setup where multiple databases.

What is really confusing, that the spanning transaction does not only spans around all tests in a test classes but also around tests in other test classes.

It might be related, that rails is running tests in any order across all assigned tests.

Does this make sense?

@palkan
Copy link
Collaborator

palkan commented Sep 12, 2023

rails is running tests in any order across all assigned tests.

As far as I can see, Rails enqueues methods one by one to be executed in the same order (using a FIFO queue). So, we shouldn't have the situation when tests from different suites are executed by the same worker interchangeably:

                        |main process| 
                               |                                                                             
                               |                                                                                                           
{A#test_one, A#test_three,A#test_two, B#test_two,B#test_one,B#test_three}
                               |
                             queue
                            /      \
                           /        \
                   |worker #1|      |worker #2|
                        |                   |
{A#test_one,A#test_three,B#test_two}    {A#test_two,B#test_one,B#test_three} 

We shouldn't have situations like {A#test_one,B#test_two,A#test_three} {B#test_one,A#test_two,B#test_three}, which would result in race conditions in before_all. Rails uses DRb over Unix sockets under the hood, which should provide delivery guarantees.

To figure out what's going on it would be helpful to see some logs:

  • SQL queries logs (you can use Test Prof logging functionality)
  • Parallel worker logs: which klass/method is being executed. We can obtain such logs by patching the perform_job method.

@palkan palkan added the stale label Nov 14, 2023
@palkan
Copy link
Collaborator

palkan commented Nov 14, 2023

Hey @quorak! Any updates on this?

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

No branches or pull requests

2 participants