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

Fix RSpec #log_deprecation matcher error when #log_deprecation uses :key #3781

Merged
merged 1 commit into from
Jul 12, 2024

Conversation

delner
Copy link
Contributor

@delner delner commented Jul 11, 2024

What does this PR do?

#3675 added a limit feature to log_deprecation. However, leveraging this feature (by passing key: as an arg) will cause the use of the log_deprecation RSpec matcher to fail, because it explicitly expects no_args. This bug has no effect on production, only our test suite.

This feature loosens this constraint, allowing use of key args with log_deprecation and corresponding RSpec matcher.

Additional Notes:

This will also need to be backported to 1.x.

It would also be nice to augment the log_deprecation matcher to more explicitly handle the presence of a key arg in a way that tests the limitation set in place, aka expect { set_service_name }.to log_deprecation(include('service name')).with_limit(key: :deprecation_conf_service_name). However, this may not be an insignificant effort, and is out of scope for my efforts right now.

@delner delner added bug Involves a bug dev/testing Involves testing processes (e.g. RSpec) labels Jul 11, 2024
@delner delner added this to the 2.3.0 milestone Jul 11, 2024
@delner delner self-assigned this Jul 11, 2024
@delner delner requested a review from a team as a code owner July 11, 2024 19:16
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 97.91%. Comparing base (2a662b0) to head (ed52bfd).

Files Patch % Lines
spec/support/core_helpers.rb 50.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3781   +/-   ##
=======================================
  Coverage   97.91%   97.91%           
=======================================
  Files        1246     1246           
  Lines       74999    74999           
  Branches     3627     3627           
=======================================
  Hits        73436    73436           
  Misses       1563     1563           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@delner delner merged commit f867bd9 into master Jul 12, 2024
170 checks passed
@delner delner deleted the fix/rspec_log_deprecation_matcher_limit_arg branch July 12, 2024 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Involves a bug dev/testing Involves testing processes (e.g. RSpec)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants