-
Notifications
You must be signed in to change notification settings - Fork 21.6k
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
ActiveRecord eager load test failure with certain test order #6465
Comments
I've made some changes to my commit. @pmahoney You can cherry-pick it and open your pull request. @rafaelfranca Thanks for your reviews and suggestions. |
So guys, any update on this? |
I am struggling with git here... I pulled yahonda's patch into my threadsafe-connection-pool branch, and I no longer get those test failures. Since threadsafe-connection-pool was already pushed to github (and merged and then reverted from rails master), I did a "git merge master", and it now seems a mess to do a pull request on that branch. I'm preparing another branch that has a squashed commit including everything. Is this the right way to go? It may lose info about the source of yahonda's changes for example. But I'm still getting test failures: https://gist.github.com/2788538 And I get similar failures on both master and my threadsafe-connection-pool branch, so I don't think my patch is causing any of these. On the other hand, I don't see these failures on ci. |
@rafaelfranca Thanks for the advice. I think I've got it: #6488 |
Please go ahead to close this issue. I believe it has been resolved. |
Aha! I forgot this. Thanks @yahonda |
With master c1487f6 I am able to get test failures when running a particular subset of tests. At least two other people (@yahonda @sikachu) have reproduced the same error. The command to run the test subset:
The errors: https://gist.github.com/b63ff56442df508b4300
It seems that the tests assume ActiveRecord has already cached some metadata when it has not done so (
SELECT name FROM sqlite_master ...
). Is there a way to force loading of this metadata before the test?@yahonda has a possible fix here: yahonda@d002176
This failure doesn't happen when running the full test suite. But it does happen with the full suite on postgres after applying this patch #6416 (full disclosure: my interests are mostly selfish; I want my pull request accepted :P ).
The text was updated successfully, but these errors were encountered: