From 7e95165a503f6ff6a2995da3acdea5419296c584 Mon Sep 17 00:00:00 2001 From: ChrisBAshton Date: Wed, 4 Dec 2024 14:41:33 +0000 Subject: [PATCH] Delete test for returning nil when requested variant not yet ready 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 ``` --- test/unit/lib/whitehall/asset_manager_storage_test.rb | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/test/unit/lib/whitehall/asset_manager_storage_test.rb b/test/unit/lib/whitehall/asset_manager_storage_test.rb index 625beefb963..00987e98c9c 100644 --- a/test/unit/lib/whitehall/asset_manager_storage_test.rb +++ b/test/unit/lib/whitehall/asset_manager_storage_test.rb @@ -61,17 +61,6 @@ class Whitehall::AssetManagerStorage::FileTest < ActiveSupport::TestCase assert_equal "http://assets-host/media/asset_manager_id_s216/s216_#{model.filename}", model.file.url(:s216) 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!