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 random failure related to migration environment #27603

Merged
merged 1 commit into from
Jan 8, 2017

Conversation

prathamesh-sonpatki
Copy link
Member

  • Reference: https://travis-ci.org/rails/rails/jobs/189764676

  • Reproduction command:

    MTB_VERBOSE=2 bundle exec minitest_bisect --seed 33328 -Itest "test/cases/migration_test.rb" "test/cases/tasks/database_tasks_test.rb"

  • You need to also add minitest_bisect gem to the Gemfile to reproduce
    this failure.

r? @schneems

- Reference: https://travis-ci.org/rails/rails/jobs/189764676

- Reproduction command:

    MTB_VERBOSE=2 bundle exec minitest_bisect --seed 33328 -Itest "test/cases/migration_test.rb" "test/cases/tasks/database_tasks_test.rb"

- You need to also add minitest_bisect gem to the Gemfile to reproduce
  this failure.
@kamipo
Copy link
Member

kamipo commented Jan 7, 2017

I was also affected and deleted environment = "foofoo" record in 13803a7.

@prathamesh-sonpatki
Copy link
Member Author

@kamipo I tried your patch locally.

This is the output on master with my reproduction script: https://gist.github.com/prathamesh-sonpatki/2da52556999822ddb919393d6ca83c57.

Very first failure: https://gist.github.com/prathamesh-sonpatki/2da52556999822ddb919393d6ca83c57#file-gistfile1-txt-L16

This is the output with your patch: https://gist.github.com/prathamesh-sonpatki/5fe2db5b07384be5036f829d7c5c10ab

First failure is: https://gist.github.com/prathamesh-sonpatki/5fe2db5b07384be5036f829d7c5c10ab#file-gistfile1-txt-L21

I think deleting the contents of the "metadata" table is not the right solution as the error about missing environment might occur. What do you think?

@kamipo
Copy link
Member

kamipo commented Jan 7, 2017

Ah... I missed requiring environment setting 😢
https://github.com/rails/rails/blob/v5.0.1/activerecord/lib/active_record/migration.rb#L1256-L1257

You are right, thank you for your time!

@spastorino spastorino merged commit 80bf338 into rails:master Jan 8, 2017
@prathamesh-sonpatki prathamesh-sonpatki deleted the fix-random-failure branch January 8, 2017 10:27
gussan pushed a commit to gussan/activerecord-turntable that referenced this pull request May 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants