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

chore: Fix solid queue tests #222

Merged
merged 8 commits into from
Oct 15, 2024
Merged

chore: Fix solid queue tests #222

merged 8 commits into from
Oct 15, 2024

Conversation

adamlogic
Copy link
Collaborator

Not sure yet if it's just a test issue with recent Solid Queue versions, or if it's an actual compatibility issue.

New error in Solid Queue 0.8.1 seems related to how we setup our test DB.
@adamlogic
Copy link
Collaborator Author

adamlogic commented Oct 12, 2024

@carlosantoniodasilva I'm incrementally moving through Solid Queue versions to see where things are failing. Our Solid Queue tests started failing in v0.7.1 with uninitialized constant SolidQueue::Processes::Callbacks::ActiveModel. I've fixed that by requiring it explicitly, although I'm unclear why we need to do that. 🤷‍♂️

Now we're encountering DB issues in v0.8.1 where there were substantial changes to how Solid Queue prepares the schema. I'm a bit fuzzy on what's happening in our test_helper to bootstrap the Solid Queue schema, so I'm hoping you could jump in and take a look.

Copy link
Member

@carlosantoniodasilva carlosantoniodasilva left a comment

Choose a reason for hiding this comment

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

Thanks for getting it started, I'll take a look at those schema changes.

Do we want to support multiple versions, or maybe we should lock on v1+ once we get tests running there?

@@ -1,5 +1,8 @@
# frozen_string_literal: true

# Tests are failing with solid_queue 0.7.1+ unless we require active_model.
# Not sure why since solid_queue depends on activerecord which depends on activemodel.
require "active_model"

Choose a reason for hiding this comment

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

They're not explicitly requiring active_model (and/or the specific callbacks module that they rely on), so I'm guessing by the time that solid_queue code gets loaded, it hasn't been required by AR yet.

I was hoping to fix it there, but if we want to support prior versions we might have to keep a require on our side for the time being as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah, got it. 👍

@adamlogic
Copy link
Collaborator Author

Do we want to support multiple versions, or maybe we should lock on v1+ once we get tests running there?

Replying in Linear.

SolidQueue v0.8+ merged all the migrations into a single schema file
that is automatically loaded onto the DB on a Rails app. They suggest a
separate DB, which AR will handle automatically by copying over this
single file schema to the app, and to use on the same DB one would have
to manually move the schema to a migration and execute it.

For our case, all we care about is to have SolidQueue schema / tables
loaded onto our test DB. We do that by manually loading the lib's schema
file via Active Record. We still execute migrations afterwards, even
though it's a no-op for current SolidQueue, if they need to make new
schema changes in the future, they'll come in the form of migrations.
This way we can test both minimum and whatever "current/latest" is.
(currently v1.0)
Update GH matrix to skip unsupported Ruby versions on Rails 7.2 as well,
(main Gemfile for now) since it requires Ruby 3.1+.
We already have a few other requires that were needed there to get tests
running, we can keep this new one there too, as it should not be a
problem in runtime within a normal Rails environment. (and if it happens
to be, we'll learn about it soon enough.)
Copy link
Member

@carlosantoniodasilva carlosantoniodasilva left a comment

Choose a reason for hiding this comment

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

@adamlogic this should be green now, let me know what you think.

.github/workflows/judoscale-rails-test.yml Show resolved Hide resolved
@@ -14,6 +14,7 @@ jobs:
matrix:
gemfile:
- Gemfile
- Gemfile-solid_queue-0-3

Choose a reason for hiding this comment

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

New Gemfile to test against solid_queue v0.3 so we get coverage on minimum version supported

judoscale-solid_queue/test/test_helper.rb Show resolved Hide resolved
judoscale-solid_queue/test/test_helper.rb Show resolved Hide resolved
judoscale-solid_queue/test/test_helper.rb Show resolved Hide resolved
@carlosantoniodasilva carlosantoniodasilva marked this pull request as ready for review October 15, 2024 17:04
@adamlogic
Copy link
Collaborator Author

@carlosantoniodasilva Awesome, thank you!

@adamlogic adamlogic merged commit 3072fa3 into main Oct 15, 2024
132 checks passed
@adamlogic adamlogic deleted the fix-solid-queue-tests branch October 15, 2024 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants