From 14482bf6e0da38cce4db27633f0fdc057ce920ae Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 27 Jun 2024 06:36:02 +0000 Subject: [PATCH 1/4] Bump rubocop-govuk from 4.18.0 to 5.0.1 Bumps [rubocop-govuk](https://github.com/alphagov/rubocop-govuk) from 4.18.0 to 5.0.1. - [Changelog](https://github.com/alphagov/rubocop-govuk/blob/main/CHANGELOG.md) - [Commits](https://github.com/alphagov/rubocop-govuk/compare/v4.18.0...v5.0.1) --- updated-dependencies: - dependency-name: rubocop-govuk dependency-type: direct:development update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- Gemfile.lock | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 454e808d..7dc7078d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -526,8 +526,8 @@ GEM table_print (~> 1.5) term-ansicolor (~> 1.7) thor (>= 0.20, < 2.0) - parallel (1.24.0) - parser (3.3.2.0) + parallel (1.25.1) + parser (3.3.3.0) ast (~> 2.4.1) racc parslet (2.0.0) @@ -607,8 +607,8 @@ GEM http-cookie (>= 1.0.2, < 2.0) mime-types (>= 1.16, < 4.0) netrc (~> 0.8) - rexml (3.2.8) - strscan (>= 3.0.9) + rexml (3.3.1) + strscan rspec (3.13.0) rspec-core (~> 3.13.0) rspec-expectations (~> 3.13.0) @@ -648,16 +648,15 @@ GEM unicode-display_width (>= 2.4.0, < 3.0) rubocop-ast (1.31.3) parser (>= 3.3.1.0) - rubocop-capybara (2.20.0) + rubocop-capybara (2.21.0) rubocop (~> 1.41) - rubocop-factory_bot (2.25.1) - rubocop (~> 1.41) - rubocop-govuk (4.18.0) + rubocop-govuk (5.0.1) rubocop (= 1.64.1) rubocop-ast (= 1.31.3) + rubocop-capybara (= 2.21.0) rubocop-rails (= 2.25.0) rubocop-rake (= 0.6.0) - rubocop-rspec (= 2.30.0) + rubocop-rspec (= 3.0.1) rubocop-rails (2.25.0) activesupport (>= 4.2.0) rack (>= 1.1) @@ -665,13 +664,8 @@ GEM rubocop-ast (>= 1.31.1, < 2.0) rubocop-rake (0.6.0) rubocop (~> 1.0) - rubocop-rspec (2.30.0) - rubocop (~> 1.40) - rubocop-capybara (~> 2.17) - rubocop-factory_bot (~> 2.22) - rubocop-rspec_rails (~> 2.28) - rubocop-rspec_rails (2.28.3) - rubocop (~> 1.40) + rubocop-rspec (3.0.1) + rubocop (~> 1.61) ruby-progressbar (1.13.0) ruby-vips (2.2.1) ffi (~> 1.12) From 88217af267eb3a79f91f3528d8fea1426a8977de Mon Sep 17 00:00:00 2001 From: Bruce Bolt Date: Thu, 27 Jun 2024 10:34:59 +0100 Subject: [PATCH 2/4] Auto-correct rubocop failures --- spec/controllers/media_controller_spec.rb | 9 +++------ spec/lib/cli_spec.rb | 4 ++-- spec/lib/govuk_configuration_spec.rb | 2 +- spec/lib/healthcheck/cloud_storage_spec.rb | 2 +- spec/lib/s3_configuration_spec.rb | 2 +- spec/lib/s3_storage_spec.rb | 8 ++++---- spec/lib/virus_scanner_spec.rb | 4 ++-- spec/models/whitehall_asset_spec.rb | 2 +- spec/requests/asset_requests_spec.rb | 2 +- spec/requests/virus_scanning_spec.rb | 2 +- spec/workers/virus_scan_worker_spec.rb | 2 +- 11 files changed, 18 insertions(+), 21 deletions(-) diff --git a/spec/controllers/media_controller_spec.rb b/spec/controllers/media_controller_spec.rb index 7bcec48b..cc3c4d2e 100644 --- a/spec/controllers/media_controller_spec.rb +++ b/spec/controllers/media_controller_spec.rb @@ -35,11 +35,9 @@ def download allow(Services).to receive(:cloud_storage).and_return(cloud_storage) allow(cloud_storage).to receive(:presigned_url_for) .with(asset, http_method:).and_return(presigned_url) - allow(asset).to receive(:etag).and_return("599ffda8-e169") - allow(asset).to receive(:last_modified).and_return(last_modified) - allow(AssetManager).to receive(:content_disposition).and_return(content_disposition) + allow(asset).to receive_messages(etag: "599ffda8-e169", last_modified:) allow(content_disposition).to receive(:header_for).with(asset).and_return("content-disposition") - allow(AssetManager).to receive(:s3).and_return(s3) + allow(AssetManager).to receive_messages(content_disposition:, s3:) end context "when using real s3 in non-local environment" do @@ -101,8 +99,7 @@ def download end it "determines an Asset's content_type by filename when it is not set" do - allow(asset).to receive(:content_type).and_return(nil) - allow(asset).to receive(:filename).and_return("file.pdf") + allow(asset).to receive_messages(content_type: nil, filename: "file.pdf") get :download, params: { id: asset.id } expect(response.headers["Content-Type"]).to eq("application/pdf") diff --git a/spec/lib/cli_spec.rb b/spec/lib/cli_spec.rb index 6e53040f..44d01008 100644 --- a/spec/lib/cli_spec.rb +++ b/spec/lib/cli_spec.rb @@ -5,7 +5,7 @@ subject(:cli) { described_class.new(output, kernel) } let(:output) { StringIO.new } - let(:kernel) { class_double("Kernel") } + let(:kernel) { class_double(Kernel) } let(:path) { fixture_file_path("asset.png") } describe "#create_asset" do @@ -31,7 +31,7 @@ end it "does not create an asset" do - expect { cli.create_asset(*args) }.to change(Asset, :count).by 0 + expect { cli.create_asset(*args) }.not_to change(Asset, :count) end it "reports that asset was not saved" do diff --git a/spec/lib/govuk_configuration_spec.rb b/spec/lib/govuk_configuration_spec.rb index 9e06f6da..67307652 100644 --- a/spec/lib/govuk_configuration_spec.rb +++ b/spec/lib/govuk_configuration_spec.rb @@ -69,7 +69,7 @@ subject(:config) { described_class.new(env, plek) } let(:env) { {} } - let(:plek) { instance_double("Plek") } + let(:plek) { instance_double(Plek) } before do allow(plek).to receive(:external_url_for).with("draft-assets") diff --git a/spec/lib/healthcheck/cloud_storage_spec.rb b/spec/lib/healthcheck/cloud_storage_spec.rb index 86155bab..b0566162 100644 --- a/spec/lib/healthcheck/cloud_storage_spec.rb +++ b/spec/lib/healthcheck/cloud_storage_spec.rb @@ -4,7 +4,7 @@ RSpec.describe Healthcheck::CloudStorage do describe "#status" do - let(:storage) { instance_double("S3Storage") } + let(:storage) { instance_double(S3Storage) } before do allow(Services).to receive(:cloud_storage).and_return(storage) diff --git a/spec/lib/s3_configuration_spec.rb b/spec/lib/s3_configuration_spec.rb index ced1730a..01db120a 100644 --- a/spec/lib/s3_configuration_spec.rb +++ b/spec/lib/s3_configuration_spec.rb @@ -78,7 +78,7 @@ context "when AWS_S3_BUCKET_NAME env var is not present" do it "returns nil" do - expect(config.bucket_name).to eq(nil) + expect(config.bucket_name).to be_nil end end end diff --git a/spec/lib/s3_storage_spec.rb b/spec/lib/s3_storage_spec.rb index 111e47b0..4490a26d 100644 --- a/spec/lib/s3_storage_spec.rb +++ b/spec/lib/s3_storage_spec.rb @@ -319,14 +319,14 @@ describe "#healthy?" do let(:head_bucket_params) { { bucket: bucket_name } } - let(:response) { instance_double("Seahorse::Client::Response") } + let(:response) { instance_double(Seahorse::Client::Response) } it "returns true when s3 bucket is reachable" do allow(response).to receive(:successful?).and_return(true) allow(s3_client).to receive(:head_bucket) .with(head_bucket_params).and_return(response) - expect(storage.healthy?).to eq true + expect(storage.healthy?).to be true end it "returns false when struct returns returns false to successful?" do @@ -334,14 +334,14 @@ allow(s3_client).to receive(:head_bucket) .with(head_bucket_params).and_return(response) - expect(storage.healthy?).to eq false + expect(storage.healthy?).to be false end it "returns false when calling head_bucket returns an error" do allow(s3_client).to receive(:head_bucket) .with(head_bucket_params).and_raise(Aws::S3::Errors::NotFound.new(nil, nil)) - expect(storage.healthy?).to eq false + expect(storage.healthy?).to be false end end end diff --git a/spec/lib/virus_scanner_spec.rb b/spec/lib/virus_scanner_spec.rb index 1c34f8a4..5d14daab 100644 --- a/spec/lib/virus_scanner_spec.rb +++ b/spec/lib/virus_scanner_spec.rb @@ -7,7 +7,7 @@ let(:file_path) { "/path/to/file" } let(:output) { "" } - let(:status) { instance_double("Process::Status", exitstatus:) } + let(:status) { instance_double(Process::Status, exitstatus:) } let(:exitstatus) { 0 } before do @@ -24,7 +24,7 @@ let(:exitstatus) { 0 } it "returns true" do - expect(scanner.scan(file_path)).to eq(true) + expect(scanner.scan(file_path)).to be(true) end end diff --git a/spec/models/whitehall_asset_spec.rb b/spec/models/whitehall_asset_spec.rb index d98caaed..fadeac45 100644 --- a/spec/models/whitehall_asset_spec.rb +++ b/spec/models/whitehall_asset_spec.rb @@ -58,7 +58,7 @@ it "can find the most recent (undeleted) asset" do path = asset.legacy_url_path[1..] - expect(described_class.from_params(path:).deleted?).to eq(false) + expect(described_class.from_params(path:).deleted?).to be(false) end end end diff --git a/spec/requests/asset_requests_spec.rb b/spec/requests/asset_requests_spec.rb index 680dd291..50727eec 100644 --- a/spec/requests/asset_requests_spec.rb +++ b/spec/requests/asset_requests_spec.rb @@ -122,7 +122,7 @@ body = JSON.parse(response.body) expect(response).to have_http_status(:ok) - expect(body["deleted"]).to eq(true) + expect(body["deleted"]).to be(true) end end diff --git a/spec/requests/virus_scanning_spec.rb b/spec/requests/virus_scanning_spec.rb index 04a27518..a76ccec0 100644 --- a/spec/requests/virus_scanning_spec.rb +++ b/spec/requests/virus_scanning_spec.rb @@ -1,6 +1,6 @@ require "rails_helper" -RSpec.describe "Virus scanning of uploaded images", type: :request, disable_cloud_storage_stub: true do +RSpec.describe "Virus scanning of uploaded images", :disable_cloud_storage_stub, type: :request do let(:s3) { S3Configuration.build } before do diff --git a/spec/workers/virus_scan_worker_spec.rb b/spec/workers/virus_scan_worker_spec.rb index d9e7ac6b..f6a7831b 100644 --- a/spec/workers/virus_scan_worker_spec.rb +++ b/spec/workers/virus_scan_worker_spec.rb @@ -5,7 +5,7 @@ RSpec.describe VirusScanWorker do let(:worker) { described_class.new } let(:asset) { FactoryBot.create(:asset) } - let(:scanner) { instance_double("VirusScanner") } + let(:scanner) { instance_double(VirusScanner) } before do allow(Services).to receive(:virus_scanner).and_return(scanner) From ee7b9a880596db2caf92da284969c5d87d17715e Mon Sep 17 00:00:00 2001 From: Bruce Bolt Date: Thu, 27 Jun 2024 10:40:24 +0100 Subject: [PATCH 3/4] Fix useless assertion We were comparing the same value with the same expected value. Updating to correct this. Fixed rubocop failure `RSpec/IdenticalEqualityAssertion`. --- spec/models/asset_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/models/asset_spec.rb b/spec/models/asset_spec.rb index 6605cbd9..a7a5f22b 100644 --- a/spec/models/asset_spec.rb +++ b/spec/models/asset_spec.rb @@ -826,8 +826,8 @@ it "has 2nd part as file size (number of bytes written in lowercase hex)" do size_hex = asset.etag_from_file.split("-").last - size = size_hex.to_i(16) - expect(size).to eq(size) + asset_size = size_hex.to_i(16) + expect(asset_size).to eq(size) end end From 7812a1240e16486e317a642a1792e5c12ca7fa86 Mon Sep 17 00:00:00 2001 From: Bruce Bolt Date: Thu, 27 Jun 2024 10:41:55 +0100 Subject: [PATCH 4/4] Give variables meaningful names This fixes the rubocop failure `RSpec/IndexedLet`. --- spec/lib/asset_processor_spec.rb | 10 +++++----- spec/requests/access_limited_assets_spec.rb | 16 ++++++++-------- .../access_limited_whitehall_assets_spec.rb | 8 ++++---- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/spec/lib/asset_processor_spec.rb b/spec/lib/asset_processor_spec.rb index 56d89a37..7c0f5512 100644 --- a/spec/lib/asset_processor_spec.rb +++ b/spec/lib/asset_processor_spec.rb @@ -8,15 +8,15 @@ let(:output) { StringIO.new } let(:report_progress_every) { 1 } - let!(:asset_1) { FactoryBot.create(:asset) } - let!(:asset_2) { FactoryBot.create(:asset) } + let!(:first_asset) { FactoryBot.create(:asset) } + let!(:second_asset) { FactoryBot.create(:asset) } it "iterates over all assets" do asset_ids = [] processor.process_all_assets_with do |asset_id| asset_ids << asset_id end - expect(asset_ids).to contain_exactly(asset_1.id.to_s, asset_2.id.to_s) + expect(asset_ids).to contain_exactly(first_asset.id.to_s, second_asset.id.to_s) end it "reports progress for every asset" do @@ -60,7 +60,7 @@ let(:scope) { Asset.deleted } before do - asset_1.destroy + first_asset.destroy end it "iterates over all deleted assets" do @@ -68,7 +68,7 @@ processor.process_all_assets_with do |asset_id| asset_ids << asset_id end - expect(asset_ids).to contain_exactly(asset_1.id.to_s) + expect(asset_ids).to contain_exactly(first_asset.id.to_s) end end diff --git a/spec/requests/access_limited_assets_spec.rb b/spec/requests/access_limited_assets_spec.rb index 9b903a67..1c0f3e5c 100644 --- a/spec/requests/access_limited_assets_spec.rb +++ b/spec/requests/access_limited_assets_spec.rb @@ -1,10 +1,10 @@ require "rails_helper" RSpec.describe "Access limited assets", type: :request do - let(:user_1) { FactoryBot.create(:user, uid: "user-1-id") } - let(:user_2) { FactoryBot.create(:user, uid: "user-2-id") } - let(:user_3) { FactoryBot.create(:user, uid: "user-3-id", organisation_content_id: "org-a") } - let(:user_4) { FactoryBot.create(:user, uid: "user-4-id", organisation_content_id: "org-b") } + let(:authorised_user) { FactoryBot.create(:user, uid: "user-1-id") } + let(:unauthorised_user) { FactoryBot.create(:user, uid: "user-2-id") } + let(:user_from_authorised_organisation) { FactoryBot.create(:user, uid: "user-3-id", organisation_content_id: "org-a") } + let(:user_from_unauthorised_organisation) { FactoryBot.create(:user, uid: "user-4-id", organisation_content_id: "org-b") } let(:asset) { FactoryBot.create(:uploaded_asset, draft: true, access_limited: ["user-1-id"], access_limited_organisation_ids: %w[org-a]) } let(:s3) { S3Configuration.build } @@ -15,7 +15,7 @@ end it "are accessible to users who are authorised to view them" do - login_as user_1 + login_as authorised_user get download_media_path(id: asset, filename: asset.filename) @@ -23,7 +23,7 @@ end it "are not accessible to users who are not authorised to view them" do - login_as user_2 + login_as unauthorised_user get download_media_path(id: asset, filename: asset.filename) @@ -31,7 +31,7 @@ end it "are accessible to users in organisations authorised to view them" do - login_as user_3 + login_as user_from_authorised_organisation get download_media_path(id: asset, filename: asset.filename) @@ -39,7 +39,7 @@ end it "are not accessible to users in organisations not authorised to view them" do - login_as user_4 + login_as user_from_unauthorised_organisation get download_media_path(id: asset, filename: asset.filename) diff --git a/spec/requests/access_limited_whitehall_assets_spec.rb b/spec/requests/access_limited_whitehall_assets_spec.rb index 7b05301c..16e3ed9a 100644 --- a/spec/requests/access_limited_whitehall_assets_spec.rb +++ b/spec/requests/access_limited_whitehall_assets_spec.rb @@ -1,8 +1,8 @@ require "rails_helper" RSpec.describe "Access limited Whitehall assets", type: :request do - let(:user_1) { FactoryBot.create(:user, uid: "user-1-id") } - let(:user_2) { FactoryBot.create(:user, uid: "user-2-id") } + let(:authorised_user) { FactoryBot.create(:user, uid: "user-1-id") } + let(:unauthorised_user) { FactoryBot.create(:user, uid: "user-2-id") } let(:asset) { FactoryBot.create(:uploaded_whitehall_asset, draft: true, access_limited: ["user-1-id"]) } let(:s3) { S3Configuration.build } @@ -13,7 +13,7 @@ end it "are accessible to users who are authorised to view them" do - login_as user_1 + login_as authorised_user get asset.legacy_url_path @@ -21,7 +21,7 @@ end it "are not accessible to users who are not authorised to view them" do - login_as user_2 + login_as unauthorised_user get asset.legacy_url_path