ErwinM / acts_as_tenant

Easy multi-tenancy for Rails in a shared database setup.
MIT License
1.53k stars 263 forks source link

current_tenant is nil in parallel tests #277

Closed jasper502 closed 2 years ago

jasper502 commented 2 years ago

My stock tests run fine but take forever. Was trying to speed them up with parallelization:

parallelize(workers: :number_of_processors)

If I add that I get failures like this on every test:

  test_should_get_index                                          ERROR (30.62s)
Minitest::UnexpectedError:         NoMethodError: undefined method `id' for nil:NilClass
            app/models/project.rb:43:in `block in <class:Project>'
            test/test_helper.rb:54:in `block in <class:TestCase>'

which is this line here:

searchkick index_prefix: -> { ActsAsTenant.current_tenant.id }

I am using the Searchkick gem and I have my indexes scoped to the tenant. Again this work fine and my test_helper looks like this:

  setup do
    DatabaseCleaner.start
    sign_in users(:one)
    @tenant = accounts(:company)
    ActsAsTenant.current_tenant = @tenant
    Project.reindex
  end

Over in the searchkick issues / docs there are suggestions like this:

  parallelize_setup do |worker|
    Searchkick.index_suffix = worker

    # reindex models
    Project.reindex

    # and disable callbacks
    Searchkick.disable_callbacks

  end

  parallelize_teardown do
    Project.search_index.delete
  end

    teardown do
    DatabaseCleaner.clean
  end

  parallelize(workers: :number_of_processors)

These don't help and it looks like the fixtures are not available in the parallelize_setup block to set the tenant. Anyone run into this before?

mikecmpbll commented 2 years ago

I had the same problem with rails system tests (which run the server in a different thread). It boiled down to the test_tenant implementation not being threadsafe. If you look at the implementation of the test tenant middleware, you can see it sets test_tenant to nil for the duration of the request;

https://github.com/ErwinM/acts_as_tenant/blob/5fa4851d1539e7004f31c853d100fa6ab162e0ba/lib/acts_as_tenant/test_tenant_middleware.rb#L7-L13

Because test_tenant modifications affect all threads, this obviously causes problems. A simple workaround is to patch test_tenant to use Thread.current:

module ActsAsTenant
  def self.test_tenant=(tenant)
    Thread.current[:test_tenant] = tenant
  end

  def self.test_tenant
    Thread.current[:test_tenant]
  end
end

I'll do a PR.

edit: thinking about your use-case again, you might need to set the test_tenant for each thread of the test runner (should be possible).