Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

inconsistent spec results #54

Closed
AlainPilon opened this Issue Mar 14, 2013 · 6 comments

Comments

Projects
None yet
2 participants

Given these specs:

describe 'MultiAccountStore' do
  let(:store) { MultiAccountStore.new(@redis) }
  let(:user_1) { FactoryGirl.create(:user)}
  let(:users) { [user_1, user_2] }
  let(:ip) { '1.1.1.1' }

  before(:each) do
    @redis = REDIS
  end

  context '.store' do
    it 'persists the data' do
      store.ips.size.should eql(0)
      store.store(ip, users)
      store.ips.size.should eql(1)
    end

    it 'does not persist data when there is a single User' do
      store.store(ip, [user_1])
      store.ips.size.should eql(0)
    end
  end
end

when I run the specs, they randomly pass or fail. The failing lines are the store.ips.size.should eql(0) Which, instead of being empty (which they should since the redis instance is restarted at every test), still contains some left over data.

NOTE: I did add:

    gem "fakeredis", :require => "fakeredis/rspec"

I must have ran the file 100 times now and I can't find any pattern in the success/failure rates.

Collaborator

caius commented Mar 20, 2013

Hey,

Thanks for reporting this, I'll take a look and see if I can see what's going on here.

Collaborator

caius commented Mar 20, 2013

Can you give any more information on how the MultiAccountStore class is using the (fake)redis instance here? Is it a hash of ip => users? array of ips? keys named after the ips storing the users?

Here is the full class.

class MultiAccountStore
  IPS_KEY = 'multi_account_ips'
  ACCOUNTS_KEY = 'multi_accounts_for_ip'

  def initialize(store = REDIS)
    @store = store
  end

  def store(ip, users)
    if users.size > 1
      @store.sadd IPS_KEY, ip
      users.each do |user|
        @store.sadd "#{ACCOUNTS_KEY}_#{ip}", user.id
      end
    end
  end

  def ips
    @store.smembers IPS_KEY
  end

  def accounts_for_ip(ip)
    user_ids = @store.smembers "#{ACCOUNTS_KEY}_#{ip}"
    User.where(id: user_ids)
  end
end
Collaborator

caius commented Mar 20, 2013

Hmm, can't reproduce that failure here with that code. I've noticed you're using a REDIS constant to pick up the connection in the specs though - where is that constant assigned? If it's done before the fakeredis library is loaded, then you're more than likely still talking to your actual redis instance - which isn't cleared out by requiring "fakeredis/rspec", and would explain your persistence issue in the specs.

I'd probably suggest trying changing the before block from @redis = REDIS to @redis = Redis.new and seeing if that solves the issue. If so, either try requiring fakeredis before you assign a connection to REDIS, or carry on creating new Redis instances in your test setup (it's all in-process & in-memory, so there's no real speed issue there.)

Thx for trying, but the REDIS instance is initialized in the first file to be loaded in the initializer so I doubt that is the issue. Also, the issue did not happen every time. Sometimes, I could run the specs 5 times in a row and everything would be fine, then the next run would make it crash.

I also played with the MockRedis gem and had the same issue so maybe it is something in my code...

My solution was to flush the DB at each test, as you suggested.

Collaborator

caius commented May 20, 2014

Sorry we couldn't figure out a conclusive fix for this, but I'm glad you've figured out a solution. Going to close this now, feel free to reopen if you come up with anything else that we could try to track this down.

@caius caius closed this May 20, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment