Add Rails 7.0 to Appraisals

[technicalpickles]

To figure out what needs to change, we start by adding it to be tested. I fully expect this to explode fantastically at first.

cc https://github.com/thoughtbot/shoulda-matchers/issues/1469

[vsppedro]

Thank you for your contribution. If you like to see the errors, you will need to update the GitHub actions too.

https://github.com/thoughtbot/shoulda-matchers/blob/4eb5bb0b350bb75b3c298f8e4acd1fc362f889ab/.github/workflows/ci.yml#L33-L36

The problem that I'm facing is that the tests use a method for ActiveSupport that was removed - unloadable. Where do we use this method? You may ask:

https://github.com/thoughtbot/shoulda-matchers/blob/4eb5bb0b350bb75b3c298f8e4acd1fc362f889ab/spec/support/unit/helpers/class_builder.rb#L37

https://github.com/thoughtbot/shoulda-matchers/blob/4eb5bb0b350bb75b3c298f8e4acd1fc362f889ab/spec/support/unit/helpers/class_builder.rb#L58

The PR that removed this method: https://github.com/rails/rails/pull/43048

[aried3r]

The problem that I'm facing is that the tests use a method for ActiveSupport that was removed - unloadable.

https://github.com/github/graphql-client/pull/274 seems to fix this and their reasoning seems sound. Would this also apply to this gem, @VSPPedro?

[vsppedro]

The problem that I'm facing is that the tests use a method for ActiveSupport that was removed - unloadable.

github/graphql-client#274 seems to fix this and their reasoning seems sound. Would this also apply to this gem, @VSPPedro?

Thanks for this link, @aried3r! Unfortunately, I've already tried to do something similar, but it didn't work. Maybe there's something I'm missing.

@mcmire, I think you have more experience with this part of the project. Do you happen to have any idea what needs to be done? If so, I will gladly work on it.

[vsppedro]

Sorry, I forgot to tell you that Rails 7.0 requires Ruby 2.7+.

We need to update the ci in a way that prevents it from running with invalid ruby ​​versions. Example:

https://github.com/thoughtbot/shoulda-matchers/blob/0a2bb552792ae23284ed331de50bf5367d9c1135/.github/workflows/ci.yml#L40-L41

[technicalpickles]

I think this gets it to the point where we surface actual errors. So far, I see:

• most of spec/unit/shoulda/matchers/active_model/validate_inclusion_of_matcher_spec.rb fails
• ~spec/unit/shoulda/matchers/active_record/have_rich_text_matcher_spec.rb fails due to action_text_rich_texts table not existing~
• ~spec/unit/shoulda/matchers/active_model/validate_presence_of_matcher_spec.rb:876 expected to raise Shoulda::Matchers::ActiveModel::CouldNotSetPasswordError but didn't~

[technicalpickles]

most of spec/unit/shoulda/matchers/active_model/validate_inclusion_of_matcher_spec.rb fails

These end up passing for me locally 🤔 Does that happen for anyone else?

spec/unit/shoulda/matchers/active_model/validate_presence_of_matcher_spec.rb:876 expected to raise Shoulda::Matchers::ActiveModel::CouldNotSetPasswordError but didn't

It looks like there was one change around secure password specifically: https://github.com/rails/rails/pull/43378

[technicalpickles]

          After setting :attr to ‹Wed, 26 Jan 2022 14:33:37 +0000› -- which was
          read back as ‹Wed, 26 Jan 2022 14:33:37.999739000 UTC +00:00› -- the
          matcher expected the Example to be valid, but it was invalid instead,
          producing these validation errors:

          * attr: ["is not included in the list"]

I actually ran into something similar to this in migrating our app. The default precision on datetime is now 6, ie:

# before
    t.datetime "created_at"
    t.datetime "updated_at"
# after
    t.datetime "created_at", precision: 6
    t.datetime "updated_at", precision: 6

The differences is that you get decimals after the second when reading it back, even though it doesn't have them going in. This is testing for exact membership in the array (ie include?), so it fails because it isn't equal. If it was a range instead, it would use cover? instead according to the docs:

:in - An enumerable object of available items. This can be supplied as a proc, lambda or symbol which returns an enumerable. If the enumerable is a numerical, time or datetime range the test is performed with Range#cover?, otherwise with include?. When using a proc or lambda the instance under validation is passed as an argument.

This may be relevant too postgresql - Millisecond resolution of DateTime in Ruby - Stack Overflow

[technicalpickles]

Possibly related: DB schema dumper uncompatibility in Rails 7.0 · Issue #43909 · rails/rails

[technicalpickles]

I was able to reproduce the validate_inclusion_of failures after getting a VScode devcontainer running on linux (cc https://github.com/thoughtbot/shoulda-matchers/pull/1487).

I have spent quite a bit of time tracing through the code, and trying to reproduce the problem, but haven't been able to find a reason for why it would fail on Rails 7, but pass otherwise. The main observable difference is that inspecting a ActiveModel attribute reader seems to include precision and timezone compared with the value is set. It's also worth noting that we are setting the attribute to a DateTime, and the reader returns a ActiveSupport::TimeWithZone which are subtly difference. I've also looked over the diffs from Rails 6.1 to 7.0, and the only thing kind of in the area is deprecating to_s(format) for to_formatted_s(format) (changelog, but that doesn't quite explain it.

I returned to what the code is testing. A model is created with some possible values starting now, and iterating 5 times subtracting a day a piece, in an array. DateTime.now is capturing microseconds. If you think about it, I am having a hard time imagining when you'd use validates_inclusion_of with an array of DateTime (instead of a range which uses .cover? and does work), that would also have microsends.

If it is a pretty artificial test, then we can artificially set the time it's using to something we have better control off. That is how I got the tests working, and it passes the other Rails versions as well.

[technicalpickles]

Down to 1 failure for Postgres:

  1) Shoulda::Matchers::ActiveRecord::HaveRichTextMatcher when the model has a RichText association matches when the subject configures has_rich_text
     Failure/Error:
       @subject.send(rich_text_attribute).
         instance_of?(ActionText::RichText)

     ActiveRecord::StatementInvalid:
       PG::UndefinedTable: ERROR:  relation "action_text_rich_texts" does not exist
       LINE 9:  WHERE a.attrelid = '"action_text_rich_texts"'::regclass
                                   ^
     # ./lib/shoulda/matchers/active_record/have_rich_text_matcher.rb:73:in `has_expected_action_text?'
     # ./lib/shoulda/matchers/active_record/have_rich_text_matcher.rb:62:in `run_checks'
     # ./lib/shoulda/matchers/active_record/have_rich_text_matcher.rb:51:in `matches?'
     # ./spec/support/unit/matchers/match_against.rb:56:in `matches?'
     # ./spec/unit/shoulda/matchers/active_record/have_rich_text_matcher_spec.rb:23:in `block (3 levels) in <main>'
     # ------------------
     # --- Caused by: ---
     # PG::UndefinedTable:
     #   ERROR:  relation "action_text_rich_texts" does not exist
     #   LINE 9:  WHERE a.attrelid = '"action_text_rich_texts"'::regclass
     #                               ^
     #   ./lib/shoulda/matchers/active_record/have_rich_text_matcher.rb:73:in `has_expected_action_text?'

Finished in 12 minutes 4 seconds (files took 12.43 seconds to load)
2533 examples, 1 failure

Failed examples:

rspec ./spec/unit/shoulda/matchers/active_record/have_rich_text_matcher_spec.rb:18 # Shoulda::Matchers::ActiveRecord::HaveRichTextMatcher when the model has a RichText association matches when the subject configures has_rich_text

[technicalpickles]

I haven't been able to reproduce locally on macOS, or in a devcontainer.

Also I merged in main, and now I'm seeing a lot of failures for spec/unit/shoulda/matchers/active_record/define_enum_for_matcher_spec.rb but only on Ruby 3. I'm guessing it's related to https://github.com/thoughtbot/shoulda-matchers/pull/1453 ?

[vsppedro]

I'll try to reproduce locally and give you some information about the problem ASAP.

[vsppedro]

Hi @technicalpickles, sorry for taking so long to get back to you.

I finally finished the PR which adds support for ruby ​​3.1. I'll take another look at this PR ASAP.

[vsppedro]

I haven't been able to reproduce locally on macOS, or in a devcontainer.

Also I merged in main, and now I'm seeing a lot of failures for spec/unit/shoulda/matchers/active_record/define_enum_for_matcher_spec.rb but only on Ruby 3. I'm guessing it's related to #1453 ?

The bug with action_text_rich_texts only happens on postgres. We need to run the tests passing the DATABASE_ADAPTER, like this:

DATABASE_ADAPTER=postgresql bundle exec appraisal rails_7_0 rspec spec/unit/shoulda/matchers/active_record/have_rich_text_matcher_spec.rb:18

The problem was because the table was not being created, to fix that was necessary to make these changes: https://github.com/VSPPedro/shoulda-matchers/pull/5/commits/8e835bc67e1c71d7f0b1b639cc763bbea12f4369

There are two problems there. First, Psych 4.0 needs to receive aliases: true to work with aliases and for some reason, db:migrate was not working properly with Rails 7.0. After separating the commands all start working just fine. To be honest, I intend to take a better look at that to understand what is happening.

What did you think of the changes I suggested?

[technicalpickles]

I haven't gotten back to this, so happy to let anyone else make an attempt at it that wants to.