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

Fix CI - Update workflow, setup, specs, and more #998

Merged
merged 10 commits into from
Dec 12, 2023
Merged

Fix CI - Update workflow, setup, specs, and more #998

merged 10 commits into from
Dec 12, 2023

Conversation

sej3506
Copy link
Contributor

@sej3506 sej3506 commented Dec 8, 2023

Context:

In #995, a PR to add support for Rails 7.1, CI was failing.
The thing causing this first failure was issue number 1
below. Each additional fix here was yet another thing
CI failed on.

Things in this commit:

  1. Split dummy:db:reset to dummy:db:drop and dummy:set:up
  2. Replace outdated use of "MiniTest" with "Minitest"
  3. Update session_spec.rb .serialize_cookies to handle old and
    new versions of Rack's handling of headers
  4. Remove deprecated active record handling in application.rb
  5. Update request_with_remember_token.rb remember_token_cookies
    handling of headers.

  1. Setup & Github Workflow - Split dummy:db:reset to
    dummy:db:drop and dummy:set:up

This issue was discovered in #995.

I'm still looking into why exactly this is happening, but solution has
been consistent for me.

To reproduce the issue for yourself:
When CI runs for each version, it does so using
the specific Gemfile for the version.
Running
BUNDLE_GEMFILE=gemfiles/rails_7.1.gemfile RAILS_ENV=test bundle exec rake dummy:db:reset
and then
bundle exec appraisal rake
the Users table will not be found, because it is not created from this
command.

This issue only exists with the 7.1 gemfile that I've found.

Splitting the command dummy:db:reset to its component parts of dummy:db:drop and
dummy:db:setup on separate lines works - for all version. Using them on a
single line does not work, though.

  1. Replace outdated use of "MiniTest" with "Minitest"

Use of "MiniTest" (vs modern "Minitest") was removed in version 5.19.0.
CI runs using version 5.20 of Minitest and was failing.

https://my.diffend.io/gems/minitest/5.18.1/5.19.0

  1. Update session_spec.rb .serialize_cookies to handle old and
    new versions of Rack's handling of headers

The way Rack::Utils.set_cookie_header! works in rack 3.0.8
is different than previous versions.
Rack downcases keys at the class level now. This update supports both
versions of the method.

rack/rack@95d2f64

Also updating spec/support/cookies.rb and
spec/support/request_with_remember_token.rb for this.

  1. Remove deprecated active record handling in application.rb

Rails 6.0 no longer uses config.activerecord.sqlite3.represent_boolean_as_integer
as it is always true now.
rails/rails@f59b081

As well, config.active_record.legacy_connection_handling has been
deprecated since Rails 7.0 and throws errors if used.

rails/rails#45835

  1. Update request_with_remember_token.rb remember_token_cookies
    handling of headers.

The value might be a string or an array.

I'm still looking into why exactly this is happening, but solution has
been consistent for me.

To reproduce the issue for yourself:
When CI runs for each version, it does so using the specific Gemfile for
the version.
Running
`BUNDLE_GEMFILE=gemfiles/rails_7.1.gemfile RAILS_ENV=test
bundle exec rake dummy:db:reset`
and then
`bundle exec appraisal rake'
the Users table will not be found, because it is not created from this
command.

This issue only exists with the 7.1 gemfile that I've found.

Splitting the command db:reset to its component parts of db:drop and
db:setup on separate lines works - for all version. Using them on a
single line does not work, though.
The way env_for works in rack 3.0.8 is different than previous versions.
It downcases keys at the class level now. This update supports both
versions.

rack/rack@95d2f64
Use of "MiniTest" (vs modern Minitest) was removed in version 5.19.0. CI
runs using version 5.20 of Minitest and was failing.

This commit replaces the use of "MiniTest" with "Minitest".

https://my.diffend.io/gems/minitest/5.18.1/5.19.0
Rails 6.0 no longer uses `config.activerecord.sqlite3.represent_boolean_as_integer`
as it is always true now.
rails/rails@f59b081

As well, `config.active_record.legacy_connection_handling` has been
deprecated since Rails 7.0.

rails/rails#45835
How rack's set_cookie_header methods might return Set-Cookie or
set-cookie as a key depending on the version of rack
Apparently the value might be a string or an array depending on the
version of ActionDispatch.
@sej3506 sej3506 mentioned this pull request Dec 8, 2023
@sej3506 sej3506 changed the title Fix CI - Update workflow, setup, and spec Fix CI - Update workflow, setup, specs, and more Dec 12, 2023
@sej3506
Copy link
Contributor Author

sej3506 commented Dec 12, 2023

I'll be squashing and merging, using the PR description as the final commit message.

@sej3506 sej3506 merged commit ab34dae into main Dec 12, 2023
14 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