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

Various CI fixes #2074

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Various CI fixes #2074

wants to merge 3 commits into from

Conversation

KaanOzkan
Copy link
Contributor

@KaanOzkan KaanOzkan commented Nov 14, 2024

Motivation

Fix CI

Implementation

Tests

Gemfile Outdated
@@ -35,7 +35,7 @@ group :development, :test do
gem "sprockets"
gem "state_machines"
gem "activerecord-typedstore"
gem "sqlite3", "~>1.4"
gem "sqlite3"
Copy link
Contributor Author

@KaanOzkan KaanOzkan Nov 14, 2024

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @andyw8

@amomchilov amomchilov changed the title Remove sqlite3 constraint Remove sqlite3 gem constraint Nov 14, 2024
@@ -1287,6 +1287,7 @@ module Foo; end
class Foo::Engine < ::Rails::Engine
class << self
def __callbacks; end
def __callbacks=(new_value); end
Copy link
Contributor Author

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

Copy link
Contributor Author

@KaanOzkan KaanOzkan Nov 15, 2024

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.

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
@KaanOzkan KaanOzkan changed the title Remove sqlite3 gem constraint Various CI fixes Nov 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant