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

Suppress output from mail parser warnings #47484

Merged
merged 2 commits into from Feb 24, 2023

Conversation

skipkayhil
Copy link
Member

Motivation / Background

Previously, requiring any of the ragel generated parsers in mail would output tons of warnings in tests, making output much harder to read (especially in Railties).

Detail

This commit extends the RaiseWarnings patch to suppress output from these mail parsers.

The suppression can be removed once mikel/mail#1557 or mikel/mail#1551 or any other PR fixes the issue

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • [ ] Tests are added or updated if you fix a bug or add a feature.
  • [ ] CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

Previously, requiring any of the ragel generated parsers in mail would
output tons of warnings in tests, making output much harder to read
(especially in Railties).

This commit extends the RaiseWarnings patch to suppress output from
these mail parsers.

The suppression can be removed once mikel/mail#1557 or mikel/mail#1551
or any other PR fixes the issue
Copy link
Member

@byroot byroot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍


SUPPRESSED_WARNINGS = Regexp.union(
# TODO: remove if https://github.com/mikel/mail/pull/1557 or similar fix
%r{/lib/mail/parsers/.*statement not reached},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 I'll investigate... that's what I get for only running Action Mailer locally:
https://buildkite.com/rails/rails/builds/94131#01868059-d79a-43f7-9b4c-4e392240a572

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hum, right it seem to fix actionmailer, but not actionmailbox.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ba028a4 should fix it, I can squash if you think they should go together

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nah it's ok

These appear to have been missed when the change was made in
d917896
@skipkayhil
Copy link
Member Author

I missed at least one more:

https://buildkite.com/rails/rails/builds/94132#0186806f-8a23-4d07-ae8f-3b937f8517ae

I'll fix that and then verify all of railties is good

@byroot byroot merged commit a2b7d8b into rails:main Feb 24, 2023
@skipkayhil skipkayhil deleted the suppress-mail-warnings-in-ci branch February 28, 2023 23:15
zzak added a commit to zzak/rails that referenced this pull request Oct 15, 2023
e.g.:

```
zzak@mbp16 railties % bin/test test/application/assets_test.rb
Run options: --seed 32028

.............../Users/zzak/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/mail-2.8.1/lib/mail/parsers/content_location_parser.rb:592: warning: assigned but unused variable - disp_type_s
I, [2023-10-15T10:38:33.005925 #97662]  INFO -- : [0ca46786-853f-4740-9e71-f644c4893502] Started GET "/assets/demo.js" for 127.0.0.1 at 2023-10-15 10:38:33 +0900
E, [2023-10-15T10:38:33.006641 #97662] ERROR -- : [0ca46786-853f-4740-9e71-f644c4893502]
[0ca46786-853f-4740-9e71-f644c4893502] ActionController::RoutingError (No route matches [GET] "/assets/demo.js"):
[0ca46786-853f-4740-9e71-f644c4893502]
../Users/zzak/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/mail-2.8.1/lib/mail/parsers/content_location_parser.rb:592: warning: assigned but unused variable - disp_type_s
..../Users/zzak/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/mail-2.8.1/lib/mail/parsers/content_location_parser.rb:592: warning: assigned but unused variable - disp_type_s
I, [2023-10-15T10:38:33.973335 #97660]  INFO -- : [fa848d82-0d80-4b23-925b-73fa5d0f47d4] Started GET "/posts?debug_assets=true" for 127.0.0.1 at 2023-10-15 10:38:33 +0900
I, [2023-10-15T10:38:33.975021 #97660]  INFO -- : [fa848d82-0d80-4b23-925b-73fa5d0f47d4] Processing by PostsController#index as HTML
I, [2023-10-15T10:38:33.975209 #97660]  INFO -- : [fa848d82-0d80-4b23-925b-73fa5d0f47d4]   Parameters: {"debug_assets"=>"true"}
I, [2023-10-15T10:38:33.977560 #97660]  INFO -- : [fa848d82-0d80-4b23-925b-73fa5d0f47d4] Completed 200 OK in 2ms (Views: 1.9ms | ActiveRecord: 0.0ms | Allocations: 1266)
......./Users/zzak/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/mail-2.8.1/lib/mail/parsers/content_location_parser.rb:592: warning: assigned but unused variable - disp_type_s
.

Finished in 4.842481s, 5.9887 runs/s, 23.3351 assertions/s.
29 runs, 113 assertions, 0 failures, 0 errors, 0 skips
```

For example:
https://buildkite.com/rails/rails/builds/94132#0186806f-8a23-4d07-ae8f-3b937f8517ae/1162-1171

See also rails#47484
hachi8833 pushed a commit to hachi8833/rails that referenced this pull request Oct 17, 2023
e.g.:

```
zzak@mbp16 railties % bin/test test/application/assets_test.rb
Run options: --seed 32028

.............../Users/zzak/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/mail-2.8.1/lib/mail/parsers/content_location_parser.rb:592: warning: assigned but unused variable - disp_type_s
I, [2023-10-15T10:38:33.005925 #97662]  INFO -- : [0ca46786-853f-4740-9e71-f644c4893502] Started GET "/assets/demo.js" for 127.0.0.1 at 2023-10-15 10:38:33 +0900
E, [2023-10-15T10:38:33.006641 #97662] ERROR -- : [0ca46786-853f-4740-9e71-f644c4893502]
[0ca46786-853f-4740-9e71-f644c4893502] ActionController::RoutingError (No route matches [GET] "/assets/demo.js"):
[0ca46786-853f-4740-9e71-f644c4893502]
../Users/zzak/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/mail-2.8.1/lib/mail/parsers/content_location_parser.rb:592: warning: assigned but unused variable - disp_type_s
..../Users/zzak/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/mail-2.8.1/lib/mail/parsers/content_location_parser.rb:592: warning: assigned but unused variable - disp_type_s
I, [2023-10-15T10:38:33.973335 #97660]  INFO -- : [fa848d82-0d80-4b23-925b-73fa5d0f47d4] Started GET "/posts?debug_assets=true" for 127.0.0.1 at 2023-10-15 10:38:33 +0900
I, [2023-10-15T10:38:33.975021 #97660]  INFO -- : [fa848d82-0d80-4b23-925b-73fa5d0f47d4] Processing by PostsController#index as HTML
I, [2023-10-15T10:38:33.975209 #97660]  INFO -- : [fa848d82-0d80-4b23-925b-73fa5d0f47d4]   Parameters: {"debug_assets"=>"true"}
I, [2023-10-15T10:38:33.977560 #97660]  INFO -- : [fa848d82-0d80-4b23-925b-73fa5d0f47d4] Completed 200 OK in 2ms (Views: 1.9ms | ActiveRecord: 0.0ms | Allocations: 1266)
......./Users/zzak/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/mail-2.8.1/lib/mail/parsers/content_location_parser.rb:592: warning: assigned but unused variable - disp_type_s
.

Finished in 4.842481s, 5.9887 runs/s, 23.3351 assertions/s.
29 runs, 113 assertions, 0 failures, 0 errors, 0 skips
```

For example:
https://buildkite.com/rails/rails/builds/94132#0186806f-8a23-4d07-ae8f-3b937f8517ae/1162-1171

See also rails#47484
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants