-
Notifications
You must be signed in to change notification settings - Fork 74
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
latest release (v0.10.0) breaks db tests #127
Comments
How do you set As I understand, tests fail with v0.10.0 and pass with v0.9.0, right? It would be helpful to see the ActiveRecord logs for a single failing test. You can enable AR logging by adding the following to your test setup ( ::ActiveRecord::Base.logger = ::ActiveSupport::LogSubscriber.logger = Logger.new(STDOUT) Or using a test-prof recipe. |
For setting
I am using stock Rails tests with fixtures. You are correct that my app's tests passed in v0.9.0 and started to fail in v0.10.0. I will take a look at your log suggestion tomorrow and come back with some more info. |
My original assumptions seem to be a bit off. Instead, at least in the first test I dug into, there are differences in how join queries are built for a model that has logidze with version v0.10.0. Without Logidze, the query begins with However, when |
The issue appears to be centralized to the |
Changing line 19 to |
If I set If there's any other info I can provide that would be helpful, or things you'd like me to try poking around in, just let me know. |
Thanks for the investigation! Really helpful! That's the breaking line, I think: logidze/lib/logidze/ignore_log_data.rb Line 29 in 9f97dc6
I gonna add a fix soon; for now you can override scope :with_log_data, lambda {
if ignored_columns == ["log_data"]
select("#{table_name}.*")
else
select(column_names + ["log_data"])
end
} |
This PR (#128) should fix this problem. I'm going to add more improvements in there but feel free to try this branch. |
Not sure if comments are better here or on your PR at this point, but your PR fixes one of the the initial issues that I submitted, but there are still others in my test suite that seem to fall into two categories. For my setup, I am not setting 1.
|
After updating to latest master
I changed |
Thanks!
Could you show a stack trace example, please? This exception is raised when you try to access |
One example is
The test is: test 'will enqueue email to admins if new user has role of "admin"' do
user = User.new(
email: 'new+user@example.com',
password: 'abc123!',
first_name: 'account_team',
last_name: 'test',
role: 'admin'
)
assert_enqueued_email_with AdminNotificationsMailer, :new_admin_user, args: [user] do
user.save!
end
end I can try to find more examples if you'd like. |
Thanks! I think, that should be enough. Pushed one more fix for this use-case: removed exception, reverted to returning |
It works! 🎉 All tests are passing with latest Awesome job! |
Tell us about your environment
Ruby Version: ruby 2.6.3p62 (2019-04-16 revision 67580) [x86_64-darwin18]
Rails Version: Rails 6.0.0.rc1
PostgreSQL Version: PostgreSQL 10.6 on x86_64-pc-linux-musl, compiled by gcc (Alpine 8.2.0) 8.2.0, 64-bit
Logidze Version: v0.10.0
What did you do?
Running tests that rely on test fixtures caused a bunch of failures that were seemingly unrelated to the tests being ran.
What did you expect to happen?
Tests would continue to pass.
What actually happened?
I have a feeling that this may be related to parallel tests and Rails stock test fixtures.
I kept my app on v0.9.0 and also got some recent console errors while trying to set a
responsible_id
in anActiveJob
. The error output wasWARNING: SET LOCAL can only be used in transaction blocks
I realize this isn't a very actionable error, but wanted to bring it up in case others ran into something similar. Feel free to close if it's not helpful.
The text was updated successfully, but these errors were encountered: