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

Bump rubocop-govuk from 4.18.0 to 5.0.1 #1425

Merged
merged 4 commits into from
Jun 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 10 additions & 16 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -648,30 +648,24 @@ 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)
rubocop (>= 1.33.0, < 2.0)
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)
Expand Down
9 changes: 3 additions & 6 deletions spec/controllers/media_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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")
Expand Down
10 changes: 5 additions & 5 deletions spec/lib/asset_processor_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -60,15 +60,15 @@
let(:scope) { Asset.deleted }

before do
asset_1.destroy
first_asset.destroy
end

it "iterates over all deleted 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)
expect(asset_ids).to contain_exactly(first_asset.id.to_s)
end
end

Expand Down
4 changes: 2 additions & 2 deletions spec/lib/cli_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion spec/lib/govuk_configuration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
2 changes: 1 addition & 1 deletion spec/lib/healthcheck/cloud_storage_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion spec/lib/s3_configuration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions spec/lib/s3_storage_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -319,29 +319,29 @@
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
allow(response).to receive(:successful?).and_return(false)
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
4 changes: 2 additions & 2 deletions spec/lib/virus_scanner_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down
4 changes: 2 additions & 2 deletions spec/models/asset_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion spec/models/whitehall_asset_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 8 additions & 8 deletions spec/requests/access_limited_assets_spec.rb
Original file line number Diff line number Diff line change
@@ -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 }

Expand All @@ -15,31 +15,31 @@
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)

expect(response).to be_successful
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)

expect(response).to be_forbidden
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)

expect(response).to be_successful
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)

Expand Down
8 changes: 4 additions & 4 deletions spec/requests/access_limited_whitehall_assets_spec.rb
Original file line number Diff line number Diff line change
@@ -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 }

Expand All @@ -13,15 +13,15 @@
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

expect(response).to be_successful
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

Expand Down
2 changes: 1 addition & 1 deletion spec/requests/asset_requests_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion spec/requests/virus_scanning_spec.rb
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion spec/workers/virus_scan_worker_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down