Skip to content

Commit

Permalink
Delete test for returning nil when requested variant not yet ready
Browse files Browse the repository at this point in the history
I tried swapping out the test definition for the ImageData model
instead, but the behaviour is different for ImageData - the
CarrierWave relative path is returned instead of the URL (as in the
`test "returns store path when the model has no assets, although it
should (still uploading or error has occurred)" do` test.

I'm not entirely clear why the behaviour of ImageData is different
to AttachmentData, but given attachments now only ever have the
'original' variant, I doubt this test is needed anymore. I'm also
not sure if there's a bit of corresponding logic in the AttachmentData
model that can be deleted (or the AssetManagerStorage model, which
is, after all, the thing we're supposed to be testing in this file).
Couldn't see any code simplification opportunities, so settling for
a deleted test.

Here's the code I attempted to swap it with:

```
  test "returns nil when the model has assets but the requested variant is not yet available" do
    model = build(:image_data)
    model.assets = model.assets - [model.assets.last]
    model.save!
    model.reload

    assert_nil model.file.url("s216".to_sym) # what would have been the 'last' asset
  end
```
  • Loading branch information
ChrisBAshton committed Dec 4, 2024
1 parent 30900b5 commit cd87e27
Showing 1 changed file with 0 additions and 11 deletions.
11 changes: 0 additions & 11 deletions test/unit/lib/whitehall/asset_manager_storage_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,17 +53,6 @@ class Whitehall::AssetManagerStorage::FileTest < ActiveSupport::TestCase
assert_equal "http://assets-host/media/asset_manager_id/sample.docx", model.file.url
end

# TODO: commented out to get the tests passing.
# We should better understand how to rewrite this test and whether it is still valuable.
#
# test "returns nil when the model has assets but the requested variant is not available" do
# model = build(:attachment_data_with_asset, attachable: build(:draft_edition, id: 1))
# model.save!
# model.reload

# assert_nil model.file.url(:thumbnail)
# end

test "returns store path when the model has no assets, although it should (still uploading or error has occurred)" do
model = build(:attachment_data_with_no_assets, attachable: build(:draft_edition, id: 1))
model.save!
Expand Down

0 comments on commit cd87e27

Please sign in to comment.