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

fix factoryGirl screws up rake db:migrate process #2955

Merged
merged 1 commit into from May 17, 2015
Merged

fix factoryGirl screws up rake db:migrate process #2955

merged 1 commit into from May 17, 2015

Conversation

marutosi
Copy link
Contributor

@marutosi marutosi commented May 2, 2015

$ rake db:migrate RAILS_ENV=test

rake aborted!
ActiveRecord::StatementInvalid: Could not find table 'work_package_journals'

http://stackoverflow.com/questions/12423273/factorygirl-screws-up-rake-dbmigrate-process

@TeatroIO
Copy link

TeatroIO commented May 2, 2015

I've prepared a stage to preview changes. Open stage or view logs.

@myabc
Copy link
Contributor

myabc commented May 2, 2015

@marutosi you need to add require statements to test_helper.rb and spec_helper.rb.

@floriank
Copy link
Contributor

floriank commented May 2, 2015

☝️ what @myabc said. Otherwise the test suite(s) won't run properly.

$ rake db:migrate RAILS_ENV=test

rake aborted!
ActiveRecord::StatementInvalid: Could not find table 'work_package_journals'

http://stackoverflow.com/questions/12423273/factorygirl-screws-up-rake-dbmigrate-process
@myabc myabc merged commit 5a0c4bd into opf:release/4.0 May 17, 2015
@marutosi marutosi deleted the finn/FactoryGirl branch May 17, 2015 12:22
@NobodysNightmare
Copy link
Contributor

Sadly after merging this PR specs for plugins are broken... See discussion here: 30dcd66

@NobodysNightmare
Copy link
Contributor

I will for now revert this PR, because running specs is kind of important to me.

If anybody has feedback on:

  • how to fix this properly
  • why this causes trouble (because I really can't imagine why)

they should call me.

Best regards

@myabc
Copy link
Contributor

myabc commented May 20, 2015

reverting this PR is not really an option as this breaks migrations locally (I believe @machisuji also hit this issue a couple times)

@NobodysNightmare
Copy link
Contributor

@myabc I think your sentence was not complete.

The problem here is that we are in a workable state without this PR and in a completely not workable state with this PR. My third question could be when do I ever need to run rake db:migrate RAILS_ENV=test? Because I never did that before...

@machisuji
Copy link
Member

I can confirm that I do run into this problem locally every time I try to set up a config. Migrations fail. Specs don't run.

Doing the 'require: false' trick does help. Dunno why it breaks the specs on the CI. I think this is a good opportunity to take a thorough look at the problem and fix it once and for all. Any volunteers?

@NobodysNightmare
Copy link
Contributor

Dunno why it breaks the specs on the C

Just for the record: It does not break the specs on the CI. It breaks the specs for plugins! Locally as well as on the CI.

@machisuji Does bundle exec rake spec work for you? This is how I performed the setup step... not quite by the books, but it works.

I am fine with someone solving the issue once and for all, but it needs to work ^^

@myabc
Copy link
Contributor

myabc commented May 20, 2015

My third question could be when do I ever need to run rake db:migrate RAILS_ENV=test?

I find myself needing this surprisingly often. For instance, when if I modify the database schema and want to run a single spec.

@NobodysNightmare
Copy link
Contributor

I guess I did not run into that issue because of my rake spec workaround. Which is not a pretty one for sure.

@myabc
Copy link
Contributor

myabc commented May 20, 2015

@NobodysNightmare can you try moving the factory_girl_rails require further up, just before Rails is required.

require 'factory_girl_rails'
require File.expand_path('../../config/environment', __FILE__)

Then bundle exec rake spec:plugins.

This works for me, but not yet 100% sure why.

@myabc
Copy link
Contributor

myabc commented May 20, 2015

FactoryGirl does some magic to find definitions (see FactoryGirl::Railtie). I guess the Railtie needs to be required before the other parts of Rails. Admittedly, my knowledge of Rails initializers / Engine load order is pretty weak.

@myabc
Copy link
Contributor

myabc commented May 20, 2015

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