[wip] Changes to support Rails 6.1 #2400
Conversation
fix pr according to @pirj remarks
Improve controller template when no action
|
|
||
| copy_file 'spec/support/default_preview_path' | ||
| chmod 'spec/support/default_preview_path', 0755 | ||
| gsub_file 'spec/support/default_preview_path', | ||
| /ExampleApp/, | ||
| Rails.application.class.parent.to_s |
There was a problem hiding this comment.
Why are we doing this? It should be replaced not removed no?
There was a problem hiding this comment.
We had errors like undefined method 'parent' for ExampleApp::Application:Class . I noticed that this gsub is not needed anymore.
| @@ -24,7 +24,11 @@ def initialize(message) | |||
|
|
|||
| def matches?(mailbox) | |||
| @mailbox = mailbox | |||
| @receiver = ApplicationMailbox.router.send(:match_to_mailbox, inbound_email) | |||
| if ApplicationMailbox.router.respond_to?(:mailbox_for) | |||
There was a problem hiding this comment.
We should be conditionally defining the method based on this.
There was a problem hiding this comment.
Good idea, I will make the change
There was a problem hiding this comment.
I switched back because I add and error on ActionMailbox and I find it more complicated to duplicated all the block. Maybe you prefer in a method? No problem if you want to push another solution on this branch. :)
On script/run_build
An error occurred while loading spec_helper.
Failure/Error: if ApplicationMailbox.router.respond_to?(:mailbox_for)
NameError:
uninitialized constant RSpec::Rails::Matchers::ActionMailbox::ReceiveInboundEmail::ApplicationMailbox
# ./spec/spec_helper.rb:27:in `<top (required)>'
No examples found.
There was a problem hiding this comment.
But we do not want to evaluate every-time ApplicationMailbox.router.respond_to?(:mailbox_for) when method is called. I will switch back and fix the error.
|
Sorry @JonRowe I should have open a PR instead of pushing to your branch. |
benoittgt
force-pushed
the
wip-rails-6-1-dev
branch
3 times, most recently
from
515ec09 to
131390e
Compare
|
CI is nearly full green. Our jruby errors may be linked to mikel/mail#912 |
benoittgt
force-pushed
the
wip-rails-6-1-dev
branch
3 times, most recently
from
e81da14 to
bcdba00
Compare
|
ATM I am not able to reproduce failing build on jruby localy. I switched to last jruby release instead of head. |
jruby not moved because there is too many issues with it at the moment. Next step fix : - fix ruby 3 - fix flacky test with "mountable_engine?" error - add JRuby
Error was + 1) InboxMailbox route email to properly mailbox + Failure/Error: + expect(InboxMailbox) + .to receive_inbound_email(to: "replies@example.com") + + ArgumentError: + wrong number of arguments (given 1, expected 0) + # /home/runner/work/rspec-rails/bundle/ruby/3.0.0/gems/actionmailbox-6.0.3.4/lib/action_mailbox/test_helper.rb:16:in `create_inbound_email_from_mail' + # ./spec/mailboxes/inbox_mailbox_spec.rb:6:in `block (2 levels) in <top (required)>' It should be fixed with Rails 6.1 rails/rails#35904
Co-authored-by: Jon Rowe <hello@jonrowe.co.uk>
Error was happening on Ruby3.0.0-preview1
Move MRI build to Github Actions
The #config option is removed from activerecord/lib/active_record/test_fixtures.rb
The parent class is already ExampleApp
benoittgt
force-pushed
the
wip-rails-6-1-dev
branch
from
bcdba00 to
2163b2e
Compare
|
@JonRowe what do you think about merging it to rails-6-1-dev? I will reopen a PR with Rails master branch to see if we have something that could break rspec-rails. |
|
Er this needs picking apart and cleaning up, its way to confused right now. |
|
Ok sorry. The thing is rails-6-1-dev is getting too much behind main? Should we rebase main into rails-6-1-dev first? It will remove lot's of commits from here. Something like: https://github.com/rspec/rspec-rails/compare/rails-6-1-dev...rails-6-1-dev-to-github-actions?expand=1 |
|
Ok. ATM I see no need to keep this PR open. Contributions to |
@benoittgt these are your changes, feel free to edit this PRs title / description.