Skip to content

Commit

Permalink
WIP: Fix some Mocha deprecation warnings
Browse files Browse the repository at this point in the history
This fixes some of the warnings related to strict keyword argument
matching, like this one:

    Mocha deprecation warning at app/services/asset_manager/attachment_updater/update.rb:15:in `call':
    Expectation defined at test/unit/services/asset_manager/attachment_updater/draft_status_updates_test.rb:28:
    in `block (3 levels) in <class:DraftStatusUpdatesTest>'
    expected keyword arguments ("draft" => true),
    but received positional hash ({"draft" => true}).
    These will stop matching when strict keyword argument matching is enabled.
    See the documentation for Mocha::Configuration#strict_keyword_argument_matching=.

In order to fix the warnings, I've changed the signature of
AssetManager::AssetUpdater#call so that new_attributes parameter must be supplied
as keyword arguments rather than a positional Hash. This had the
advantage that I only needed to change
AssetManager::AttachmentUpdater::Update#call to convert the
deep_stringify_keys into keyword arguments using a double-splat; I
didn't need to change all the places where
AssetManager::AssetUpdater#call is stubbed.

An alternative would be to leave the signature of
AssetManager::AssetUpdater#call unchanged and change the calls to
Expectation#with for all the relevant stubs to pass a positional Hash,
i.e. wrapped in braces.

This article [1] is a very useful reference.

[1]: https://www.ruby-lang.org/en/news/2019/12/12/separation-of-positional-and-keyword-arguments-in-ruby-3-0/
  • Loading branch information
floehopper committed Oct 18, 2022
1 parent 4b62bcb commit 0a434c0
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 6 deletions.
6 changes: 3 additions & 3 deletions app/services/asset_manager/asset_updater.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ def initialize(attachment_data_id, legacy_url_path)
end
end

def self.call(*args)
new.call(*args)
def self.call(...)
new.call(...)
end

def call(asset_data, legacy_url_path, new_attributes = {})
def call(asset_data, legacy_url_path, **new_attributes)
attributes = find_asset_by(legacy_url_path)
asset_deleted = attributes["deleted"]

Expand Down
2 changes: 1 addition & 1 deletion app/services/asset_manager/attachment_updater/update.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ def call
AssetManager::AssetUpdater.call(
attachment_data,
legacy_url_path,
new_attributes.deep_stringify_keys,
**new_attributes.deep_stringify_keys,
)
end

Expand Down
4 changes: 2 additions & 2 deletions test/unit/services/asset_manager/asset_updater_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ class AssetManager::AssetUpdaterTest < ActiveSupport::TestCase
.with(@asset_id, "replacement_id" => replacement_id)

attributes = { "replacement_legacy_url_path" => replacement_legacy_url_path }
@worker.call(@attachment_data, @legacy_url_path, attributes)
@worker.call(@attachment_data, @legacy_url_path, **attributes)
end

test "does not mark asset as replaced if already replaced by same asset" do
Expand All @@ -130,6 +130,6 @@ class AssetManager::AssetUpdaterTest < ActiveSupport::TestCase
Services.asset_manager.expects(:update_asset).never

attributes = { "replacement_legacy_url_path" => replacement_legacy_url_path }
@worker.call(@attachment_data, @legacy_url_path, attributes)
@worker.call(@attachment_data, @legacy_url_path, **attributes)
end
end

0 comments on commit 0a434c0

Please sign in to comment.