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

Add Rails 8 compatibility #590

Merged
merged 1 commit into from
Nov 21, 2024
Merged

Conversation

tagliala
Copy link
Contributor

@tagliala tagliala commented Nov 10, 2024

In specific:

  • Do not test Rails 7.0 against 3.3
  • Allow 8.0 in gemspec, by relaxing the upper constraint to < 8.1.
    Rails uses shifted SemVer, so 8.1 is considered a Major.
  • Conditionally load the sqlite3 gem based on the Rails version.
  • Conditionally load selenium webdriver gem based on the Rails version.
  • Simplify error page assertions to account for variations in
    apostrophes, as "We're sorry" changed to "We`re sorry" in Rails 8.

References:

Close #589


CI: https://github.com/tagliala/cucumber-rails/actions/runs/11764599692/job/32769937431

ActiveAdmin CI against the fork (7.0, 7.1, 7.2, 8.0): https://github.com/activeadmin/activeadmin/actions/runs/11764550754/job/32769831332


🤔 What's changed?

Rails 8.0 compatibility

⚡️ What's your motivation?

Unexpected downgrade to 1.4 after upgrading to Rails 8.x

🏷️ What kind of change is this?

  • ⚡ New feature (non-breaking change which adds new behaviour)

♻️ Anything particular you want feedback on?

n/a

📋 Checklist:

  • I agree to respect and uphold the Cucumber Community Code of Conduct
  • I've changed the behaviour of the code
    • I have added/updated tests to cover my changes.
  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.
  • Users should know about my change
    • I have added an entry to the "Unreleased" section of the CHANGELOG, linking to this pull request.

This text was originally generated from a template, then edited by hand. You can modify the template here.

README.md Show resolved Hide resolved
@luke-hill luke-hill self-requested a review November 11, 2024 08:42
@luke-hill luke-hill self-assigned this Nov 11, 2024
skelz0r added a commit to etalab/data_pass that referenced this pull request Nov 12, 2024
* Remove obosolete dependency from 12/2023 which was a proof of concept at
  the time.
* Fix deprecated
  ```
  DEPRECATION WARNING: `to_time` will always preserve the full timezone
  rather than offset of the receiver in Rails 8.1. To opt in to the new
  behavior, set `config.active_support.to_time_preserves_timezone =
  :zone`.
  ```
* `cucumber-rails` was not ready for rails 8, point to branch with some
  config change (no feature changed)

  cucumber/cucumber-rails#590
skelz0r added a commit to etalab/data_pass that referenced this pull request Nov 12, 2024
* Remove obosolete dependency from 12/2023 which was a proof of concept at
  the time.
* Fix deprecated
  ```
  DEPRECATION WARNING: `to_time` will always preserve the full timezone
  rather than offset of the receiver in Rails 8.1. To opt in to the new
  behavior, set `config.active_support.to_time_preserves_timezone =
  :zone`.
  ```
* `cucumber-rails` was not ready for rails 8, point to branch with some
  config change (no feature changed)

  cucumber/cucumber-rails#590
* `MalwareScan#analyzed_at` is a datetime, not an integer, which raises
  an error now in Rails 8
skelz0r added a commit to etalab/data_pass that referenced this pull request Nov 12, 2024
* Remove obosolete dependency from 12/2023 which was a proof of concept at
  the time.
* Fix deprecated
  ```
  DEPRECATION WARNING: `to_time` will always preserve the full timezone
  rather than offset of the receiver in Rails 8.1. To opt in to the new
  behavior, set `config.active_support.to_time_preserves_timezone =
  :zone`.
  ```
* `cucumber-rails` was not ready for rails 8, point to branch with some
  config change (no feature changed)

  cucumber/cucumber-rails#590
* `MalwareScan#analyzed_at` is a datetime, not an integer, which raises
  an error now in Rails 8
Copy link
Contributor

@luke-hill luke-hill 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 this change. Few minor tweaks and then we can get this merged in

cucumber-rails.gemspec Show resolved Hide resolved
features/support/cucumber_rails_gem_helper.rb Show resolved Hide resolved
.github/workflows/test.yml Outdated Show resolved Hide resolved
features/allow_rescue.feature Show resolved Hide resolved
Copy link
Contributor

@luke-hill luke-hill left a comment

Choose a reason for hiding this comment

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

Hi there. Can we make these 2 minor tweaks then I'll get it merged in.

.github/workflows/test.yml Outdated Show resolved Hide resolved
cucumber-rails.gemspec Show resolved Hide resolved
In specific:
- Do not test Rails 7.0 and 7.1 against 3.3
- Allow 8.0 in gemspec, by relaxing the upper constraint to `< 9`.
  Rails uses shifted SemVer, so `8.1` is considered a Major, but it
  has been decided to use `< 9` anyway
- Conditionally load the sqlite3 gem based on the Rails version.
- Conditionally load selenium webdriver gem based on the Rails version.
- Simplify error page assertions to account for variations in
  apostrophes, as "We're sorry" changed to "We`re sorry" in Rails 8.

References:
- https://guides.rubyonrails.org/maintenance_policy.html#versioning
- rails/rails#53045

Close cucumber#589
@@ -5,7 +5,7 @@
[![Code Climate](https://codeclimate.com/github/cucumber/cucumber-rails.svg)](https://codeclimate.com/github/cucumber/cucumber-rails)
[![Open Source Helpers](https://www.codetriage.com/cucumber/cucumber-rails/badges/users.svg)](https://www.codetriage.com/cucumber/cucumber-rails)

Cucumber-Rails brings Cucumber to Rails 5.2, 6.x and 7.x.
Cucumber-Rails brings Cucumber to Rails 5.2, 6.x, 7.x, and 8.0.
Copy link
Contributor Author

@tagliala tagliala Nov 21, 2024

Choose a reason for hiding this comment

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

Since the constraint is < 9, 8.0 may not play well with the gemspec but reflects actual compatibility

8.0 should be changed to 8.x after it is certain that cucumber-rails will also support 8.1

@luke-hill luke-hill merged commit ca0ecab into cucumber:main Nov 21, 2024
13 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.

Rails 8 Compatibility (PR attached)
3 participants