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

Ensure fixtures are loaded for FoxyFixturesTest #29298

Merged
merged 1 commit into from
May 31, 2017

Conversation

alexcameron89
Copy link
Member

NOTE: Related to the Unlock minitest for Rails' test suite PR - #29271

Failing Seed

bin/test --seed 59389 test/cases/fixtures_test.rb
When tests are randomized, sometimes FoxyFixturesTests are failing due to unloaded
fixtures.

Solution

I followed along with what many of the other tests are doing in the file to bypass this issue by using self.use_transactional_tests = false.

Ensure that the fixtures are properly loaded for FoxyFixturesTest

When tests are randomized, FoxyFixturesTest often fails due to unloaded
fixtures.
@rails-bot
Copy link

warning Warning warning

  • Pull requests are usually filed against the master branch for this repo, but this one is against unlock-minitest. Please double check that you specified the right target!

@rafaelfranca rafaelfranca merged commit cc07560 into rails:unlock-minitest May 31, 2017
@matthewd
Copy link
Member

matthewd commented Jun 3, 2017

AFAICS, this is actually working around a bug in AR::Fixtures.

This cut-down test fails on master:

require "cases/helper"
require "models/parrot"
require "models/treasure"

class A < ActiveRecord::TestCase
  fixtures :parrots

  def test_fixtures_loaded
    # Dusty may or may not be present, depending on whether some other
    # test has loaded the :live_parrots fixture.
    assert_equal [nil, "Curious George", "King Louis", "frederick", "polly"], Parrot.order(:name).pluck(:name) - ["Dusty Bluebird"]
  end
end

class B < ActiveRecord::TestCase
  fixtures :parrots, :live_parrots

  def test_fixtures_loaded
    assert_equal [nil, "Curious George", "Dusty Bluebird", "King Louis", "frederick", "polly"], Parrot.order(:name).pluck(:name)
  end
end

Specifically, B fails because only "Dusty Bluebird" (that is, the entry from live_parrots.yml) is present.

@alexcameron89
Copy link
Member Author

@matthewd interesting, so we're losing the :parrots fixtures in class B when we ask for an additional fixture? They end up passing if ask for both in class A as well: fixtures :parrots, :live_parrots.

That would make sense for similar failures I've been catching.

@matthewd
Copy link
Member

matthewd commented Jun 4, 2017

Right. Because we first filter the requested fixtures to skip any that are already loaded, and then we empty out the table as part of inserting the loaded fixtures. A perfectly fine pair of strategies, unless two sets of fixtures are using the same table.

Assuming my intuition of the problem is right, I think we need to group the requested fixtures by their table name first, then decide whether the whole group can be skipped (because they're all already loaded) or not (if any of them are not).

Want to take a look?

@alexcameron89
Copy link
Member Author

Yeah, I'll take a look and work on that. 👍

alexcameron89 added a commit to alexcameron89/rails that referenced this pull request Aug 1, 2017
Problem

``` ruby
class A < ActiveRecord::TestCase
  fixtures :parrots, :cars

  def test_fixtures_loaded
    # Dusty may or may not be present, depending on whether some other
    # test has loaded the :live_parrots fixture.
    assert_equal [nil, "Curious George", "King Louis", "frederick", polly"], Parrot.order(:name).pluck(:name) - ["Dusty Bluebird"]
  end
end

class B < ActiveRecord::TestCase
  fixtures :parrots, :live_parrots, :cars

  def test_fixtures_loaded
    assert_equal [nil, "Curious George", "Dusty Bluebird", "King Louis", "frederick", "polly"], Parrot.order(:name).pluck(:name)
  end
end
```

If Class B runs after class A, class B's test will fail. Because
:parrots is already cached, it skips the fixture, but when it gets to
:live_parrots, it drops the table and loads up the :live_parrots
fixtures.

Solution

Keep a map of fixtures by table. If when it comes time to create
fixtures and the fixtures for the table match what is cached, then we
can skip creating them in the database. If the fixtures are short of
what we have cached, then the database needs to be cleaned to remove old
records.

See the conversation on rails#29298 for
more information.
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

Successfully merging this pull request may close these issues.

None yet

4 participants