Add compatibility with Rails 6.1

[sedubois]

Fixes #1272

---

• Wrote good commit messages.
• Feature branch is up-to-date with master (if not - rebase it).
• Squashed related commits together.
• Added tests.
• Added an entry to the Changelog if the new
  code introduces user-observable changes.
• The PR relates to only one subject with a clear title
  and description in grammatically correct, complete sentences.

[aried3r]

I'll have a look a this, thanks!

[sedubois]

Getting error:

An error occurred while loading spec_helper.
Failure/Error:
  ::ActiveRecord::MigrationContext.new(
    @migrations_path,
    ::ActiveRecord::Base.connection.schema_migration
  ).migrate
NameError:
  uninitialized constant ActiveRecord::MigrationContext
# ./spec/support/paper_trail_spec_migrator.rb:16:in `migrate'
# ./spec/spec_helper.rb:38:in `<top (required)>'

[sedubois]

@aried3r any idea what might cause this error?

[aried3r]

I believe MigrationContext is private API, as per rails/rails#36439. devise and derailed_benchmarks seem to use some helpers for this, but neither have been updated for Rails 6.1 yet, so I'm not sure if that would work, since the constant cannot be found.

See also rails/rails#36544.

I currently don't have time to look into this further myself.

[aried3r]

I should have checked better, paper_trail has a helper of that kind itself already.

paper_trail/spec/support/paper_trail_spec_migrator.rb

Lines 13 to 25 in f66c702

	def migrate
	v = ::ActiveRecord.gem_version
	if v >= ::Gem::Version.new("6.0.0.rc2")
	::ActiveRecord::MigrationContext.new(
	@migrations_path,
	::ActiveRecord::Base.connection.schema_migration
	).migrate
	elsif ::Gem::Requirement.new([">= 5.2.0.rc1", "< 6.0.0.rc2"]).satisfied_by?(v)
	::ActiveRecord::MigrationContext.new(@migrations_path).migrate
	else
	::ActiveRecord::Migrator.migrate(@migrations_path)
	end
	end

From what I see it is still there https://github.com/rails/rails/blob/ddec53519e2c61665761e26d8c91a18724570b96/activerecord/lib/active_record/migration.rb#L1050.

This is just a wild guess, but maybe it has to do with zeitwerk and the dummy app in the specs not yet compatible with it But again, just a guess.

[sedubois]

maybe it has to do with zeitwerk

AFAIK zeitwerk is used since Rails 6. Did Zeitwerk change in 6.1?

[aried3r]

Good news! rails/rails#40806

I don't think this directly fixes our problem here, but at least this method will be officially supported, should it be merged and released.

AFAIK zeitwerk is used since Rails 6. Did Zeitwerk change in 6.1?

Again, this is just a wild guess and maybe not worth going on a wild goose chase. But I tried to read up on it a bit more, it says zeitwerk is the default for Rails 6 applications. Our dummy_app here uses Rails 6[.1] in the spec, but I'm not sure if it's a fully upgraded Rails 6 application. In the sense, rails app:update wasn't called, we're not loading any framework defaults (config.load_defaults) etc, so I'm not sure in which state this dummy app is.

From the docs:

# config/application.rb

config.load_defaults 6.0

enables zeitwerk autoloading mode on CRuby. In that mode, autoloading, reloading, and eager loading are managed by Zeitwerk.

https://guides.rubyonrails.org/upgrading_ruby_on_rails.html#autoloading

I don't see what happens if you omit this line. My assumption was, that with 6.1 the new defaults of 6.0 become the default, enabling zeitwerk. These are the settings for each version: https://guides.rubyonrails.org/configuring.html#results-of-config-load-defaults.

But, again, I'm not even sure it's that. Maybe shouldn't have mentioned it.

Does anyone here maybe have time to look into this locally some more? I'm currently busy at work with the last tickets before the winter holidays, so I cannot promise looking into it. But happy to review and discuss potential solutions.

[aried3r]

Ok, I was curious and tried it locally. Adding require "active_record/migration" on top of the paper_trail_spec_migrator.rb solves this issue (although I cannot tell you exactly why and when this changed).

However, then other tests start to fail, some of them being because we use Arel, which I believe is still private API. One example is:

  1) PaperTrail::Serializers::YAML.where_object constructs the correct WHERE query
     Failure/Error: expect(matches.right.val).to eq("%\narg1: Val 1\n%")

     NoMethodError:
       undefined method `val' for #<Arel::Nodes::Casted:0x00007fb99a4298d8>
       Did you mean?  value
     # ./spec/paper_trail/serializers/yaml_spec.rb:42:in `block (3 levels) in <module:Serializers>'

Indeed, val has been renamed to value: rails/rails@280d6eb#diff-1c9b3a4b77db2a841dbe5b36ef9d63f7f9c646896944efdfd26c388396203c94

If you'd like to continue, these would need to be fixed for us to release a version compatible with Rails 6.1.

[jaredbeck]

@aried3r Good analysis. I've addressed many of the issues you found, in #1274

@sedubois Please rebase this once #1274 is merged.

[jaredbeck]

@sedubois #1274 is merged, please rebase.

[pedrofurtado]

oh yeah 🏆

[jaredbeck]

@sedubois #1274 is merged, please rebase.

Nevermind, I couldn't wait 😁 ; Closing via #1275, perserving @sedubois authorship. Thanks Sébastien for getting the ball rolling.