-
Notifications
You must be signed in to change notification settings - Fork 125
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
Various CI fixes #2074
base: main
Are you sure you want to change the base?
Various CI fixes #2074
Conversation
d4ced12
to
d997aaf
Compare
Gemfile
Outdated
@@ -35,7 +35,7 @@ group :development, :test do | |||
gem "sprockets" | |||
gem "state_machines" | |||
gem "activerecord-typedstore" | |||
gem "sqlite3", "~>1.4" | |||
gem "sqlite3" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm hoping the issue in #1875 (which I don't know) is resolved by now. We can't constraint it due to https://github.com/rails/rails/blob/main/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb#L14
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @andyw8
@@ -1287,6 +1287,7 @@ module Foo; end | |||
class Foo::Engine < ::Rails::Engine | |||
class << self | |||
def __callbacks; end | |||
def __callbacks=(new_value); end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't find where this is defined in Rails but I think it's a safe change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This definition is flaky. It looks like a normal class_attribute
so it should always be there 🤷 . There's a recent change to it in rails/rails@1962e8d but that doesn't explain my change which reduced the number of CI failures.
f3b6a2e
to
cb9dc71
Compare
3ae2d03
to
a456ca1
Compare
Development version of sqlite3 used by tapioca seems to leak into the tests. I don't fully understand it however, one solution is sticking to the coupling between ActiveRecord and sqlite3 versions https://github.com/rails/rails/blob/dd8f7185faeca6ee968a6e9367f6d8601a83b8db/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb#L14
Not needed after the previous commit
Not sure why yet 😬
cd40979
to
1ecf288
Compare
Motivation
Fix CI
Implementation
Tests