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

Update gush dependencies #107

Merged
merged 11 commits into from
Feb 29, 2024
Merged

Update gush dependencies #107

merged 11 commits into from
Feb 29, 2024

Conversation

krzyzak
Copy link
Collaborator

@krzyzak krzyzak commented Feb 28, 2024

This PR introduces following changes:

  • Drop support for EOL ruby versions: now at least ruby 3.0.0 is required
  • Drop support for EOL rails versions: at least rails 6.1.0 is required
  • small patch for integration tests

@natemontgomery
Copy link

natemontgomery commented Feb 28, 2024

I see that there is a failure in the actions running due to a NoMethodError on 'assert'. I think your adding of require 'active_support' seems logical but if that is not working another thought would be to add an include to the config in spec_helper:

require 'active_support'
require 'active_support/testing/assertions'
require 'active_support/testing/time_helpers'
...

RSpec.configure do |config|
  config.include ActiveSupport::Testing::Assertions
  config.include ActiveSupport::Testing::TimeHelpers
  ...

I haven't been able to reproduce this locally yet though so I am not 100% sure on this as a fix in the GitHub run.

Also, looks like the actions for testing are raising a deprecation about Node 16, I think this means a need for change of the actions/checkout@v2 line to something newer, https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#example-using-versioned-actions.

@natemontgomery
Copy link

The fact that I can't seem to easily reproduce the action test failure locally makes me think it could be something to do with the container environment in the action. Might be worth trying out adding a before hook to reset the test adapter, as I have seen some suggestions that container environments with multiple containers can have some edge with ActiveJob assertions (I have never seen it myself before though).

That would mean something like
before { ActiveJob::Base.queue_adapter = :test }

I'm going to keep plugging at trying to reproduce locally, but wanted to post some thoughts in case I can help. Thanks for opening this PR btw :)

@krzyzak
Copy link
Collaborator Author

krzyzak commented Feb 28, 2024

Hi @natemontgomery!

I was able to debug this locally by running RAILS_VERSION='7.1.2' bundle exec rspec. I found the root cause and now the CI is green. We'll release new version that modernises gush a bit very soon, stay tuned! ;)

@krzyzak krzyzak self-assigned this Feb 28, 2024
@natemontgomery
Copy link

Yea I was able to reproduce locally once I found the versions issues (and after running through the GitHub action locally). It seems like you have it well in hand, thanks again!

@krzyzak krzyzak marked this pull request as ready for review February 28, 2024 21:58
@pokonski
Copy link
Contributor

Looks good but I think we should make it a 3.0.0 version

@krzyzak krzyzak merged commit 00d0bc5 into master Feb 29, 2024
24 checks passed
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.

3 participants