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.

Sign up for GitHub

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

Jump to bottom

Fix have_enqueued_mail for Rails 7 and argument matching #2546

Merged
merged 10 commits into from Jan 10, 2022
Merged

Fix have_enqueued_mail for Rails 7 and argument matching #2546

merged 10 commits into from Jan 10, 2022

Conversation

pirj
Copy link
Member

@pirj pirj commented Dec 20, 2021 •
edited

This is a combo of #2516 and #2537, supposed to fix #2351 and #2531 with a slight fix for argument matching and a small fix for our system spec spec on Rails 7.

@pirj pirj self-assigned this Dec 20, 2021
@pirj pirj added the rails7 label Dec 20, 2021
@pirj pirj requested a review from JonRowe December 20, 2021 23:08
@pirj pirj mentioned this pull request Dec 20, 2021
Closed
@pirj pirj marked this pull request as ready for review December 20, 2021 23:17
@pirj
Copy link
Member Author

pirj commented Dec 21, 2021

@JonRowe Rails 7 specs are green. This is a good first step to support it.

@AlfonsoUceda
Copy link

Thanks @pirj for your work.

Is there an ETA for this PR to be merged and a release version?

@JonRowe
Copy link
Member

JonRowe commented Dec 23, 2021

Is there an ETA for this PR to be merged and a release version?

RSpec is a community driven project (e.g. volunteers in there free time only) and doesn't commit to ETAs

@skelz0r skelz0r mentioned this pull request Dec 29, 2021
Merged
@pirj pirj mentioned this pull request Dec 31, 2021
Closed
@pirj pirj mentioned this pull request Jan 9, 2022
Closed
@pirj pirj changed the title Fix have_enqueued_mail for Rails 7 and argument matching for older Fix have_enqueued_mail for Rails 7 and argument matching Jan 10, 2022
@pirj pirj mentioned this pull request Jan 10, 2022
Closed
fabn and others added 10 commits January 10, 2022 22:53
@fabn @pirj
Change HaveEnqueuedEmail to work as usual in Rails 6
50e2c72
@fabn @pirj
Add missing relish documentation for parameter matching
7adcef2
@fabn @pirj
Ensure ActionMailer::MailDeliveryJob is defined
1092437
@fabn @pirj
Silence rubocop for HaveEnqueuedMail class length
54d9692
@fabn @pirj
Split all syntax in different examples
84ac4f8
@fabn @pirj
Clarify examples with better params usage
08d92ce
@mhenrixon @pirj
Add compatibility for have_enqueued_mail (rails 7)
ed71aed 
1. ActionMailer::DeliveryJob does not exist anymore
2. ActionMailer::Parameterized::DeliveryJob

According to my research they have both been superseeded by ActionMailer::MailDeliveryJob

Closes #2531
@pirj
Fix unified mailer args
1dbacd7
@pirj
Fix system spec spec on Rails 7
a9cbac6 
ActionPack is checking `instance_created?` now, and it was `false`, so
`take_screenshot` was not called.

```
def take_failed_screenshot
  take_screenshot if failed? && supports_screenshot? && Capybara::Session.instance_created?
end
```

rails/rails@1dfcffb
@pirj
Add Changelog entry
dd48c6b
@pirj pirj force-pushed the combo-have_enqueued_mail-fixes branch from 9628d30 to dd48c6b Compare January 10, 2022 19:53
@pirj
Copy link
Member Author

pirj commented Jan 10, 2022

I intend to merge this on green. In any case, RSpec is unusable with Rails 7 without this.
Checked using my current project (Rails 6.0, previously rspec-rails 4.0.2) - everything seems to be intact.

I'll keep the branch not removed not to break it for those who gem 'rspec-rails', github: 'rspec/rspec-rails', branch: 'combo-have_enqueued_mail-fixes'.

@pirj pirj merged commit 3e7d8d0 into main Jan 10, 2022
This was referenced Jan 10, 2022
Closed
Closed
@dwightwatson
Copy link

Thank you! Will there be a tagged release to go with this or should we just use main in the meantime?

@pirj
Copy link
Member Author

pirj commented Jan 10, 2022

We're working on an undetermined number of failures now. The release will come when our test suite will pass with Rails 7. Using main is the only safe option now.

@JonRowe
Copy link
Member

JonRowe commented Jan 11, 2022 •
edited

I was holding off avoiding merging these @pirj, but sorry I didn't communicate that with you, because it makes it easier to do our release steps.

  • Merge any outstanding 6.x patches we can, release 5.0.3/
  • Release 5.1.0 with the few tweaks for rails 6.1
  • Release 6.0.0 supporting Rails 7.

This is because major version changes are required for new rails versions for us under our release policy, but this means a bunch of branch wrangling.

@JonRowe JonRowe deleted the combo-have_enqueued_mail-fixes branch January 11, 2022 10:04
@pirj
Copy link
Member Author

pirj commented Jan 11, 2022

I was suspecting this. My apologies for rushing.

Can I help with the release somehow, @JonRowe ?

@pirj pirj mentioned this pull request Jan 12, 2022
Merged
@mitchellhenke mitchellhenke mentioned this pull request Jan 13, 2022
Merged
@JonRowe
Copy link
Member

JonRowe commented Jan 18, 2022

See #2560 @pirj

@petergoldstein petergoldstein mentioned this pull request Jan 19, 2022
Closed
@pirj pirj restored the combo-have_enqueued_mail-fixes branch January 19, 2022 09:17
@pirj pirj mentioned this pull request Jan 19, 2022
Closed
21 tasks
@pirj
Copy link
Member Author

pirj commented Jan 19, 2022

If you happen to track main in your Gemfile and the support for Rails 7 breaks while we're working on properly releasing a version of rspec-rails for it in #2560, please use combo-have_enqueued_mail-fixes branch in your Gemfile instead:

gem 'rspec-rails`, github: 'rspec/rspec-rails', branch: 'combo-have_enqueued_mail-fixes'

@Matt-Yorkley
Copy link

When attempting to use the combo-have_enqueued_mail-fixes branch, I'm getting:

Bundler could not find compatible versions for gem "rspec-core":
  In Gemfile:
    rspec-rails was resolved to 5.1.0.pre, which depends on
      rspec-core (= 3.11.0.pre)

Could not find gem 'rspec-core (= 3.11.0.pre)', which is required by gem 'rspec-rails', in any of the sources.

@rwojnarowski
Copy link

This temp patch fixes issue, im on rspec-rails (5.0.2) and works great:
#2531 (comment)

@pirj
Copy link
Member Author

pirj commented Jan 20, 2022

You also have to specify the rest of RSpec gems to be fetched from GitHub like here https://github.com/rspec/rspec-rails/blob/main/Gemfile-rspec-dependencies#L7

@pirj pirj deleted the combo-have_enqueued_mail-fixes branch January 20, 2022 08:27
@pirj pirj restored the combo-have_enqueued_mail-fixes branch January 20, 2022 08:28
@pirj pirj added this to the 6.0 milestone Jan 20, 2022
@pirj pirj mentioned this pull request Jan 29, 2022
Merged
@zspencer zspencer mentioned this pull request Mar 1, 2022
Closed
waiting-for-dev added a commit to solidusio-contrib/solidus_tracking that referenced this pull request May 4, 2022
@waiting-for-dev
Fix rspec-rails incompatbility with Rails 7
d1b25c2 
rspec-rails v6.0.0.rc1 needs to be used to support `have_enqueued_email`
in Rails 7 (see rspec/rspec-rails#2546).
However, that would break tests for previous versions. On top of that,
`rspec-rails` dependency is transitive through `solidus_dev_support`,
which is not compatible with rsec-rails v6.0.0.rc1.
waiting-for-dev added a commit to solidusio-contrib/solidus_tracking that referenced this pull request May 4, 2022
@waiting-for-dev
Fix rspec-rails incompatbility with Rails 7
3538a10 
rspec-rails v6.0.0.rc1 needs to be used to support `have_enqueued_email`
in Rails 7 (see rspec/rspec-rails#2546).
However, that would break tests for previous versions. On top of that,
`rspec-rails` dependency is transitive through `solidus_dev_support`,
which is not compatible with rsec-rails v6.0.0.rc1.
waiting-for-dev added a commit to solidusio-contrib/solidus_tracking that referenced this pull request May 4, 2022
@waiting-for-dev
Fix rspec-rails incompatbility with Rails 7
f631933 
rspec-rails v6.0.0.rc1 needs to be used to support `have_enqueued_email`
in Rails 7 (see rspec/rspec-rails#2546).
However, that would break tests for previous versions. On top of that,
`rspec-rails` dependency is transitive through `solidus_dev_support`,
which is not compatible with rsec-rails v6.0.0.rc1.
@pirj pirj deleted the combo-have_enqueued_mail-fixes branch July 23, 2022 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JonRowe JonRowe Awaiting requested review from JonRowe

Assignees

@pirj pirj

Labels
rails7
Projects
None yet
Milestone
6.0
Development

Successfully merging this pull request may close these issues.

have_enqueued_mail with options doesn't match
8 participants
@pirj @AlfonsoUceda @JonRowe @dwightwatson @Matt-Yorkley @rwojnarowski @fabn @mhenrixon