From 59a7a7648bd7617c35f797017ff48abc465fa58b Mon Sep 17 00:00:00 2001 From: Keith Lawrence Date: Wed, 11 Dec 2024 09:46:38 +0000 Subject: [PATCH 01/19] configure rubocop for rspec --- .rubocop.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.rubocop.yml b/.rubocop.yml index 716f3c555e..990519905b 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -2,6 +2,7 @@ inherit_gem: rubocop-govuk: - config/default.yml - config/rails.yml + - config/rspec.yml inherit_mode: merge: From 49d93bbb0239e07cae500481c6d98b81843c1e29 Mon Sep 17 00:00:00 2001 From: Keith Lawrence Date: Wed, 8 Jan 2025 15:15:16 +0000 Subject: [PATCH 02/19] run rubocop --autocorrect --- spec/helpers/parts_navigation_helper_spec.rb | 1 + spec/helpers/theme_type_helper_spec.rb | 1 + spec/models/content_item_spec.rb | 4 +- spec/models/get_involved_spec.rb | 2 +- .../landing_page/block/block_error_spec.rb | 6 +-- .../block/blocks_container_spec.rb | 6 +-- spec/models/landing_page/block/box_spec.rb | 6 +-- spec/models/landing_page/block/card_spec.rb | 6 +-- .../landing_page/block/columns_layout_spec.rb | 6 +-- .../landing_page/block/featured_spec.rb | 6 +-- spec/models/landing_page/block/hero_spec.rb | 8 ++-- spec/models/landing_page/block/image_spec.rb | 6 +-- .../block/main_navigation_spec.rb | 6 +-- .../landing_page/block/share_links_spec.rb | 6 +-- .../block/side_navigation_spec.rb | 6 +-- .../block/two_column_layout_spec.rb | 6 +-- spec/models/travel_advice_spec.rb | 4 +- spec/requests/local_transactions_spec.rb | 2 +- spec/requests/travel_advice_spec.rb | 1 + spec/support/concerns/parts.rb | 4 +- spec/system/find_local_council_spec.rb | 12 +++--- spec/system/get_involved_spec.rb | 3 +- spec/system/help_page_spec.rb | 4 +- spec/system/licence_transaction_spec.rb | 38 ++++++++++--------- spec/system/local_transactions_spec.rb | 12 +++--- spec/system/phase_banner_spec.rb | 1 + spec/system/place_spec.rb | 5 ++- spec/system/simple_smart_answers_spec.rb | 30 +++++++-------- spec/system/take_part_spec.rb | 6 +-- spec/system/travel_advice_spec.rb | 10 ++--- spec/unit/calendar_division_spec.rb | 2 + spec/unit/calendar_spec.rb | 2 +- spec/unit/locales_validation_spec.rb | 2 +- .../content_item_model_presenter_spec.rb | 2 +- .../travel_advice_presenter_spec.rb | 2 +- .../unit/services/csv_preview_service_spec.rb | 1 + 36 files changed, 118 insertions(+), 107 deletions(-) diff --git a/spec/helpers/parts_navigation_helper_spec.rb b/spec/helpers/parts_navigation_helper_spec.rb index 393331e3e3..80ce8b15b5 100644 --- a/spec/helpers/parts_navigation_helper_spec.rb +++ b/spec/helpers/parts_navigation_helper_spec.rb @@ -59,6 +59,7 @@ describe "part_link_elements" do let(:parts) { [part_one, part_two] } + it "sets an active link on the cuurent part" do current_part = part_two expected_elements = [ diff --git a/spec/helpers/theme_type_helper_spec.rb b/spec/helpers/theme_type_helper_spec.rb index 6c7543b1b0..fee1692c71 100644 --- a/spec/helpers/theme_type_helper_spec.rb +++ b/spec/helpers/theme_type_helper_spec.rb @@ -5,6 +5,7 @@ it "returns theme type default when style is empty" do expect(style("")).to eq("theme-default") end + it "returns theme type default when style is invalid" do expect(style("asdfghjkl")).to eq("theme-default") end diff --git a/spec/models/content_item_spec.rb b/spec/models/content_item_spec.rb index d6a37caccc..172d6d1116 100644 --- a/spec/models/content_item_spec.rb +++ b/spec/models/content_item_spec.rb @@ -1,8 +1,8 @@ RSpec.describe ContentItem do - it_behaves_like "it can be withdrawn", "detailed_guide", "withdrawn_detailed_guide" - let(:subject) { build(:content_item_with_data_attachments, schema_name: "fancy_page_type") } + it_behaves_like "it can be withdrawn", "detailed_guide", "withdrawn_detailed_guide" + describe "ordered_related_items attribute" do it "leaves ordered_related_items if set" do subject = described_class.new( diff --git a/spec/models/get_involved_spec.rb b/spec/models/get_involved_spec.rb index e184515eb3..95d0a22d15 100644 --- a/spec/models/get_involved_spec.rb +++ b/spec/models/get_involved_spec.rb @@ -27,7 +27,7 @@ end describe "#consultation outcome" do - it "returns the recent consultation outcome " do + it "returns the recent consultation outcome" do model_instance = GetInvolved.new(content_item) expect(model_instance.recent_consultation_outcomes).to eq([consultation_result]) diff --git a/spec/models/landing_page/block/block_error_spec.rb b/spec/models/landing_page/block/block_error_spec.rb index 594078b099..65e67b47f6 100644 --- a/spec/models/landing_page/block/block_error_spec.rb +++ b/spec/models/landing_page/block/block_error_spec.rb @@ -1,13 +1,13 @@ RSpec.describe LandingPage::Block::BlockError do - it_behaves_like "it is a landing-page block" - + let(:subject) { described_class.new(blocks_hash, build(:landing_page)) } let(:blocks_hash) do { "type" => "block_error", "error" => StandardError.new("This error"), } end - let(:subject) { described_class.new(blocks_hash, build(:landing_page)) } + + it_behaves_like "it is a landing-page block" it "has an error" do expect(subject.error).to be_instance_of(StandardError) diff --git a/spec/models/landing_page/block/blocks_container_spec.rb b/spec/models/landing_page/block/blocks_container_spec.rb index aeff940fe8..71716ca75e 100644 --- a/spec/models/landing_page/block/blocks_container_spec.rb +++ b/spec/models/landing_page/block/blocks_container_spec.rb @@ -1,6 +1,5 @@ RSpec.describe LandingPage::Block::BlocksContainer do - it_behaves_like "it is a landing-page block" - + let(:subject) { described_class.new(blocks_hash, build(:landing_page)) } let(:blocks_hash) do { "type" => "blocks_container", @@ -20,7 +19,8 @@ ], } end - let(:subject) { described_class.new(blocks_hash, build(:landing_page)) } + + it_behaves_like "it is a landing-page block" it "has children blocks" do expect(subject.children.size).to eq(3) diff --git a/spec/models/landing_page/block/box_spec.rb b/spec/models/landing_page/block/box_spec.rb index 1391621d9d..a78551a144 100644 --- a/spec/models/landing_page/block/box_spec.rb +++ b/spec/models/landing_page/block/box_spec.rb @@ -1,6 +1,5 @@ RSpec.describe LandingPage::Block::Box do - it_behaves_like "it is a landing-page block" - + let(:subject) { described_class.new(blocks_hash, build(:landing_page)) } let(:blocks_hash) do { "type" => "box", "href" => "/landing-page/something", @@ -12,7 +11,8 @@ ], } } end - let(:subject) { described_class.new(blocks_hash, build(:landing_page)) } + + it_behaves_like "it is a landing-page block" it "includes a link" do expect(subject.href).to eq "/landing-page/something" diff --git a/spec/models/landing_page/block/card_spec.rb b/spec/models/landing_page/block/card_spec.rb index 344e78a45e..0a76469b50 100644 --- a/spec/models/landing_page/block/card_spec.rb +++ b/spec/models/landing_page/block/card_spec.rb @@ -1,6 +1,5 @@ RSpec.describe LandingPage::Block::Card do - it_behaves_like "it is a landing-page block" - + let(:subject) { described_class.new(blocks_hash, build(:landing_page)) } let(:blocks_hash) do { "type" => "card", "href" => "/landing-page/something", @@ -16,7 +15,8 @@ ], } } end - let(:subject) { described_class.new(blocks_hash, build(:landing_page)) } + + it_behaves_like "it is a landing-page block" describe "#link" do it "includes a link" do diff --git a/spec/models/landing_page/block/columns_layout_spec.rb b/spec/models/landing_page/block/columns_layout_spec.rb index f9c86b734d..aac737fb9d 100644 --- a/spec/models/landing_page/block/columns_layout_spec.rb +++ b/spec/models/landing_page/block/columns_layout_spec.rb @@ -1,6 +1,5 @@ RSpec.describe LandingPage::Block::ColumnsLayout do - it_behaves_like "it is a landing-page block" - + let(:subject) { described_class.new(blocks_hash, build(:landing_page)) } let(:blocks_hash) do { "type" => "columns_layout", @@ -20,7 +19,8 @@ ], } end - let(:subject) { described_class.new(blocks_hash, build(:landing_page)) } + + it_behaves_like "it is a landing-page block" it "has column blocks" do expect(subject.blocks.size).to eq(3) diff --git a/spec/models/landing_page/block/featured_spec.rb b/spec/models/landing_page/block/featured_spec.rb index 8130a7a8af..cd22e59076 100644 --- a/spec/models/landing_page/block/featured_spec.rb +++ b/spec/models/landing_page/block/featured_spec.rb @@ -1,6 +1,5 @@ RSpec.describe LandingPage::Block::Featured do - it_behaves_like "it is a landing-page block" - + let(:subject) { described_class.new(blocks_hash, build(:landing_page)) } let(:blocks_hash) do { "type" => "featured", "image" => { @@ -21,7 +20,8 @@ ], } } end - let(:subject) { described_class.new(blocks_hash, build(:landing_page)) } + + it_behaves_like "it is a landing-page block" describe "#image" do it "returns the properties of the image" do diff --git a/spec/models/landing_page/block/hero_spec.rb b/spec/models/landing_page/block/hero_spec.rb index 2ff422bdff..549d1ab814 100644 --- a/spec/models/landing_page/block/hero_spec.rb +++ b/spec/models/landing_page/block/hero_spec.rb @@ -1,6 +1,5 @@ RSpec.describe LandingPage::Block::Hero do - it_behaves_like "it is a landing-page block" - + let(:subject) { described_class.new(blocks_hash, build(:landing_page)) } let(:blocks_hash) do { "type" => "hero", "image" => { @@ -21,7 +20,8 @@ ], } } end - let(:subject) { described_class.new(blocks_hash, build(:landing_page)) } + + it_behaves_like "it is a landing-page block" describe "#image" do it "returns the properties of the image" do @@ -60,7 +60,7 @@ hero_block = described_class.new(blocks_hash, build(:landing_page)) - expect(hero_block.hero_content).to be nil + expect(hero_block.hero_content).to be_nil end end diff --git a/spec/models/landing_page/block/image_spec.rb b/spec/models/landing_page/block/image_spec.rb index 4b7670ee7f..77a7dbc916 100644 --- a/spec/models/landing_page/block/image_spec.rb +++ b/spec/models/landing_page/block/image_spec.rb @@ -1,6 +1,5 @@ RSpec.describe LandingPage::Block::Image do - it_behaves_like "it is a landing-page block" - + let(:subject) { described_class.new(blocks_hash, build(:landing_page)) } let(:blocks_hash) do { "type" => "image", "image" => { @@ -15,7 +14,8 @@ }, } } end - let(:subject) { described_class.new(blocks_hash, build(:landing_page)) } + + it_behaves_like "it is a landing-page block" describe "#image" do it "returns the properties of the image" do diff --git a/spec/models/landing_page/block/main_navigation_spec.rb b/spec/models/landing_page/block/main_navigation_spec.rb index ff3f90d6fe..a081a1ceab 100644 --- a/spec/models/landing_page/block/main_navigation_spec.rb +++ b/spec/models/landing_page/block/main_navigation_spec.rb @@ -1,13 +1,13 @@ RSpec.describe LandingPage::Block::MainNavigation do - it_behaves_like "it is a landing-page block" - + let(:subject) { described_class.new(blocks_hash, build(:landing_page_with_navigation_groups)) } let(:blocks_hash) do { "type" => "main_navigation", "navigation_group_id" => "Top Menu", } end - let(:subject) { described_class.new(blocks_hash, build(:landing_page_with_navigation_groups)) } + + it_behaves_like "it is a landing-page block" describe "#links" do it "returns an array of links from the specified navigation group" do diff --git a/spec/models/landing_page/block/share_links_spec.rb b/spec/models/landing_page/block/share_links_spec.rb index 78fb3896e4..fdea42de39 100644 --- a/spec/models/landing_page/block/share_links_spec.rb +++ b/spec/models/landing_page/block/share_links_spec.rb @@ -1,6 +1,5 @@ RSpec.describe LandingPage::Block::ShareLinks do - it_behaves_like "it is a landing-page block" - + let(:subject) { described_class.new(blocks_hash, build(:landing_page)) } let(:blocks_hash) do { "type" => "share_links", @@ -20,7 +19,8 @@ ], } end - let(:subject) { described_class.new(blocks_hash, build(:landing_page)) } + + it_behaves_like "it is a landing-page block" it "returns all of the side navigation links" do expect(subject.links.count).to eq(2) diff --git a/spec/models/landing_page/block/side_navigation_spec.rb b/spec/models/landing_page/block/side_navigation_spec.rb index acae55e6ef..74b68c957b 100644 --- a/spec/models/landing_page/block/side_navigation_spec.rb +++ b/spec/models/landing_page/block/side_navigation_spec.rb @@ -1,13 +1,13 @@ RSpec.describe LandingPage::Block::SideNavigation do - it_behaves_like "it is a landing-page block" - + let(:subject) { described_class.new(blocks_hash, build(:landing_page_with_navigation_groups)) } let(:blocks_hash) do { "type" => "side_navigation", "navigation_group_id" => "Submenu", } end - let(:subject) { described_class.new(blocks_hash, build(:landing_page_with_navigation_groups)) } + + it_behaves_like "it is a landing-page block" describe "#links" do it "returns all of the navigation links from the specified navigation group" do diff --git a/spec/models/landing_page/block/two_column_layout_spec.rb b/spec/models/landing_page/block/two_column_layout_spec.rb index 4f53701205..d96d9095b6 100644 --- a/spec/models/landing_page/block/two_column_layout_spec.rb +++ b/spec/models/landing_page/block/two_column_layout_spec.rb @@ -1,6 +1,5 @@ RSpec.describe LandingPage::Block::TwoColumnLayout do - it_behaves_like "it is a landing-page block" - + let(:subject) { described_class.new(blocks_hash, build(:landing_page)) } let(:blocks_hash) do { "type" => "two_column_layout", "theme" => "two_thirds_one_third", @@ -9,7 +8,8 @@ { "type" => "govspeak", "content" => "

Right content!

" }, ] } end - let(:subject) { described_class.new(blocks_hash, build(:landing_page)) } + + it_behaves_like "it is a landing-page block" describe "#left_column_class" do it "returns two thirds when the theme is two_thirds_one_third" do diff --git a/spec/models/travel_advice_spec.rb b/spec/models/travel_advice_spec.rb index 8b934a2dc8..05c7499139 100644 --- a/spec/models/travel_advice_spec.rb +++ b/spec/models/travel_advice_spec.rb @@ -1,10 +1,10 @@ RSpec.describe TravelAdvice do - it_behaves_like "it has parts", "travel_advice", "full-country" - before do @content_store_response = GovukSchemas::Example.find("travel_advice", example_name: "full-country") end + it_behaves_like "it has parts", "travel_advice", "full-country" + describe "#alert_status" do it "adds allowed statuses" do @content_store_response["details"]["alert_status"] = [described_class::ALERT_STATUSES.first] diff --git a/spec/requests/local_transactions_spec.rb b/spec/requests/local_transactions_spec.rb index daaf7aee41..45303f1394 100644 --- a/spec/requests/local_transactions_spec.rb +++ b/spec/requests/local_transactions_spec.rb @@ -90,7 +90,7 @@ get "/send-a-bear-to-your-local-council" expect(response).to have_http_status(:ok) - expect("Send a bear to your local council").to eq(assigns(:publication).title) + expect(assigns(:publication).title).to eq("Send a bear to your local council") end it "sets correct expiry headers" do diff --git a/spec/requests/travel_advice_spec.rb b/spec/requests/travel_advice_spec.rb index 02d1183a3a..5d76ffcf76 100644 --- a/spec/requests/travel_advice_spec.rb +++ b/spec/requests/travel_advice_spec.rb @@ -111,6 +111,7 @@ @country_path = @content_item.fetch("base_path") stub_content_store_has_item(@country_path, @content_item) end + it "redirects to the base_path if the part doesn't exist" do get "#{@country_path}/i-dont-exist" diff --git a/spec/support/concerns/parts.rb b/spec/support/concerns/parts.rb index 60a2574c06..bbd75593ac 100644 --- a/spec/support/concerns/parts.rb +++ b/spec/support/concerns/parts.rb @@ -30,14 +30,14 @@ part_slug = @content_store_response.dig("details", "parts").first["slug"] content_item.set_current_part(part_slug) - expect(content_item.next_part["slug"]).to_not eq(part_slug) + expect(content_item.next_part["slug"]).not_to eq(part_slug) end it "gets the previous part" do content_item = described_class.new(@content_store_response) content_item.set_current_part(@part_slug) - expect(content_item.previous_part["slug"]).to_not eq(@part_slug) + expect(content_item.previous_part["slug"]).not_to eq(@part_slug) end describe "#first_part?" do diff --git a/spec/system/find_local_council_spec.rb b/spec/system/find_local_council_spec.rb index 8018e2651c..a510bea0ca 100644 --- a/spec/system/find_local_council_spec.rb +++ b/spec/system/find_local_council_spec.rb @@ -53,7 +53,7 @@ end it "redirects to the authority slug" do - expect(current_path).to eq("/find-local-council/westminster") + expect(page).to have_current_path("/find-local-council/westminster", ignore_query: true) end it "shows the local authority result" do @@ -107,7 +107,7 @@ end it "redirects to the district authority slug" do - expect(current_path).to eq("/find-local-council/aylesbury") + expect(page).to have_current_path("/find-local-council/aylesbury", ignore_query: true) end it "includes the search result text in the page title" do @@ -171,7 +171,7 @@ end it "redirects to the district authority slug" do - expect(current_path).to eq("/find-local-council/aylesbury") + expect(page).to have_current_path("/find-local-council/aylesbury", ignore_query: true) end it "includes the search result text in the page title" do @@ -220,7 +220,7 @@ end it "remains on the find your local council page" do - expect(current_path).to eq("/find-local-council") + expect(page).to have_current_path("/find-local-council", ignore_query: true) expect(page).to have_content("Find your local council") expect(page).to have_content("Find the website for your local council.") end @@ -257,7 +257,7 @@ end it "remains on the find your local council page" do - expect(current_path).to eq("/find-local-council") + expect(page).to have_current_path("/find-local-council", ignore_query: true) expect(page).to have_content("Find your local council") expect(page).to have_content("Find the website for your local council.") end @@ -373,7 +373,7 @@ end it "remains on the find your local council page" do - expect(current_path).to eq("/find-local-council") + expect(page).to have_current_path("/find-local-council", ignore_query: true) expect(page).to have_content("Find your local council") end diff --git a/spec/system/get_involved_spec.rb b/spec/system/get_involved_spec.rb index 0fce635368..2574bf3844 100644 --- a/spec/system/get_involved_spec.rb +++ b/spec/system/get_involved_spec.rb @@ -1,5 +1,4 @@ RSpec.describe "Get Involved" do - it_behaves_like "it has meta tags", "get_involved", "/government/get-involved" before do content_store_has_example_item("/government/get-involved", schema: :get_involved) stub_search_query(query: hash_including(filter_content_store_document_type: "open_consultation"), response: { "results" => [], "total" => 83 }) @@ -8,6 +7,8 @@ visit "/government/get-involved" end + it_behaves_like "it has meta tags", "get_involved", "/government/get-involved" + context "when visiting get involved page" do it "displays the get involved page with the correct title" do expect(page).to have_title("Get involved - GOV.UK") diff --git a/spec/system/help_page_spec.rb b/spec/system/help_page_spec.rb index ca8bcd0e2d..0427e7e0cb 100644 --- a/spec/system/help_page_spec.rb +++ b/spec/system/help_page_spec.rb @@ -1,11 +1,11 @@ RSpec.describe "HelpPage" do - it_behaves_like "it has meta tags", "help_page", "/help/about-govuk" - before do content_store_has_example_item("/help/about-govuk", schema: :help_page, example: "about-govuk") content_store_has_example_item("/help/cookie-details", schema: :help_page, example: "cookie-details") end + it_behaves_like "it has meta tags", "help_page", "/help/about-govuk" + context "when visiting 'help/:slug'" do it "displays the help page using a content item" do visit "/help/about-govuk" diff --git a/spec/system/licence_transaction_spec.rb b/spec/system/licence_transaction_spec.rb index 4bfbffa7ab..f76731e0ff 100644 --- a/spec/system/licence_transaction_spec.rb +++ b/spec/system/licence_transaction_spec.rb @@ -126,7 +126,7 @@ end it "redirects to the appropriate authority slug" do - expect(current_path).to eq("/find-licences/licence-to-kill/westminster") + expect(page).to have_current_path("/find-licences/licence-to-kill/westminster", ignore_query: true) end it "includes the authority name in the title element" do @@ -287,7 +287,7 @@ end it "redirects to the appropriate authority slug" do - expect(current_path).to eq("/find-licences/licence-to-kill/buckinghamshire") + expect(page).to have_current_path("/find-licences/licence-to-kill/buckinghamshire", ignore_query: true) end it "includes the authority name in the title element" do @@ -353,7 +353,7 @@ end it "redirects to the appropriate authority slug" do - expect(current_path).to eq("/find-licences/licence-to-kill/dorset") + expect(page).to have_current_path("/find-licences/licence-to-kill/dorset", ignore_query: true) end it "includes the authority name in the title element" do @@ -450,7 +450,7 @@ end it "remains on the licence page" do - expect(current_path).to eq("/find-licences/licence-to-kill") + expect(page).to have_current_path("/find-licences/licence-to-kill", ignore_query: true) end it "shows an error message" do @@ -485,7 +485,7 @@ end it "remains on the licence page" do - expect(current_path).to eq("/find-licences/licence-to-kill") + expect(page).to have_current_path("/find-licences/licence-to-kill", ignore_query: true) end it "shows an error message" do @@ -511,7 +511,7 @@ end it "remains on the licence page" do - expect(current_path).to eq("/find-licences/licence-to-kill") + expect(page).to have_current_path("/find-licences/licence-to-kill", ignore_query: true) end it "shows an error message" do @@ -576,7 +576,7 @@ end it "returns the first authority with an actionable licence" do - expect(current_path).to eq("/find-licences/licence-to-kill/staffordshire") + expect(page).to have_current_path("/find-licences/licence-to-kill/staffordshire", ignore_query: true) end end @@ -594,7 +594,7 @@ end it "returns licence not found template" do - expect(current_path).to eq("/find-licences/licence-to-kill") + expect(page).to have_current_path("/find-licences/licence-to-kill", ignore_query: true) expect(page).to have_content("You cannot apply for this licence online.") end end @@ -799,11 +799,12 @@ expect(page).to have_content("Start now") end end + context "when visiting the licence with an authority slug" do before { visit "/find-licences/artistic-license/miniluv" } it "redirects to the search page" do - expect(page.current_path).to eq("/find-licences/artistic-license") + expect(page).to have_current_path("/find-licences/artistic-license", ignore_query: true) end end end @@ -1020,7 +1021,7 @@ it "displays the interactions for the licence if usesLicensify is set to true" do click_link("How to apply") - expect(current_path).to eq("/find-licences/licence-to-kill/miniluv/apply") + expect(page).to have_current_path("/find-licences/licence-to-kill/miniluv/apply", ignore_query: true) expect(page).to have_button_as_link("Apply online", href: "/new-licence/ministry-of-love/apply-1", start: true) expect(page).not_to have_content("You cannot apply for this licence online") expect(page).not_to have_content("Contact your local council") @@ -1029,7 +1030,7 @@ it "does not display the interactions for the licence if usesLicensify is set to false" do click_link("How to renew") - expect(current_path).to eq("/find-licences/licence-to-kill/miniluv/renew") + expect(page).to have_current_path("/find-licences/licence-to-kill/miniluv/renew", ignore_query: true) expect(page).not_to have_button_as_link("Apply online", href: "/new-licence/ministry-of-love/renew-1", start: true) expect(page).to have_content("You cannot apply for this licence online") expect(page).to have_content("Contact your local council") @@ -1095,7 +1096,7 @@ it "displays the licence unavailable message after you click on the first action" do click_on("How to apply") - expect(current_path).to eq("/find-licences/licence-to-kill/miniluv/apply") + expect(page).to have_current_path("/find-licences/licence-to-kill/miniluv/apply", ignore_query: true) expect(page).not_to have_button_as_link("Apply online", href: "/new-licence/ministry-of-love/apply-1", start: true) expect(page).to have_content("You cannot apply for this licence online") expect(page).to have_content("Contact your local council") @@ -1104,12 +1105,13 @@ it "displays the licence unavailable message after you click on the second action" do click_on("How to renew") - expect(current_path).to eq("/find-licences/licence-to-kill/miniluv/renew") + expect(page).to have_current_path("/find-licences/licence-to-kill/miniluv/renew", ignore_query: true) expect(page).not_to have_button_as_link("Apply online", href: "/new-licence/ministry-of-love/renew-1", start: true) expect(page).to have_content("You cannot apply for this licence online") expect(page).to have_content("Contact your local council") end end + context "when usesLicensify is missing for one action" do before do authorities = [ @@ -1166,7 +1168,7 @@ it "does not display interactions for licence with missing usesLicensify" do click_on("How to apply") - expect(current_path).to eq("/find-licences/licence-to-kill/miniluv/apply") + expect(page).to have_current_path("/find-licences/licence-to-kill/miniluv/apply", ignore_query: true) expect(page).not_to have_button_as_link("Apply online", href: "/new-licence/ministry-of-love/apply-1", start: true) expect(page).to have_content("You cannot apply for this licence online") expect(page).to have_content("Contact your local council") @@ -1175,7 +1177,7 @@ it "displays interactions for licence with usesLicensify set to true" do click_on("How to renew") - expect(current_path).to eq("/find-licences/licence-to-kill/miniluv/renew") + expect(page).to have_current_path("/find-licences/licence-to-kill/miniluv/renew", ignore_query: true) expect(page).to have_button_as_link("Apply online", href: "/new-licence/ministry-of-love/renew-1", start: true) expect(page).not_to have_content("You cannot apply for this licence online") expect(page).not_to have_content("Contact your local council") @@ -1239,7 +1241,7 @@ it "displays the licence unavailable message after you click on an action" do click_on("How to apply") - expect(current_path).to eq("/find-licences/licence-to-kill/miniluv/apply") + expect(page).to have_current_path("/find-licences/licence-to-kill/miniluv/apply", ignore_query: true) expect(page).not_to have_button_as_link("Apply online", href: "/new-licence/ministry-of-love/apply-1", start: true) expect(page).to have_content("You cannot apply for this licence online") expect(page).to have_content("Contact your local council") @@ -1326,7 +1328,7 @@ it "displays the interactions for the licence if usesLicensify is set to true for a link" do click_link("How to apply") - expect(current_path).to eq("/find-licences/licence-to-kill/miniluv/apply") + expect(page).to have_current_path("/find-licences/licence-to-kill/miniluv/apply", ignore_query: true) expect(page).to have_button_as_link("Apply online", href: "/new-licence/ministry-of-love/apply-1", start: true) expect(page).to have_button_as_link("Apply online", start: true, href: "/new-licence/ministry-of-love/apply-3") expect(page).not_to have_button_as_link("Apply online", href: "/new-licence/ministry-of-love/apply-2", start: true) @@ -1337,7 +1339,7 @@ it "does not display the interactions for the licence if usesLicensify is set to false or is missing for a link" do click_link("How to renew") - expect(current_path).to eq("/find-licences/licence-to-kill/miniluv/renew") + expect(page).to have_current_path("/find-licences/licence-to-kill/miniluv/renew", ignore_query: true) expect(page).not_to have_button_as_link("Apply online", href: "/new-licence/ministry-of-love/renew-1", start: true) expect(page).not_to have_button_as_link("Apply online", href: "/new-licence/ministry-of-love/renew-2", start: true) expect(page).to have_content("You cannot apply for this licence online") diff --git a/spec/system/local_transactions_spec.rb b/spec/system/local_transactions_spec.rb index e117614144..7e307cf2d9 100644 --- a/spec/system/local_transactions_spec.rb +++ b/spec/system/local_transactions_spec.rb @@ -102,7 +102,7 @@ end it "redirects to the appropriate authority slug" do - expect(current_path).to eq("/pay-bear-tax/westminster") + expect(page).to have_current_path("/pay-bear-tax/westminster", ignore_query: true) end it "includes the search result text in the page title" do @@ -161,7 +161,7 @@ end it "remains on the pay bear tax page" do - expect(current_path).to eq("/pay-bear-tax") + expect(page).to have_current_path("/pay-bear-tax", ignore_query: true) end it "shows an error message" do @@ -188,7 +188,7 @@ end it "remains on the local transaction page" do - expect(current_path).to eq("/pay-bear-tax") + expect(page).to have_current_path("/pay-bear-tax", ignore_query: true) end it "shows an error message" do @@ -212,7 +212,7 @@ end it "remains on the local transaction page" do - expect(current_path).to eq("/pay-bear-tax") + expect(page).to have_current_path("/pay-bear-tax", ignore_query: true) end it "shows an error message" do @@ -377,7 +377,7 @@ end it "redirects to the appropriate authority slug" do - expect(current_path).to eq("/pay-bear-tax/westminster") + expect(page).to have_current_path("/pay-bear-tax/westminster", ignore_query: true) end it "includes the search result text in the page title" do @@ -411,7 +411,7 @@ end it "redirects to the appropriate authority slug" do - expect(current_path).to eq("/pay-bear-tax/westminster") + expect(page).to have_current_path("/pay-bear-tax/westminster", ignore_query: true) end it "does not link to the authority" do diff --git a/spec/system/phase_banner_spec.rb b/spec/system/phase_banner_spec.rb index 924c005618..eabb3d675e 100644 --- a/spec/system/phase_banner_spec.rb +++ b/spec/system/phase_banner_spec.rb @@ -3,6 +3,7 @@ RSpec.describe "Phase Banner" do let(:base_path) { "/help/about-govuk" } + context "in the live phase" do before do content_store_has_example_item(base_path, schema: :help_page, example: "about-govuk") diff --git a/spec/system/place_spec.rb b/spec/system/place_spec.rb index 2f31151ed5..c85f04e59d 100644 --- a/spec/system/place_spec.rb +++ b/spec/system/place_spec.rb @@ -70,6 +70,7 @@ context "when visiting the start page" do before { visit "/passport-interview-office" } + it "renders the place page" do expect(page.status_code).to eq(200) @@ -168,7 +169,7 @@ end it "redirects to same page and not put postcode as URL query parameter" do - expect(page.current_path).to eq("/passport-interview-office") + expect(page).to have_current_path("/passport-interview-office", ignore_query: true) end it "does not display an error message" do @@ -323,7 +324,7 @@ it "reroutes to the base slug if requested with part route" do visit "/passport-interview-office/old-part-route" - expect(page.current_path).to eq("/passport-interview-office") + expect(page).to have_current_path("/passport-interview-office", ignore_query: true) end end diff --git a/spec/system/simple_smart_answers_spec.rb b/spec/system/simple_smart_answers_spec.rb index 7647e023b7..523f979555 100644 --- a/spec/system/simple_smart_answers_spec.rb +++ b/spec/system/simple_smart_answers_spec.rb @@ -108,7 +108,7 @@ it "reroutes to the base slug if requested with part route" do visit "/the-bridge-of-death/old-part-route" - expect(page.current_path).to eq("/the-bridge-of-death") + expect(page).to have_current_path("/the-bridge-of-death", ignore_query: true) end end @@ -116,7 +116,7 @@ visit "/the-bridge-of-death" click_on("Start now") - expect(page.current_path).to eq("/the-bridge-of-death/y") + expect(page).to have_current_path("/the-bridge-of-death/y", ignore_query: true) within("head", visible: :all) do expect(page).to have_selector("title", exact_text: "What...is your name? - The Bridge of Death - GOV.UK", visible: :all) @@ -173,7 +173,7 @@ choose("Sir Lancelot of Camelot") click_on("Next step") - expect(page.current_path).to eq("/the-bridge-of-death/y/sir-lancelot-of-camelot") + expect(page).to have_current_path("/the-bridge-of-death/y/sir-lancelot-of-camelot", ignore_query: true) within(".gem-c-heading + .govuk-body") do expect(page).to have_link("Start again", href: "/the-bridge-of-death") @@ -212,7 +212,7 @@ choose("Blue") click_on("Next step") - expect(page.current_path).to eq("/the-bridge-of-death/y/sir-lancelot-of-camelot/blue") + expect(page).to have_current_path("/the-bridge-of-death/y/sir-lancelot-of-camelot/blue", ignore_query: true) within(".gem-c-heading + .govuk-body") do expect(page).to have_link("Start again", href: "/the-bridge-of-death") @@ -266,7 +266,7 @@ click_on("Start now") - expect(page.current_path).to eq("/the-bridge-of-death/y") + expect(page).to have_current_path("/the-bridge-of-death/y", ignore_query: true) expect(page).not_to have_selector("[data-ga4-link='{\"event_name\":\"form_start\",\"type\":\"simple smart answer\",\"section\":\"start page\",\"action\":\"start\",\"tool_name\":\"The Bridge of Death\"}']") expect(page).to have_selector("[data-module='ga4-form-tracker']") expect(page).to have_selector("[data-ga4-form='{\"event_name\":\"form_response\",\"type\":\"simple smart answer\",\"section\":\"What...is your name?\",\"action\":\"next step\",\"tool_name\":\"The Bridge of Death\"}']") @@ -274,7 +274,7 @@ choose("Sir Lancelot of Camelot") click_on("Next step") - expect(page.current_path).to eq("/the-bridge-of-death/y/sir-lancelot-of-camelot") + expect(page).to have_current_path("/the-bridge-of-death/y/sir-lancelot-of-camelot", ignore_query: true) expect(page).not_to have_selector("[data-ga4-link='{\"event_name\":\"form_start\",\"type\":\"simple smart answer\",\"section\":\"start page\",\"action\":\"start\",\"tool_name\":\"The Bridge of Death\"}']") expect(page).to have_selector("[data-module='ga4-form-tracker']") expect(page).to have_selector("[data-ga4-form='{\"event_name\":\"form_response\",\"type\":\"simple smart answer\",\"section\":\"What...is your favorite colour?\",\"action\":\"next step\",\"tool_name\":\"The Bridge of Death\"}']") @@ -286,7 +286,7 @@ visit "/the-bridge-of-death" click_on("Start now") - expect(page.current_path).to eq("/the-bridge-of-death/y") + expect(page).to have_current_path("/the-bridge-of-death/y", ignore_query: true) click_on("Next step") @@ -298,19 +298,19 @@ visit "/the-bridge-of-death" click_on("Start now") - expect(page.current_path).to eq("/the-bridge-of-death/y") + expect(page).to have_current_path("/the-bridge-of-death/y", ignore_query: true) expect(page).not_to have_selector("[data-module='ga4-auto-tracker']") choose("Sir Lancelot of Camelot") click_on("Next step") - expect(page.current_path).to eq("/the-bridge-of-death/y/sir-lancelot-of-camelot") + expect(page).to have_current_path("/the-bridge-of-death/y/sir-lancelot-of-camelot", ignore_query: true) expect(page).not_to have_selector("[data-module='ga4-auto-tracker']") choose("Blue") click_on("Next step") - expect(page.current_path).to eq("/the-bridge-of-death/y/sir-lancelot-of-camelot/blue") + expect(page).to have_current_path("/the-bridge-of-death/y/sir-lancelot-of-camelot/blue", ignore_query: true) expect(page).to have_selector("[data-module='ga4-auto-tracker']") expect(page).to have_selector("[data-ga4-auto='{\"event_name\":\"form_complete\",\"type\":\"simple smart answer\",\"section\":\"Right, off you go.\",\"action\":\"complete\",\"tool_name\":\"The Bridge of Death\",\"text\":\"right-off-you-go\"}']") expect(page).to have_selector("[data-ga4-link='{\"event_name\":\"information_click\",\"type\":\"simple smart answer\",\"section\":\"Right, off you go.\",\"action\":\"information click\",\"tool_name\":\"The Bridge of Death\"}']") @@ -334,7 +334,7 @@ click_on("Change") end - expect(page.current_path).to eq("/the-bridge-of-death/y/sir-lancelot-of-camelot") + expect(page).to have_current_path("/the-bridge-of-death/y/sir-lancelot-of-camelot", ignore_query: true) within(".govuk-summary-list") do expect(page).to have_selector(".govuk-summary-list__row", count: 1) @@ -367,7 +367,7 @@ choose("Blue... NO! YELLOOOOOOOOOOOOOOOOWWW!!!!") click_on("Next step") - expect(page.current_path).to eq("/the-bridge-of-death/y/sir-lancelot-of-camelot/blue-no-yelloooooooooooooooowww") + expect(page).to have_current_path("/the-bridge-of-death/y/sir-lancelot-of-camelot/blue-no-yelloooooooooooooooowww", ignore_query: true) expect(page).to have_content("AAAAARRRRRRRRRRRRRRRRGGGGGHHH!!!!!!!") end @@ -402,7 +402,7 @@ choose("Blue") click_on("Next step") - expect(page.current_path).to eq("/the-bridge-of-death/y/sir-lancelot-of-camelot/blue") + expect(page).to have_current_path("/the-bridge-of-death/y/sir-lancelot-of-camelot/blue", ignore_query: true) expect(page).to have_selector(".govuk-summary-list .govuk-summary-list__row", count: 2) expect(page).to have_content("Right, off you go") end @@ -458,7 +458,7 @@ choose("Blue") click_on("Next step") - expect(page.current_path).to eq("/the-bridge-of-death/y/sir-lancelot-of-camelot/blue") + expect(page).to have_current_path("/the-bridge-of-death/y/sir-lancelot-of-camelot/blue", ignore_query: true) expect(page).to have_selector(".govuk-summary-list .govuk-summary-list__row", count: 2) expect(page).to have_content("Right, off you go") end @@ -493,7 +493,7 @@ choose("Blue") click_on("Next step") - expect(page.current_path).to eq("/the-bridge-of-death/y/sir-lancelot-of-camelot/blue") + expect(page).to have_current_path("/the-bridge-of-death/y/sir-lancelot-of-camelot/blue", ignore_query: true) expect(page).to have_selector(".govuk-summary-list .govuk-summary-list__row", count: 2) expect(page).to have_content("Right, off you go") end diff --git a/spec/system/take_part_spec.rb b/spec/system/take_part_spec.rb index 869f91f46c..2b03f9eb76 100644 --- a/spec/system/take_part_spec.rb +++ b/spec/system/take_part_spec.rb @@ -1,11 +1,11 @@ RSpec.describe "TakePart" do - it_behaves_like "it has meta tags", "take_part", "/government/get-involved/take-part/tp1" - it_behaves_like "it has meta tags for images", "take_part", "/government/get-involved/take-part/tp1" - before do content_store_has_example_item("/government/get-involved/take-part/tp1", schema: :take_part) end + it_behaves_like "it has meta tags", "take_part", "/government/get-involved/take-part/tp1" + it_behaves_like "it has meta tags for images", "take_part", "/government/get-involved/take-part/tp1" + context "when visiting a Take Part page" do it "displays the take_part page" do visit "/government/get-involved/take-part/tp1" diff --git a/spec/system/travel_advice_spec.rb b/spec/system/travel_advice_spec.rb index 16b1870534..c2cccd454a 100644 --- a/spec/system/travel_advice_spec.rb +++ b/spec/system/travel_advice_spec.rb @@ -134,7 +134,7 @@ expect(page).to have_css(".part-navigation-container nav li", count: parts_size) expect(page).to have_css(".part-navigation-container nav li", text: @content_store_response["details"]["parts"].first["title"]) - expect(page).to_not have_css(".part-navigation li a", text: @content_store_response["details"]["parts"].first["title"]) + expect(page).not_to have_css(".part-navigation li a", text: @content_store_response["details"]["parts"].first["title"]) @content_store_response["details"]["parts"][1..parts_size].each do |part| expect(page).to have_css(".part-navigation-container nav li a[href*=\"#{part['slug']}\"]", text: part["title"]) @@ -192,15 +192,15 @@ it "does not display a map" do visit "#{@base_path}/#{@part['slug']}" - expect(page).to_not have_css(".map") - expect(page).to_not have_css(".gem-c-metadata") + expect(page).not_to have_css(".map") + expect(page).not_to have_css(".gem-c-metadata") end it "does not display navigation links for the part" do visit "#{@base_path}/#{@part['slug']}" expect(page).to have_css(".part-navigation-container nav li", text: @part["title"]) - expect(page).to_not have_css(".part-navigation-container nav li a", text: @part["title"]) + expect(page).not_to have_css(".part-navigation-container nav li a", text: @part["title"]) end end @@ -213,7 +213,7 @@ it "does not render with the single page notification button" do visit @base_path - expect(page).to_not have_css(".gem-c-single-page-notification-button") + expect(page).not_to have_css(".gem-c-single-page-notification-button") end it "has GA4 tracking on the print link" do diff --git a/spec/unit/calendar_division_spec.rb b/spec/unit/calendar_division_spec.rb index b3530548a0..bdf2d6b073 100644 --- a/spec/unit/calendar_division_spec.rb +++ b/spec/unit/calendar_division_spec.rb @@ -183,6 +183,7 @@ describe "#show_bunting?" do before { @div = Calendar::Division.new("something") } + it "is true if there is a buntable bank holiday today" do @event = instance_double("Event", bunting: true, date: Time.zone.today) allow(@div).to receive(:upcoming_event).and_return(@event) @@ -207,6 +208,7 @@ describe "#as_json" do before { @div = Calendar::Division.new("something") } + it "returns division slug" do hash = @div.as_json expect(hash["division"]).to eq("something") diff --git a/spec/unit/calendar_spec.rb b/spec/unit/calendar_spec.rb index e6892fb101..9d0d4fded3 100644 --- a/spec/unit/calendar_spec.rb +++ b/spec/unit/calendar_spec.rb @@ -106,7 +106,7 @@ end end - context "#show_bunting?" do + describe "#show_bunting?" do before { @cal = Calendar.new("a-calendar") } it "is true when one division is buntable" do diff --git a/spec/unit/locales_validation_spec.rb b/spec/unit/locales_validation_spec.rb index 43c405519c..9293c00d33 100644 --- a/spec/unit/locales_validation_spec.rb +++ b/spec/unit/locales_validation_spec.rb @@ -1,6 +1,6 @@ RSpec.describe "Locales Validation" do # Stop the checker putting it's jaunty message to stdout - around(:example) do |ex| + around do |ex| original_stdout = $stdout $stdout = File.open(File::NULL, "w") ex.run diff --git a/spec/unit/presenters/content_item_model_presenter_spec.rb b/spec/unit/presenters/content_item_model_presenter_spec.rb index b280df1c1c..dca552e193 100644 --- a/spec/unit/presenters/content_item_model_presenter_spec.rb +++ b/spec/unit/presenters/content_item_model_presenter_spec.rb @@ -15,7 +15,7 @@ @content_store_response["withdrawn_notice"] = {} content_item = ContentItem.new(@content_store_response) - expect(described_class.new(content_item).page_title).to_not include("[Withdrawn]") + expect(described_class.new(content_item).page_title).not_to include("[Withdrawn]") expect(described_class.new(content_item).page_title).to include(content_item.title) end end diff --git a/spec/unit/presenters/travel_advice_presenter_spec.rb b/spec/unit/presenters/travel_advice_presenter_spec.rb index f50c6ec5e5..28256f68f1 100644 --- a/spec/unit/presenters/travel_advice_presenter_spec.rb +++ b/spec/unit/presenters/travel_advice_presenter_spec.rb @@ -16,7 +16,7 @@ part_slug = content_store_response.dig("details", "parts").first["slug"] content_item.set_current_part(part_slug) - expect(described_class.new(content_item).page_title).to_not include(content_item.current_part_title) + expect(described_class.new(content_item).page_title).not_to include(content_item.current_part_title) expect(described_class.new(content_item).page_title).to include(content_item.title) end diff --git a/spec/unit/services/csv_preview_service_spec.rb b/spec/unit/services/csv_preview_service_spec.rb index b211d002ce..593846c3dc 100644 --- a/spec/unit/services/csv_preview_service_spec.rb +++ b/spec/unit/services/csv_preview_service_spec.rb @@ -28,6 +28,7 @@ describe "#csv_rows" do context "with normal CSV" do subject { described_class.new(@csv).csv_rows } + it "parses the CSV correctly" do expect(subject.first).to eq(@parsed_csv) end From 3408d887884e3bcaefa4c6dbc6647171e79f5da9 Mon Sep 17 00:00:00 2001 From: Keith Lawrence Date: Wed, 11 Dec 2024 14:55:30 +0000 Subject: [PATCH 03/19] Run rubocop -A - This is the "include dangerous autocorrects" version, and indeed some of these changes break tests. I will fix in the following commits. --- spec/helpers/application_helper_spec.rb | 6 +- spec/helpers/block_helper_spec.rb | 2 +- spec/helpers/contents_list_helper_spec.rb | 2 +- spec/helpers/currency_helper_spec.rb | 2 +- spec/helpers/date_helper_spec.rb | 2 +- spec/helpers/error_items_helper_spec.rb | 2 +- spec/helpers/link_helper_spec.rb | 2 +- spec/helpers/locale_helper_spec.rb | 2 +- spec/helpers/parts_navigation_helper_spec.rb | 2 +- spec/helpers/phone_number_helper_spec.rb | 2 +- spec/helpers/theme_type_helper_spec.rb | 2 +- spec/models/get_involved_spec.rb | 8 +- spec/models/landing_page/block/base_spec.rb | 2 +- .../landing_page/block/featured_spec.rb | 2 +- spec/models/landing_page/block/hero_spec.rb | 4 +- .../block/main_navigation_spec.rb | 2 +- .../block/two_column_layout_spec.rb | 2 +- spec/requests/calendars_spec.rb | 8 +- spec/unit/calendar_division_spec.rb | 80 +++++++++---------- spec/unit/calendar_event_spec.rb | 6 +- spec/unit/calendar_spec.rb | 54 ++++++------- spec/unit/calendar_year_spec.rb | 18 ++--- spec/unit/ics_renderer_spec.rb | 14 ++-- spec/unit/postcode_sanitizer_spec.rb | 8 +- spec/unit/simple_smart_answers/flow_spec.rb | 4 +- spec/unit/simple_smart_answers/node_spec.rb | 14 ++-- 26 files changed, 126 insertions(+), 126 deletions(-) diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index 94296db289..44452450f1 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -54,7 +54,7 @@ def dummy_publication let(:content_item) { ContentItem.new({ "schema_name" => "landing_page" }) } it "removes breadcrumbs" do - expect(remove_breadcrumbs(content_item)).to eq(true) + expect(remove_breadcrumbs(content_item)).to be(true) end end @@ -62,7 +62,7 @@ def dummy_publication let(:content_item) { ContentItem.new({ "schema_name" => "a_different_page" }) } it "does not remove breadcrumbs" do - expect(remove_breadcrumbs(content_item)).to eq(false) + expect(remove_breadcrumbs(content_item)).to be(false) end end @@ -70,7 +70,7 @@ def dummy_publication let(:content_item) { ContentItem.new({}) } it "does not remove breadcrumbs" do - expect(remove_breadcrumbs(content_item)).to eq(false) + expect(remove_breadcrumbs(content_item)).to be(false) end end end diff --git a/spec/helpers/block_helper_spec.rb b/spec/helpers/block_helper_spec.rb index 3725e01d68..134e27ecc4 100644 --- a/spec/helpers/block_helper_spec.rb +++ b/spec/helpers/block_helper_spec.rb @@ -1,5 +1,5 @@ RSpec.describe BlockHelper do - include BlockHelper + include described_class describe "#render_block" do it "returns an empty string when a partial template doesn't exist" do diff --git a/spec/helpers/contents_list_helper_spec.rb b/spec/helpers/contents_list_helper_spec.rb index a355a8fa83..23b81590cc 100644 --- a/spec/helpers/contents_list_helper_spec.rb +++ b/spec/helpers/contents_list_helper_spec.rb @@ -1,5 +1,5 @@ RSpec.describe ContentsListHelper do - include ContentsListHelper + include described_class describe "#contents_list" do let(:current_path) { "/active-page" } diff --git a/spec/helpers/currency_helper_spec.rb b/spec/helpers/currency_helper_spec.rb index cd678ec4e7..a03c1c3b00 100644 --- a/spec/helpers/currency_helper_spec.rb +++ b/spec/helpers/currency_helper_spec.rb @@ -1,5 +1,5 @@ RSpec.describe CurrencyHelper do - include CurrencyHelper + include described_class let(:sample_number) { "12000" } diff --git a/spec/helpers/date_helper_spec.rb b/spec/helpers/date_helper_spec.rb index 7a95fcf27e..97dd03697e 100644 --- a/spec/helpers/date_helper_spec.rb +++ b/spec/helpers/date_helper_spec.rb @@ -1,5 +1,5 @@ RSpec.describe DateHelper do - include DateHelper + include described_class let(:timestamp) { "2024-10-03 19:30:22 +0100" } diff --git a/spec/helpers/error_items_helper_spec.rb b/spec/helpers/error_items_helper_spec.rb index 8528602f1c..07bc5afe96 100644 --- a/spec/helpers/error_items_helper_spec.rb +++ b/spec/helpers/error_items_helper_spec.rb @@ -1,5 +1,5 @@ RSpec.describe ErrorItemsHelper do - include ErrorItemsHelper + include described_class before do flash[:validation] = [ diff --git a/spec/helpers/link_helper_spec.rb b/spec/helpers/link_helper_spec.rb index a1c84078b6..b72df390b3 100644 --- a/spec/helpers/link_helper_spec.rb +++ b/spec/helpers/link_helper_spec.rb @@ -1,5 +1,5 @@ RSpec.describe LinkHelper do - include LinkHelper + include described_class describe "#feed_link" do it "appends .atom to the base_path" do diff --git a/spec/helpers/locale_helper_spec.rb b/spec/helpers/locale_helper_spec.rb index 36593b5140..2c4d35ffc2 100644 --- a/spec/helpers/locale_helper_spec.rb +++ b/spec/helpers/locale_helper_spec.rb @@ -1,5 +1,5 @@ RSpec.describe LocaleHelper do - include LocaleHelper + include described_class describe "#translations_for_nav" do it "returns translations in a suitable format for the translation nav component" do diff --git a/spec/helpers/parts_navigation_helper_spec.rb b/spec/helpers/parts_navigation_helper_spec.rb index 80ce8b15b5..a01742b9a0 100644 --- a/spec/helpers/parts_navigation_helper_spec.rb +++ b/spec/helpers/parts_navigation_helper_spec.rb @@ -1,5 +1,5 @@ RSpec.describe PartsNavigationHelper do - include PartsNavigationHelper + include described_class let(:part_one) do { diff --git a/spec/helpers/phone_number_helper_spec.rb b/spec/helpers/phone_number_helper_spec.rb index f506e813d4..c2c70d78c0 100644 --- a/spec/helpers/phone_number_helper_spec.rb +++ b/spec/helpers/phone_number_helper_spec.rb @@ -1,5 +1,5 @@ RSpec.describe PhoneNumberHelper do - include PhoneNumberHelper + include described_class let(:sample_phone_text) { "023 4567 8910 (with some text)" } diff --git a/spec/helpers/theme_type_helper_spec.rb b/spec/helpers/theme_type_helper_spec.rb index fee1692c71..e60295911f 100644 --- a/spec/helpers/theme_type_helper_spec.rb +++ b/spec/helpers/theme_type_helper_spec.rb @@ -1,5 +1,5 @@ RSpec.describe ThemeTypeHelper do - include ThemeTypeHelper + include described_class describe "no theme type" do it "returns theme type default when style is empty" do diff --git a/spec/models/get_involved_spec.rb b/spec/models/get_involved_spec.rb index 95d0a22d15..b0404d6364 100644 --- a/spec/models/get_involved_spec.rb +++ b/spec/models/get_involved_spec.rb @@ -12,7 +12,7 @@ describe "#open_consultation_count" do it "returns the total number of open consultations" do - model_instance = GetInvolved.new(content_item) + model_instance = described_class.new(content_item) expect(model_instance.open_consultation_count).to eq(83) end @@ -20,7 +20,7 @@ describe "#closed_consultation_count" do it "returns the total number of closed consultations" do - model_instance = GetInvolved.new(content_item) + model_instance = described_class.new(content_item) expect(model_instance.closed_consultation_count).to eq(110) end @@ -28,7 +28,7 @@ describe "#consultation outcome" do it "returns the recent consultation outcome" do - model_instance = GetInvolved.new(content_item) + model_instance = described_class.new(content_item) expect(model_instance.recent_consultation_outcomes).to eq([consultation_result]) end @@ -36,7 +36,7 @@ describe "#next closing consultation" do it "returns the next closing consultation" do - model_instance = GetInvolved.new(content_item) + model_instance = described_class.new(content_item) expect(model_instance.next_closing_consultation).to eq(next_closing_consultation) end diff --git a/spec/models/landing_page/block/base_spec.rb b/spec/models/landing_page/block/base_spec.rb index 67a2359444..d93fecd7f1 100644 --- a/spec/models/landing_page/block/base_spec.rb +++ b/spec/models/landing_page/block/base_spec.rb @@ -1,7 +1,7 @@ RSpec.describe LandingPage::Block::Base do describe "#full_width?" do it "is false by default" do - expect(described_class.new({}, build(:landing_page)).full_width?).to eq(false) + expect(described_class.new({}, build(:landing_page)).full_width?).to be(false) end end end diff --git a/spec/models/landing_page/block/featured_spec.rb b/spec/models/landing_page/block/featured_spec.rb index cd22e59076..33dc6154ba 100644 --- a/spec/models/landing_page/block/featured_spec.rb +++ b/spec/models/landing_page/block/featured_spec.rb @@ -45,7 +45,7 @@ describe "#full_width?" do it "is false" do - expect(subject.full_width?).to eq(false) + expect(subject.full_width?).to be(false) end end end diff --git a/spec/models/landing_page/block/hero_spec.rb b/spec/models/landing_page/block/hero_spec.rb index 549d1ab814..840e3c399a 100644 --- a/spec/models/landing_page/block/hero_spec.rb +++ b/spec/models/landing_page/block/hero_spec.rb @@ -77,7 +77,7 @@ describe "#theme_colour" do it "defaults to nil" do - expect(subject.theme_colour).to eq(nil) + expect(subject.theme_colour).to be_nil end it "returns the theme_colour from config" do @@ -88,7 +88,7 @@ describe "#full_width?" do it "is true" do - expect(subject.full_width?).to eq(true) + expect(subject.full_width?).to be(true) end end end diff --git a/spec/models/landing_page/block/main_navigation_spec.rb b/spec/models/landing_page/block/main_navigation_spec.rb index a081a1ceab..62f77deb4a 100644 --- a/spec/models/landing_page/block/main_navigation_spec.rb +++ b/spec/models/landing_page/block/main_navigation_spec.rb @@ -39,7 +39,7 @@ describe "#full_width?" do it "is true" do - expect(subject.full_width?).to eq(true) + expect(subject.full_width?).to be(true) end end diff --git a/spec/models/landing_page/block/two_column_layout_spec.rb b/spec/models/landing_page/block/two_column_layout_spec.rb index d96d9095b6..e3f9c6580f 100644 --- a/spec/models/landing_page/block/two_column_layout_spec.rb +++ b/spec/models/landing_page/block/two_column_layout_spec.rb @@ -24,7 +24,7 @@ it "returns offset column when the theme is two_thirds_right" do blocks_hash["theme"] = "two_thirds_right" expect(subject.left_column_class).to eq "govuk-grid-column-one-third grid-column-one-third-offset" - expect(subject.left).to eq nil + expect(subject.left).to be_nil end end diff --git a/spec/requests/calendars_spec.rb b/spec/requests/calendars_spec.rb index 88dc7400b9..2e07e759dd 100644 --- a/spec/requests/calendars_spec.rb +++ b/spec/requests/calendars_spec.rb @@ -48,7 +48,7 @@ context "json request" do it "loads the calendar and return its json representation" do - expect(Calendar).to receive(:find).with("bank-holidays").and_return(instance_double("Calendar", to_json: "json_calendar")) + expect(Calendar).to receive(:find).with("bank-holidays").and_return(instance_double(Calendar, to_json: "json_calendar")) get "/bank-holidays.json" expect(response.body).to eq("json_calendar") @@ -85,8 +85,8 @@ context "GET 'division'" do before do - @division = instance_double("Division", to_json: "", events: []) - @calendar = instance_double("Calendar", division: @division) + @division = instance_double(Division, to_json: "", events: []) + @calendar = instance_double(Calendar, division: @division) allow(Calendar).to receive(:find).and_return(@calendar) stub_content_store_has_item("/a-calendar", { schema_name: "calendar", @@ -109,7 +109,7 @@ expect(@division).to receive(:events).and_return(:some_events) expect(@calendar).to receive(:division).with("a-division").and_return(@division) allow(Calendar).to receive(:find).with("a-calendar").and_return(@calendar) - expect(IcsRenderer).to receive(:new).with(:some_events, "/a-calendar/a-division.ics", :en).and_return(instance_double("Renderer", render: "ics_division")) + expect(IcsRenderer).to receive(:new).with(:some_events, "/a-calendar/a-division.ics", :en).and_return(instance_double(Renderer, render: "ics_division")) get "/a-calendar/a-division.ics" expect(response.body).to eq("ics_division") diff --git a/spec/unit/calendar_division_spec.rb b/spec/unit/calendar_division_spec.rb index bdf2d6b073..358c9c13a6 100644 --- a/spec/unit/calendar_division_spec.rb +++ b/spec/unit/calendar_division_spec.rb @@ -1,21 +1,21 @@ RSpec.describe Calendar::Division do it "returns the slug" do - expect(Calendar::Division.new("a-slug", {}).slug).to eq("a-slug") + expect(described_class.new("a-slug", {}).slug).to eq("a-slug") end it "returns the slug for to_param" do - expect(Calendar::Division.new("a-slug", {}).to_param).to eq("a-slug") + expect(described_class.new("a-slug", {}).to_param).to eq("a-slug") end describe "#title" do it "returns the title from the data if given" do - d = Calendar::Division.new("a-slug", "title" => "something") + d = described_class.new("a-slug", "title" => "something") expect(d.title).to eq("something") end it "humanizes the slug otherwise" do - d = Calendar::Division.new("a-slug", {}) + d = described_class.new("a-slug", {}) expect(d.title).to eq("A slug") end @@ -23,7 +23,7 @@ describe "#years" do it "construct a year for each one in the data" do - div = Calendar::Division.new("something", "2012" => [1, 2], "2013" => [3, 4]) + div = described_class.new("something", "2012" => [1, 2], "2013" => [3, 4]) expect(Calendar::Year).to receive(:new).with("2012", div, [1, 2]).and_return(:y_2012) expect(Calendar::Year).to receive(:new).with("2013", div, [3, 4]).and_return(:y_2013) @@ -31,7 +31,7 @@ end it "caches the constructed instances" do - div = Calendar::Division.new("something", "2012" => [1, 2], "2013" => [3, 4]) + div = described_class.new("something", "2012" => [1, 2], "2013" => [3, 4]) first = div.years expect(Calendar::Year).not_to receive(:new) @@ -39,7 +39,7 @@ end it "ignores non-year keys in the data" do - div = Calendar::Division.new("something", "title" => "A Thing", "2012" => [1, 2], "2013" => [3, 4], "foo" => "bar") + div = described_class.new("something", "title" => "A Thing", "2012" => [1, 2], "2013" => [3, 4], "foo" => "bar") allow(Calendar::Year).to receive(:new).with("2012", div, [1, 2]).and_return(:y_2012) allow(Calendar::Year).to receive(:new).with("2013", div, [3, 4]).and_return(:y_2013) @@ -50,7 +50,7 @@ context "finding a year by name" do before do - @div = Calendar::Division.new("something", "title" => "A Division", "2012" => [1, 2], "2013" => [3, 4]) + @div = described_class.new("something", "title" => "A Division", "2012" => [1, 2], "2013" => [3, 4]) end it "returns the year with the matching name" do @@ -69,22 +69,22 @@ describe "#events" do before do @years = [] - @div = Calendar::Division.new("something") + @div = described_class.new("something") allow(@div).to receive(:years).and_return(@years) end it "merges events for all years into single array" do - @years << instance_double("Year1", events: [1, 2]) - @years << instance_double("Year2", events: [3, 4, 5]) - @years << instance_double("Year3", events: [6, 7]) + @years << instance_double(Year1, events: [1, 2]) + @years << instance_double(Year2, events: [3, 4, 5]) + @years << instance_double(Year3, events: [6, 7]) expect(@div.events).to eq([1, 2, 3, 4, 5, 6, 7]) end it "handles years with no events" do - @years << instance_double("Year1", events: [1, 2]) - @years << instance_double("Year2", events: []) - @years << instance_double("Year3", events: [6, 7]) + @years << instance_double(Year1, events: [1, 2]) + @years << instance_double(Year2, events: []) + @years << instance_double(Year3, events: [6, 7]) expect(@div.events).to eq([1, 2, 6, 7]) end @@ -93,7 +93,7 @@ describe "#upcoming_event" do before do @years = [] - @div = Calendar::Division.new("something") + @div = described_class.new("something") allow(@div).to receive(:years).and_return(@years) end @@ -102,23 +102,23 @@ end it "returns nil if no years have upcoming_events" do - @years << instance_double("Year", upcoming_event: nil) - @years << instance_double("Year", upcoming_event: nil) + @years << instance_double(Year, upcoming_event: nil) + @years << instance_double(Year, upcoming_event: nil) expect(@div.upcoming_event).to be_nil end it "returns the upcoming event for the first year that has one" do - @years << instance_double("Year", upcoming_event: nil) - @years << instance_double("Year", upcoming_event: :event_1) - @years << instance_double("Year", upcoming_event: :event_2) + @years << instance_double(Year, upcoming_event: nil) + @years << instance_double(Year, upcoming_event: :event_1) + @years << instance_double(Year, upcoming_event: :event_2) expect(@div.upcoming_event).to eq(:event_1) end it "caches the event" do - y1 = instance_double("Year") - y2 = instance_double("Year", upcoming_event: :event_1) + y1 = instance_double(Year) + y2 = instance_double(Year, upcoming_event: :event_1) @years << y1 @years << y2 @@ -132,21 +132,21 @@ describe "#upcoming_events_by_year" do before do @years = [] - @div = Calendar::Division.new("something") + @div = described_class.new("something") allow(@div).to receive(:years).and_return(@years) end it "returns a hash of year => events for upcoming events" do - y1 = instance_double("Year1", upcoming_events: %i[e1 e2]) - y2 = instance_double("Year2", upcoming_events: %i[e3 e4 e5]) + y1 = instance_double(Year1, upcoming_events: %i[e1 e2]) + y2 = instance_double(Year2, upcoming_events: %i[e3 e4 e5]) ((@years << y1) << y2) expected = { y1 => %i[e1 e2], y2 => %i[e3 e4 e5] } expect(@div.upcoming_events_by_year).to eq(expected) end it "does not include any years with no upcoming events" do - y1 = instance_double("Year1", upcoming_events: []) - y2 = instance_double("Year2", upcoming_events: %i[e1 e2 e3]) + y1 = instance_double(Year1, upcoming_events: []) + y2 = instance_double(Year2, upcoming_events: %i[e1 e2 e3]) ((@years << y1) << y2) expected = { y2 => %i[e1 e2 e3] } expect(@div.upcoming_events_by_year).to eq(expected) @@ -156,13 +156,13 @@ describe "#past_events_by_year" do before do @years = [] - @div = Calendar::Division.new("something") + @div = described_class.new("something") allow(@div).to receive(:years).and_return(@years) end it "returns a hash of year => reversed events for past events" do - y1 = instance_double("Year", past_events: %i[e1 e2]) - y2 = instance_double("Year", past_events: %i[e3 e4 e5]) + y1 = instance_double(Year, past_events: %i[e1 e2]) + y2 = instance_double(Year, past_events: %i[e3 e4 e5]) ((@years << y1) << y2) expected = { y1 => %i[e2 e1], y2 => %i[e5 e4 e3] } events_by_year = @div.past_events_by_year @@ -172,8 +172,8 @@ end it "does not include any years with no past events" do - y1 = instance_double("Year", past_events: %i[e1 e2]) - y2 = instance_double("Year", past_events: []) + y1 = instance_double(Year, past_events: %i[e1 e2]) + y2 = instance_double(Year, past_events: []) ((@years << y1) << y2) expected = { y1 => %i[e2 e1] } @@ -182,24 +182,24 @@ end describe "#show_bunting?" do - before { @div = Calendar::Division.new("something") } + before { @div = described_class.new("something") } it "is true if there is a buntable bank holiday today" do - @event = instance_double("Event", bunting: true, date: Time.zone.today) + @event = instance_double(Event, bunting: true, date: Time.zone.today) allow(@div).to receive(:upcoming_event).and_return(@event) expect(@div.show_bunting?).to be true end it "is false if there is a non-buntable bank holiday today" do - @event = instance_double("Event", bunting: false, date: Time.zone.today) + @event = instance_double(Event, bunting: false, date: Time.zone.today) allow(@div).to receive(:upcoming_event).and_return(@event) expect(@div.show_bunting?).to be false end it "is false if there is no bank holiday today" do - @event = instance_double("Event", bunting: true, date: (Time.zone.today + 1.week)) + @event = instance_double(Event, bunting: true, date: (Time.zone.today + 1.week)) allow(@div).to receive(:upcoming_event).and_return(@event) expect(@div.show_bunting?).to be false @@ -207,7 +207,7 @@ end describe "#as_json" do - before { @div = Calendar::Division.new("something") } + before { @div = described_class.new("something") } it "returns division slug" do hash = @div.as_json @@ -215,8 +215,8 @@ end it "returns all events from all years" do - y1 = instance_double("Year", events: [1, 2]) - y2 = instance_double("Year", events: [3, 4]) + y1 = instance_double(Year, events: [1, 2]) + y2 = instance_double(Year, events: [3, 4]) allow(@div).to receive(:years).and_return([y1, y2]) hash = @div.as_json diff --git a/spec/unit/calendar_event_spec.rb b/spec/unit/calendar_event_spec.rb index 332b50cf31..1bdf9f0829 100644 --- a/spec/unit/calendar_event_spec.rb +++ b/spec/unit/calendar_event_spec.rb @@ -1,13 +1,13 @@ RSpec.describe Calendar::Event do context "construction" do it "parses a date given as a string" do - e = Calendar::Event.new("date" => "2012-02-04") + e = described_class.new("date" => "2012-02-04") expect(e.date).to eq(Date.civil(2012, 2, 4)) end it "allows construction with dates as well as string dates" do - e = Calendar::Event.new("date" => Date.civil(2012, 2, 4)) + e = described_class.new("date" => Date.civil(2012, 2, 4)) expect(e.date).to eq(Date.civil(2012, 2, 4)) end @@ -16,7 +16,7 @@ context "as_json in English" do it "returns a hash representation" do I18n.locale = :en - e = Calendar::Event.new("title" => "bank_holidays.new_year", "date" => "02/01/2012", "notes" => "common.substitute_day", "bunting" => true) + e = described_class.new("title" => "bank_holidays.new_year", "date" => "02/01/2012", "notes" => "common.substitute_day", "bunting" => true) expected = { "title" => "New Year\u2019s Day", "date" => Date.civil(2012, 1, 2), "notes" => "Substitute day", "bunting" => true } expect(e.as_json).to eq(expected) diff --git a/spec/unit/calendar_spec.rb b/spec/unit/calendar_spec.rb index 9d0d4fded3..020ff93ecf 100644 --- a/spec/unit/calendar_spec.rb +++ b/spec/unit/calendar_spec.rb @@ -8,28 +8,28 @@ context "finding a calendar by slug" do it "constructs a calendar with the slug and data from the corresponding JSON file" do data_from_json = JSON.parse(File.read(Rails.root.join(Calendar::REPOSITORY_PATH, "single-calendar.json"))) - expect(Calendar).to receive(:new).with("single-calendar", data_from_json).and_return(:a_calendar) - cal = Calendar.find("single-calendar") + expect(described_class).to receive(:new).with("single-calendar", data_from_json).and_return(:a_calendar) + cal = described_class.find("single-calendar") expect(cal).to eq(:a_calendar) end it "raises exception when calendar doesn't exist" do - expect { Calendar.find("non-existent") }.to raise_error(Calendar::CalendarNotFound) + expect { described_class.find("non-existent") }.to raise_error(Calendar::CalendarNotFound) end end it "returns the slug" do - expect(Calendar.new("a-slug", {}).slug).to eq("a-slug") + expect(described_class.new("a-slug", {}).slug).to eq("a-slug") end it "returns the slug for to_param" do - expect(Calendar.new("a-slug", {}).to_param).to eq("a-slug") + expect(described_class.new("a-slug", {}).to_param).to eq("a-slug") end describe "#divisions" do before do - @cal = Calendar.new( + @cal = described_class.new( "a-calendar", "title" => "UK bank holidays", "divisions" => { @@ -71,22 +71,22 @@ describe "#events" do before do @divisions = [] - @calendar = Calendar.new("a-calendar") + @calendar = described_class.new("a-calendar") allow(@calendar).to receive(:divisions).and_return(@divisions) end it "merges events for all years into single array" do - @divisions << instance_double("Division", events: [1, 2]) - @divisions << instance_double("Division", events: [3, 4, 5]) - @divisions << instance_double("Division", events: [6, 7]) + @divisions << instance_double(Division, events: [1, 2]) + @divisions << instance_double(Division, events: [3, 4, 5]) + @divisions << instance_double(Division, events: [6, 7]) expect(@calendar.events).to eq([1, 2, 3, 4, 5, 6, 7]) end it "handles years with no events" do - @divisions << instance_double("Division1", events: [1, 2]) - @divisions << instance_double("Division2", events: []) - @divisions << instance_double("Division3", events: [6, 7]) + @divisions << instance_double(Division1, events: [1, 2]) + @divisions << instance_double(Division2, events: []) + @divisions << instance_double(Division3, events: [6, 7]) expect(@calendar.events).to eq([1, 2, 6, 7]) end @@ -94,7 +94,7 @@ context "attribute accessors" do before do - @cal = Calendar.new("a-calendar", "title" => "bank_holidays.calendar.title", "description" => "bank_holidays.calendar.description") + @cal = described_class.new("a-calendar", "title" => "bank_holidays.calendar.title", "description" => "bank_holidays.calendar.description") end it "has an accessor for the title" do @@ -107,30 +107,30 @@ end describe "#show_bunting?" do - before { @cal = Calendar.new("a-calendar") } + before { @cal = described_class.new("a-calendar") } it "is true when one division is buntable" do - @div1 = instance_double("Division", show_bunting?: true) - @div2 = instance_double("Division", show_bunting?: false) - @div3 = instance_double("Division", show_bunting?: false) + @div1 = instance_double(Division, show_bunting?: true) + @div2 = instance_double(Division, show_bunting?: false) + @div3 = instance_double(Division, show_bunting?: false) allow(@cal).to receive(:divisions).and_return([@div1, @div2, @div3]) expect(@cal.show_bunting?).to be true end it "is true when more than one division is buntable" do - @div1 = instance_double("Division", show_bunting?: true) - @div2 = instance_double("Division", show_bunting?: true) - @div3 = instance_double("Division", show_bunting?: false) + @div1 = instance_double(Division, show_bunting?: true) + @div2 = instance_double(Division, show_bunting?: true) + @div3 = instance_double(Division, show_bunting?: false) allow(@cal).to receive(:divisions).and_return([@div1, @div2, @div3]) expect(@cal.show_bunting?).to be true end it "is false when no divisions are buntable" do - @div1 = instance_double("Division", show_bunting?: false) - @div2 = instance_double("Division", show_bunting?: false) - @div3 = instance_double("Division", show_bunting?: false) + @div1 = instance_double(Division, show_bunting?: false) + @div2 = instance_double(Division, show_bunting?: false) + @div3 = instance_double(Division, show_bunting?: false) allow(@cal).to receive(:divisions).and_return([@div1, @div2, @div3]) expect(@cal.show_bunting?).to be false @@ -139,9 +139,9 @@ describe "#as_json" do before do - @div1 = instance_double("Division", slug: "division-1", as_json: "div1 json") - @div2 = instance_double("Division", slug: "division-2", as_json: "div2 json") - @cal = Calendar.new("a-calendar") + @div1 = instance_double(Division, slug: "division-1", as_json: "div1 json") + @div2 = instance_double(Division, slug: "division-2", as_json: "div2 json") + @cal = described_class.new("a-calendar") allow(@cal).to receive(:divisions).and_return([@div1, @div2]) end diff --git a/spec/unit/calendar_year_spec.rb b/spec/unit/calendar_year_spec.rb index 0d7b1dfa9f..1497b17f69 100644 --- a/spec/unit/calendar_year_spec.rb +++ b/spec/unit/calendar_year_spec.rb @@ -1,11 +1,11 @@ RSpec.describe Calendar::Year do it "returns the year string for to_s" do - expect(Calendar::Year.new("2012", :a_division, []).to_s).to eq("2012") + expect(described_class.new("2012", :a_division, []).to_s).to eq("2012") end describe "#events" do before do - @y = Calendar::Year.new("1234", :a_division, [ + @y = described_class.new("1234", :a_division, [ { "title" => "bank_holidays.new_year", "date" => "02/01/2012" }, { "title" => "bank_holidays.good_friday", "date" => "27/08/2012" }, ]) @@ -28,13 +28,13 @@ describe "#upcoming_event" do it "return nil with no events" do - y = Calendar::Year.new("1234", :a_division, []) + y = described_class.new("1234", :a_division, []) expect(y.upcoming_event).to be_nil end it "returns nil with no future events" do - y = Calendar::Year.new("1234", :a_division, [ + y = described_class.new("1234", :a_division, [ { "title" => "bank_holidays.new_year", "date" => "02/01/2012" }, { "title" => "bank_holidays.good_friday", "date" => "27/08/2012" }, ]) @@ -43,7 +43,7 @@ end it "returns the first event that's in the future" do - y = Calendar::Year.new("1234", :a_division, [ + y = described_class.new("1234", :a_division, [ { "title" => "bank_holidays.new_year", "date" => "02/01/2012" }, { "title" => "bank_holidays.good_friday", "date" => "27/08/2012" }, { "title" => "bank_holidays.easter_monday", "date" => "16/10/2012" }, @@ -54,7 +54,7 @@ end it "counts an event today as a future event" do - y = Calendar::Year.new("1234", :a_division, [ + y = described_class.new("1234", :a_division, [ { "title" => "bank_holidays.new_year", "date" => "02/01/2012" }, { "title" => "bank_holidays.good_friday", "date" => "27/08/2012" }, { "title" => "bank_holidays.easter_monday", "date" => "16/10/2012" }, @@ -65,7 +65,7 @@ end it "caches the event" do - y = Calendar::Year.new("1234", :a_division, [ + y = described_class.new("1234", :a_division, [ { "title" => "bank_holidays.new_year", "date" => "02/01/2012" }, { "title" => "bank_holidays.good_friday", "date" => "27/08/2012" }, { "title" => "bank_holidays.easter_monday", "date" => "16/10/2012" }, @@ -82,7 +82,7 @@ describe "#upcoming_events" do before do - @year = Calendar::Year.new("1234", :a_division, [ + @year = described_class.new("1234", :a_division, [ { "title" => "bank_holidays.new_year", "date" => "02/01/2012" }, { "title" => "bank_holidays.good_friday", "date" => "27/08/2012" }, { "title" => "bank_holidays.easter_monday", "date" => "16/10/2012" }, @@ -106,7 +106,7 @@ describe "#past_events" do before do - @year = Calendar::Year.new("1234", :a_division, [ + @year = described_class.new("1234", :a_division, [ { "title" => "bank_holidays.new_year", "date" => "02/01/2012" }, { "title" => "bank_holidays.good_friday", "date" => "27/08/2012" }, { "title" => "bank_holidays.easter_monday", "date" => "16/10/2012" }, diff --git a/spec/unit/ics_renderer_spec.rb b/spec/unit/ics_renderer_spec.rb index 726573af5b..c060798c1f 100644 --- a/spec/unit/ics_renderer_spec.rb +++ b/spec/unit/ics_renderer_spec.rb @@ -3,7 +3,7 @@ RSpec.describe IcsRenderer do context "generating complete ics file" do it "generates correct ics header and footer" do - r = IcsRenderer.new([], "/foo/ics", :en) + r = described_class.new([], "/foo/ics", :en) expected = "BEGIN:VCALENDAR\r\n" (expected << "VERSION:2.0\r\n") @@ -15,13 +15,13 @@ end it "handles locale correctly" do - r = IcsRenderer.new([], "/foo/ics", :cy) + r = described_class.new([], "/foo/ics", :cy) expect(r.render).to match("PRODID:-//uk.gov/GOVUK calendars//CY") end it "generates an event for each given event" do - r = IcsRenderer.new(%i[e1 e2], "/foo/ics", :en) + r = described_class.new(%i[e1 e2], "/foo/ics", :en) expect(r).to receive(:render_event).with(:e1).and_return("Event1 ics\r\n") expect(r).to receive(:render_event).with(:e2).and_return("Event2 ics\r\n") @@ -40,8 +40,8 @@ context "generating an event" do before do @path = "/foo/ics" - @r = IcsRenderer.new([], @path, :en) - allow_any_instance_of(IcsRenderer).to receive(:dtstamp).and_return("20121017T0100Z") + @r = described_class.new([], @path, :en) + allow_any_instance_of(described_class).to receive(:dtstamp).and_return("20121017T0100Z") end it "generates an event" do @@ -63,7 +63,7 @@ context "generating a uid" do before do @path = "/foo/bar.ics" - @r = IcsRenderer.new([], @path, :en) + @r = described_class.new([], @path, :en) @hash = Digest::MD5.hexdigest(@path) @first_event = Calendar::Event.new("title" => "bank_holidays.new_year", "date" => Date.new(1982, 5, 28)) @second_event = Calendar::Event.new("title" => "bank_holidays.good_friday", "date" => Date.new(1984, 1, 16)) @@ -81,7 +81,7 @@ end context "generating dtstamp" do - before { @r = IcsRenderer.new([], "/foo/ics", :en) } + before { @r = described_class.new([], "/foo/ics", :en) } it "returns the mtime of the REVISION file" do expect(File).to receive(:mtime).with(Rails.root.join("REVISION")).and_return(Time.zone.parse("2012-04-06 14:53:54Z")) diff --git a/spec/unit/postcode_sanitizer_spec.rb b/spec/unit/postcode_sanitizer_spec.rb index 4b085cd4c8..f4d59c2359 100644 --- a/spec/unit/postcode_sanitizer_spec.rb +++ b/spec/unit/postcode_sanitizer_spec.rb @@ -1,21 +1,21 @@ RSpec.describe PostcodeSanitizer do context "postcodes are sanitized before being sent to LocationsApi" do it "strips trailing spaces from entered postcodes" do - expect(PostcodeSanitizer.sanitize("WC2B 6NH ")).to eq("WC2B 6NH") + expect(described_class.sanitize("WC2B 6NH ")).to eq("WC2B 6NH") end it "strips non-alphanumerics from entered postcodes" do - expect(PostcodeSanitizer.sanitize("WC2B -6NH]")).to eq("WC2B 6NH") + expect(described_class.sanitize("WC2B -6NH]")).to eq("WC2B 6NH") end it "transposes O/0 and I/1 if necessary" do - expect(PostcodeSanitizer.sanitize("WIA OAA")).to eq("W1A 0AA") + expect(described_class.sanitize("WIA OAA")).to eq("W1A 0AA") end end context "if the postcode parameter is nil or an empty string" do it "returns nil and does not try to perform sanitization" do - expect(PostcodeSanitizer.sanitize("")).to be_nil + expect(described_class.sanitize("")).to be_nil end end end diff --git a/spec/unit/simple_smart_answers/flow_spec.rb b/spec/unit/simple_smart_answers/flow_spec.rb index 187049cb1a..6c277b8879 100644 --- a/spec/unit/simple_smart_answers/flow_spec.rb +++ b/spec/unit/simple_smart_answers/flow_spec.rb @@ -30,7 +30,7 @@ module SimpleSmartAnswers "body" => "

This is outcome 1

", }, ] - @flow = Flow.new(@nodes) + @flow = described_class.new(@nodes) end it "returns the node matching the slug" do @@ -55,7 +55,7 @@ module SimpleSmartAnswers describe "#state_for_responses" do before do @flow = - Flow.new( + described_class.new( [ { "kind" => "question", diff --git a/spec/unit/simple_smart_answers/node_spec.rb b/spec/unit/simple_smart_answers/node_spec.rb index a4c5160b7c..bc652b96c1 100644 --- a/spec/unit/simple_smart_answers/node_spec.rb +++ b/spec/unit/simple_smart_answers/node_spec.rb @@ -2,7 +2,7 @@ module SimpleSmartAnswers RSpec.describe Node do it "has attribute accessors for basic fields" do node = - Node.new( + described_class.new( :a_flow, "kind" => "question", "slug" => "question-1", @@ -18,19 +18,19 @@ module SimpleSmartAnswers describe "#outcome?" do it "is true for an outcome node" do - expect(Node.new(:a_flow, "kind" => "outcome").outcome?).to be true + expect(described_class.new(:a_flow, "kind" => "outcome").outcome?).to be true end it "is false for a question node" do - expect(Node.new(:a_flow, "kind" => "question").outcome?).to be false + expect(described_class.new(:a_flow, "kind" => "question").outcome?).to be false end end describe "#options" do before do - @flow = instance_double("Flow", node_for_slug: :a_node) + @flow = instance_double(Flow, node_for_slug: :a_node) @node = - Node.new( + described_class.new( @flow, "kind" => "question", "slug" => "question-1", @@ -64,9 +64,9 @@ module SimpleSmartAnswers describe "#process_response" do before do - @flow = instance_double("Flow", node_for_slug: :a_node) + @flow = instance_double(Flow, node_for_slug: :a_node) @node = - Node.new( + described_class.new( @flow, "kind" => "question", "slug" => "question-1", From 85dff8e703c74b4aa65c18a08aa3a3b63e82b6eb Mon Sep 17 00:00:00 2001 From: Keith Lawrence Date: Thu, 12 Dec 2024 16:12:22 +0000 Subject: [PATCH 04/19] Fix tests broken by rubocop -A --- spec/requests/calendars_spec.rb | 4 +-- spec/unit/calendar_division_spec.rb | 52 ++++++++++++++--------------- spec/unit/calendar_spec.rb | 34 +++++++++---------- 3 files changed, 45 insertions(+), 45 deletions(-) diff --git a/spec/requests/calendars_spec.rb b/spec/requests/calendars_spec.rb index 2e07e759dd..af13a26cf6 100644 --- a/spec/requests/calendars_spec.rb +++ b/spec/requests/calendars_spec.rb @@ -85,7 +85,7 @@ context "GET 'division'" do before do - @division = instance_double(Division, to_json: "", events: []) + @division = instance_double(Calendar::Division, to_json: "", events: []) @calendar = instance_double(Calendar, division: @division) allow(Calendar).to receive(:find).and_return(@calendar) stub_content_store_has_item("/a-calendar", { @@ -109,7 +109,7 @@ expect(@division).to receive(:events).and_return(:some_events) expect(@calendar).to receive(:division).with("a-division").and_return(@division) allow(Calendar).to receive(:find).with("a-calendar").and_return(@calendar) - expect(IcsRenderer).to receive(:new).with(:some_events, "/a-calendar/a-division.ics", :en).and_return(instance_double(Renderer, render: "ics_division")) + expect(IcsRenderer).to receive(:new).with(:some_events, "/a-calendar/a-division.ics", :en).and_return(instance_double(IcsRenderer, render: "ics_division")) get "/a-calendar/a-division.ics" expect(response.body).to eq("ics_division") diff --git a/spec/unit/calendar_division_spec.rb b/spec/unit/calendar_division_spec.rb index 358c9c13a6..259da3479e 100644 --- a/spec/unit/calendar_division_spec.rb +++ b/spec/unit/calendar_division_spec.rb @@ -74,17 +74,17 @@ end it "merges events for all years into single array" do - @years << instance_double(Year1, events: [1, 2]) - @years << instance_double(Year2, events: [3, 4, 5]) - @years << instance_double(Year3, events: [6, 7]) + @years << instance_double(Calendar::Year, events: [1, 2]) + @years << instance_double(Calendar::Year, events: [3, 4, 5]) + @years << instance_double(Calendar::Year, events: [6, 7]) expect(@div.events).to eq([1, 2, 3, 4, 5, 6, 7]) end it "handles years with no events" do - @years << instance_double(Year1, events: [1, 2]) - @years << instance_double(Year2, events: []) - @years << instance_double(Year3, events: [6, 7]) + @years << instance_double(Calendar::Year, events: [1, 2]) + @years << instance_double(Calendar::Year, events: []) + @years << instance_double(Calendar::Year, events: [6, 7]) expect(@div.events).to eq([1, 2, 6, 7]) end @@ -102,23 +102,23 @@ end it "returns nil if no years have upcoming_events" do - @years << instance_double(Year, upcoming_event: nil) - @years << instance_double(Year, upcoming_event: nil) + @years << instance_double(Calendar::Year, upcoming_event: nil) + @years << instance_double(Calendar::Year, upcoming_event: nil) expect(@div.upcoming_event).to be_nil end it "returns the upcoming event for the first year that has one" do - @years << instance_double(Year, upcoming_event: nil) - @years << instance_double(Year, upcoming_event: :event_1) - @years << instance_double(Year, upcoming_event: :event_2) + @years << instance_double(Calendar::Year, upcoming_event: nil) + @years << instance_double(Calendar::Year, upcoming_event: :event_1) + @years << instance_double(Calendar::Year, upcoming_event: :event_2) expect(@div.upcoming_event).to eq(:event_1) end it "caches the event" do - y1 = instance_double(Year) - y2 = instance_double(Year, upcoming_event: :event_1) + y1 = instance_double(Calendar::Year) + y2 = instance_double(Calendar::Year, upcoming_event: :event_1) @years << y1 @years << y2 @@ -137,16 +137,16 @@ end it "returns a hash of year => events for upcoming events" do - y1 = instance_double(Year1, upcoming_events: %i[e1 e2]) - y2 = instance_double(Year2, upcoming_events: %i[e3 e4 e5]) + y1 = instance_double(Calendar::Year, upcoming_events: %i[e1 e2]) + y2 = instance_double(Calendar::Year, upcoming_events: %i[e3 e4 e5]) ((@years << y1) << y2) expected = { y1 => %i[e1 e2], y2 => %i[e3 e4 e5] } expect(@div.upcoming_events_by_year).to eq(expected) end it "does not include any years with no upcoming events" do - y1 = instance_double(Year1, upcoming_events: []) - y2 = instance_double(Year2, upcoming_events: %i[e1 e2 e3]) + y1 = instance_double(Calendar::Year, upcoming_events: []) + y2 = instance_double(Calendar::Year, upcoming_events: %i[e1 e2 e3]) ((@years << y1) << y2) expected = { y2 => %i[e1 e2 e3] } expect(@div.upcoming_events_by_year).to eq(expected) @@ -161,8 +161,8 @@ end it "returns a hash of year => reversed events for past events" do - y1 = instance_double(Year, past_events: %i[e1 e2]) - y2 = instance_double(Year, past_events: %i[e3 e4 e5]) + y1 = instance_double(Calendar::Year, past_events: %i[e1 e2]) + y2 = instance_double(Calendar::Year, past_events: %i[e3 e4 e5]) ((@years << y1) << y2) expected = { y1 => %i[e2 e1], y2 => %i[e5 e4 e3] } events_by_year = @div.past_events_by_year @@ -172,8 +172,8 @@ end it "does not include any years with no past events" do - y1 = instance_double(Year, past_events: %i[e1 e2]) - y2 = instance_double(Year, past_events: []) + y1 = instance_double(Calendar::Year, past_events: %i[e1 e2]) + y2 = instance_double(Calendar::Year, past_events: []) ((@years << y1) << y2) expected = { y1 => %i[e2 e1] } @@ -185,21 +185,21 @@ before { @div = described_class.new("something") } it "is true if there is a buntable bank holiday today" do - @event = instance_double(Event, bunting: true, date: Time.zone.today) + @event = Calendar::Event.new("bunting" => true, "date" => Time.zone.today) allow(@div).to receive(:upcoming_event).and_return(@event) expect(@div.show_bunting?).to be true end it "is false if there is a non-buntable bank holiday today" do - @event = instance_double(Event, bunting: false, date: Time.zone.today) + @event = Calendar::Event.new("bunting" => false, "date" => Time.zone.today) allow(@div).to receive(:upcoming_event).and_return(@event) expect(@div.show_bunting?).to be false end it "is false if there is no bank holiday today" do - @event = instance_double(Event, bunting: true, date: (Time.zone.today + 1.week)) + @event = Calendar::Event.new("bunting" => true, "date" => Time.zone.today + 1.week) allow(@div).to receive(:upcoming_event).and_return(@event) expect(@div.show_bunting?).to be false @@ -215,8 +215,8 @@ end it "returns all events from all years" do - y1 = instance_double(Year, events: [1, 2]) - y2 = instance_double(Year, events: [3, 4]) + y1 = instance_double(Calendar::Year, events: [1, 2]) + y2 = instance_double(Calendar::Year, events: [3, 4]) allow(@div).to receive(:years).and_return([y1, y2]) hash = @div.as_json diff --git a/spec/unit/calendar_spec.rb b/spec/unit/calendar_spec.rb index 020ff93ecf..6cf29e5e58 100644 --- a/spec/unit/calendar_spec.rb +++ b/spec/unit/calendar_spec.rb @@ -76,17 +76,17 @@ end it "merges events for all years into single array" do - @divisions << instance_double(Division, events: [1, 2]) - @divisions << instance_double(Division, events: [3, 4, 5]) - @divisions << instance_double(Division, events: [6, 7]) + @divisions << instance_double(Calendar::Division, events: [1, 2]) + @divisions << instance_double(Calendar::Division, events: [3, 4, 5]) + @divisions << instance_double(Calendar::Division, events: [6, 7]) expect(@calendar.events).to eq([1, 2, 3, 4, 5, 6, 7]) end it "handles years with no events" do - @divisions << instance_double(Division1, events: [1, 2]) - @divisions << instance_double(Division2, events: []) - @divisions << instance_double(Division3, events: [6, 7]) + @divisions << instance_double(Calendar::Division, events: [1, 2]) + @divisions << instance_double(Calendar::Division, events: []) + @divisions << instance_double(Calendar::Division, events: [6, 7]) expect(@calendar.events).to eq([1, 2, 6, 7]) end @@ -110,27 +110,27 @@ before { @cal = described_class.new("a-calendar") } it "is true when one division is buntable" do - @div1 = instance_double(Division, show_bunting?: true) - @div2 = instance_double(Division, show_bunting?: false) - @div3 = instance_double(Division, show_bunting?: false) + @div1 = instance_double(Calendar::Division, show_bunting?: true) + @div2 = instance_double(Calendar::Division, show_bunting?: false) + @div3 = instance_double(Calendar::Division, show_bunting?: false) allow(@cal).to receive(:divisions).and_return([@div1, @div2, @div3]) expect(@cal.show_bunting?).to be true end it "is true when more than one division is buntable" do - @div1 = instance_double(Division, show_bunting?: true) - @div2 = instance_double(Division, show_bunting?: true) - @div3 = instance_double(Division, show_bunting?: false) + @div1 = instance_double(Calendar::Division, show_bunting?: true) + @div2 = instance_double(Calendar::Division, show_bunting?: true) + @div3 = instance_double(Calendar::Division, show_bunting?: false) allow(@cal).to receive(:divisions).and_return([@div1, @div2, @div3]) expect(@cal.show_bunting?).to be true end it "is false when no divisions are buntable" do - @div1 = instance_double(Division, show_bunting?: false) - @div2 = instance_double(Division, show_bunting?: false) - @div3 = instance_double(Division, show_bunting?: false) + @div1 = instance_double(Calendar::Division, show_bunting?: false) + @div2 = instance_double(Calendar::Division, show_bunting?: false) + @div3 = instance_double(Calendar::Division, show_bunting?: false) allow(@cal).to receive(:divisions).and_return([@div1, @div2, @div3]) expect(@cal.show_bunting?).to be false @@ -139,8 +139,8 @@ describe "#as_json" do before do - @div1 = instance_double(Division, slug: "division-1", as_json: "div1 json") - @div2 = instance_double(Division, slug: "division-2", as_json: "div2 json") + @div1 = instance_double(Calendar::Division, slug: "division-1", as_json: "div1 json") + @div2 = instance_double(Calendar::Division, slug: "division-2", as_json: "div2 json") @cal = described_class.new("a-calendar") allow(@cal).to receive(:divisions).and_return([@div1, @div2]) end From bf1d376d89cb0cefb951dd1daaf8b21d630b7539 Mon Sep 17 00:00:00 2001 From: Keith Lawrence Date: Thu, 12 Dec 2024 16:13:34 +0000 Subject: [PATCH 05/19] Fix lints in spec/components --- spec/components/published_dates_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/components/published_dates_spec.rb b/spec/components/published_dates_spec.rb index 1e99c2e46c..031e2c0890 100644 --- a/spec/components/published_dates_spec.rb +++ b/spec/components/published_dates_spec.rb @@ -47,7 +47,7 @@ def component_name it "only adds history id when passed page history" do render_component(published: "1st November 2000") - expect(rendered).not_to have_css("#full-publication-update-history", visible: false) + expect(rendered).not_to have_css("#full-publication-update-history", visible: :hidden) render_component( published: "1st November 2000", From 2ac0c68ba7ee6362be51d576aad90aa0b770619c Mon Sep 17 00:00:00 2001 From: Keith Lawrence Date: Thu, 12 Dec 2024 16:18:14 +0000 Subject: [PATCH 06/19] lint: fix spec/helpers - Here the ?query=true parameter was added because otherwise there wasn't actually a query string to strip, and rubocop identified it as being the identical to the test on line 39. --- spec/helpers/application_helper_spec.rb | 2 +- spec/helpers/block_helper_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index 44452450f1..2e8b2b31ae 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -43,7 +43,7 @@ def dummy_publication end it "returns the path of the current request stripping off any query string parameters" do - allow(self).to receive(:request).and_return(ActionDispatch::TestRequest.new("PATH_INFO" => "/foo/bar")) + allow(self).to receive(:request).and_return(ActionDispatch::TestRequest.new("PATH_INFO" => "/foo/bar?query=true")) expect(current_path_without_query_string).to eq("/foo/bar") end diff --git a/spec/helpers/block_helper_spec.rb b/spec/helpers/block_helper_spec.rb index 134e27ecc4..c6d3d78584 100644 --- a/spec/helpers/block_helper_spec.rb +++ b/spec/helpers/block_helper_spec.rb @@ -3,7 +3,7 @@ describe "#render_block" do it "returns an empty string when a partial template doesn't exist" do - block = double(type: "not_a_block") + block = instance_double(LandingPage::Block::Base, type: "not_a_block") expect(render_block(block)).to be_empty end end From d6ec7db9d418173f10ea9beafd75ecf6879eab08 Mon Sep 17 00:00:00 2001 From: Keith Lawrence Date: Thu, 12 Dec 2024 16:28:39 +0000 Subject: [PATCH 07/19] lint: fix spec/models --- spec/models/content_item_spec.rb | 24 +++++++------- spec/models/homepage_spec.rb | 11 +++---- .../landing_page/block/block_error_spec.rb | 7 ++-- .../block/blocks_container_spec.rb | 5 +-- spec/models/landing_page/block/box_spec.rb | 13 ++++---- spec/models/landing_page/block/card_spec.rb | 17 +++++----- .../landing_page/block/columns_layout_spec.rb | 5 +-- .../landing_page/block/featured_spec.rb | 25 +++++++------- spec/models/landing_page/block/hero_spec.rb | 33 ++++++++++--------- spec/models/landing_page/block/image_spec.rb | 17 +++++----- .../landing_page/block/layout_base_spec.rb | 7 ++-- .../block/main_navigation_spec.rb | 15 +++++---- .../landing_page/block/share_links_spec.rb | 13 ++++---- .../block/side_navigation_spec.rb | 9 ++--- .../block/two_column_layout_spec.rb | 17 +++++----- spec/models/travel_advice_spec.rb | 14 ++++---- 16 files changed, 120 insertions(+), 112 deletions(-) diff --git a/spec/models/content_item_spec.rb b/spec/models/content_item_spec.rb index 172d6d1116..f30ea6fa8e 100644 --- a/spec/models/content_item_spec.rb +++ b/spec/models/content_item_spec.rb @@ -1,5 +1,5 @@ RSpec.describe ContentItem do - let(:subject) { build(:content_item_with_data_attachments, schema_name: "fancy_page_type") } + subject(:content_item) { build(:content_item_with_data_attachments, schema_name: "fancy_page_type") } it_behaves_like "it can be withdrawn", "detailed_guide", "withdrawn_detailed_guide" @@ -55,37 +55,37 @@ describe "#is_a_xxxx?" do it "returns true when called with the schema name of the object" do - expect(subject.is_a_fancy_page_type?).to be true + expect(content_item.is_a_fancy_page_type?).to be true end it "returns false when called with a mismatching schema name" do - expect(subject.is_a_place?).to be false + expect(content_item.is_a_place?).to be false end it "also handles is_an_xxxx?" do - expect(subject.is_an_organisation?).to be false + expect(content_item.is_an_organisation?).to be false end it "still passes other missing methods to parent" do - expect { subject.was_a_fancy_page_type? }.to raise_error(NoMethodError) + expect { content_item.was_a_fancy_page_type? }.to raise_error(NoMethodError) end it "responds to the various methods" do - expect(subject.respond_to?(:is_a_fancy_page_type?)).to be true - expect(subject.respond_to?(:is_an_organisation?)).to be true - expect(subject.respond_to?(:was_a_landing_page?)).to be false + expect(content_item.respond_to?(:is_a_fancy_page_type?)).to be true + expect(content_item.respond_to?(:is_an_organisation?)).to be true + expect(content_item.respond_to?(:was_a_landing_page?)).to be false end end describe "#attachments" do it "loads the attachment data from the content item" do - expect(subject.attachments.count).to eq(4) - expect(subject.attachments[0].title).to eq("Data One") + expect(content_item.attachments.count).to eq(4) + expect(content_item.attachments[0].title).to eq("Data One") end end describe "#available_translations" do - let(:subject) do + subject(:content_item) do described_class.new( { "links" => { @@ -99,7 +99,7 @@ end it "returns sorted translations with default translation at the top" do - expect(subject.available_translations).to eq([ + expect(content_item.available_translations).to eq([ { "locale" => "en" }, { "locale" => "cy" }, ]) diff --git a/spec/models/homepage_spec.rb b/spec/models/homepage_spec.rb index 159766d12d..cc7f5f30dd 100644 --- a/spec/models/homepage_spec.rb +++ b/spec/models/homepage_spec.rb @@ -1,18 +1,15 @@ RSpec.describe Homepage do - before do - @content_store_response = GovukSchemas::Example.find("homepage", example_name: "homepage_with_popular_links_on_govuk") - end - - let(:subject) { described_class.new(@content_store_response) } + let!(:content_store_response) { GovukSchemas::Example.find("homepage", example_name: "homepage_with_popular_links_on_govuk") } + let(:homepage) { described_class.new(content_store_response) } describe "#popular_links" do context "when popular links in the content item" do it "returns popular links from the content item" do - expected_popular_links = @content_store_response + expected_popular_links = content_store_response .dig("links", "popular_links", 0, "details", "link_items") .collect(&:with_indifferent_access) - expect(subject.popular_links).to eq(expected_popular_links) + expect(homepage.popular_links).to eq(expected_popular_links) end end end diff --git a/spec/models/landing_page/block/block_error_spec.rb b/spec/models/landing_page/block/block_error_spec.rb index 65e67b47f6..b54b970326 100644 --- a/spec/models/landing_page/block/block_error_spec.rb +++ b/spec/models/landing_page/block/block_error_spec.rb @@ -1,5 +1,6 @@ RSpec.describe LandingPage::Block::BlockError do - let(:subject) { described_class.new(blocks_hash, build(:landing_page)) } + subject(:block_error) { described_class.new(blocks_hash, build(:landing_page)) } + let(:blocks_hash) do { "type" => "block_error", @@ -10,7 +11,7 @@ it_behaves_like "it is a landing-page block" it "has an error" do - expect(subject.error).to be_instance_of(StandardError) - expect(subject.error.message).to eq("This error") + expect(block_error.error).to be_instance_of(StandardError) + expect(block_error.error.message).to eq("This error") end end diff --git a/spec/models/landing_page/block/blocks_container_spec.rb b/spec/models/landing_page/block/blocks_container_spec.rb index 71716ca75e..4703916500 100644 --- a/spec/models/landing_page/block/blocks_container_spec.rb +++ b/spec/models/landing_page/block/blocks_container_spec.rb @@ -1,5 +1,6 @@ RSpec.describe LandingPage::Block::BlocksContainer do - let(:subject) { described_class.new(blocks_hash, build(:landing_page)) } + subject(:blocks_container) { described_class.new(blocks_hash, build(:landing_page)) } + let(:blocks_hash) do { "type" => "blocks_container", @@ -23,6 +24,6 @@ it_behaves_like "it is a landing-page block" it "has children blocks" do - expect(subject.children.size).to eq(3) + expect(blocks_container.children.size).to eq(3) end end diff --git a/spec/models/landing_page/block/box_spec.rb b/spec/models/landing_page/block/box_spec.rb index a78551a144..aa08a5a96a 100644 --- a/spec/models/landing_page/block/box_spec.rb +++ b/spec/models/landing_page/block/box_spec.rb @@ -1,5 +1,6 @@ RSpec.describe LandingPage::Block::Box do - let(:subject) { described_class.new(blocks_hash, build(:landing_page)) } + subject(:box) { described_class.new(blocks_hash, build(:landing_page)) } + let(:blocks_hash) do { "type" => "box", "href" => "/landing-page/something", @@ -15,15 +16,15 @@ it_behaves_like "it is a landing-page block" it "includes a link" do - expect(subject.href).to eq "/landing-page/something" - expect(subject.content).to eq "This is a link" + expect(box.href).to eq "/landing-page/something" + expect(box.content).to eq "This is a link" end describe "#box_content" do it "returns an array of instantiated blocks" do - expect(subject.box_content.size).to eq 2 - expect(subject.box_content.first.data).to eq("type" => "govspeak", "content" => "

Title

") - expect(subject.box_content.second.data).to eq("type" => "govspeak", "content" => "

Some content!

") + expect(box.box_content.size).to eq 2 + expect(box.box_content.first.data).to eq("type" => "govspeak", "content" => "

Title

") + expect(box.box_content.second.data).to eq("type" => "govspeak", "content" => "

Some content!

") end end end diff --git a/spec/models/landing_page/block/card_spec.rb b/spec/models/landing_page/block/card_spec.rb index 0a76469b50..3b7b9f3109 100644 --- a/spec/models/landing_page/block/card_spec.rb +++ b/spec/models/landing_page/block/card_spec.rb @@ -1,5 +1,6 @@ RSpec.describe LandingPage::Block::Card do - let(:subject) { described_class.new(blocks_hash, build(:landing_page)) } + subject(:card) { described_class.new(blocks_hash, build(:landing_page)) } + let(:blocks_hash) do { "type" => "card", "href" => "/landing-page/something", @@ -20,23 +21,23 @@ describe "#link" do it "includes a link" do - expect(subject.href).to eq "/landing-page/something" - expect(subject.content).to eq "This is a link" + expect(card.href).to eq "/landing-page/something" + expect(card.content).to eq "This is a link" end end describe "#image" do it "returns the properties of the image" do - expect(subject.image.alt).to eq "some alt text" - expect(subject.image.source).to eq "landing_page/placeholder/chart.png" + expect(card.image.alt).to eq "some alt text" + expect(card.image.source).to eq "landing_page/placeholder/chart.png" end end describe "#card_content" do it "returns an array of instantiated blocks" do - expect(subject.card_content.size).to eq 2 - expect(subject.card_content.first.data).to eq("type" => "govspeak", "content" => "

Title

") - expect(subject.card_content.second.data).to eq("type" => "govspeak", "content" => "

Some content!

") + expect(card.card_content.size).to eq 2 + expect(card.card_content.first.data).to eq("type" => "govspeak", "content" => "

Title

") + expect(card.card_content.second.data).to eq("type" => "govspeak", "content" => "

Some content!

") end end end diff --git a/spec/models/landing_page/block/columns_layout_spec.rb b/spec/models/landing_page/block/columns_layout_spec.rb index aac737fb9d..dd2d4fb3fa 100644 --- a/spec/models/landing_page/block/columns_layout_spec.rb +++ b/spec/models/landing_page/block/columns_layout_spec.rb @@ -1,5 +1,6 @@ RSpec.describe LandingPage::Block::ColumnsLayout do - let(:subject) { described_class.new(blocks_hash, build(:landing_page)) } + subject(:columns_layout) { described_class.new(blocks_hash, build(:landing_page)) } + let(:blocks_hash) do { "type" => "columns_layout", @@ -23,6 +24,6 @@ it_behaves_like "it is a landing-page block" it "has column blocks" do - expect(subject.blocks.size).to eq(3) + expect(columns_layout.blocks.size).to eq(3) end end diff --git a/spec/models/landing_page/block/featured_spec.rb b/spec/models/landing_page/block/featured_spec.rb index 33dc6154ba..49e91a65e5 100644 --- a/spec/models/landing_page/block/featured_spec.rb +++ b/spec/models/landing_page/block/featured_spec.rb @@ -1,5 +1,6 @@ RSpec.describe LandingPage::Block::Featured do - let(:subject) { described_class.new(blocks_hash, build(:landing_page)) } + subject(:featured) { described_class.new(blocks_hash, build(:landing_page)) } + let(:blocks_hash) do { "type" => "featured", "image" => { @@ -25,27 +26,27 @@ describe "#image" do it "returns the properties of the image" do - expect(subject.image.alt).to eq "some alt text" - expect(subject.image.sources.desktop).to eq "landing_page/desktop.jpeg" - expect(subject.image.sources.desktop_2x).to eq "landing_page/desktop_2x.jpeg" - expect(subject.image.sources.mobile).to eq "landing_page/mobile.jpeg" - expect(subject.image.sources.mobile_2x).to eq "landing_page/mobile_2x.jpeg" - expect(subject.image.sources.tablet).to eq "landing_page/tablet.jpeg" - expect(subject.image.sources.tablet_2x).to eq "landing_page/tablet_2x.jpeg" + expect(featured.image.alt).to eq "some alt text" + expect(featured.image.sources.desktop).to eq "landing_page/desktop.jpeg" + expect(featured.image.sources.desktop_2x).to eq "landing_page/desktop_2x.jpeg" + expect(featured.image.sources.mobile).to eq "landing_page/mobile.jpeg" + expect(featured.image.sources.mobile_2x).to eq "landing_page/mobile_2x.jpeg" + expect(featured.image.sources.tablet).to eq "landing_page/tablet.jpeg" + expect(featured.image.sources.tablet_2x).to eq "landing_page/tablet_2x.jpeg" end end describe "#featured_content" do it "returns an array of instantiated blocks" do - expect(subject.featured_content.size).to eq 2 - expect(subject.featured_content.first.data).to eq("type" => "govspeak", "content" => "

Some content!

") - expect(subject.featured_content.second.data).to eq("type" => "govspeak", "content" => "

More content!

") + expect(featured.featured_content.size).to eq 2 + expect(featured.featured_content.first.data).to eq("type" => "govspeak", "content" => "

Some content!

") + expect(featured.featured_content.second.data).to eq("type" => "govspeak", "content" => "

More content!

") end end describe "#full_width?" do it "is false" do - expect(subject.full_width?).to be(false) + expect(featured.full_width?).to be(false) end end end diff --git a/spec/models/landing_page/block/hero_spec.rb b/spec/models/landing_page/block/hero_spec.rb index 840e3c399a..f30d1491b8 100644 --- a/spec/models/landing_page/block/hero_spec.rb +++ b/spec/models/landing_page/block/hero_spec.rb @@ -1,5 +1,6 @@ RSpec.describe LandingPage::Block::Hero do - let(:subject) { described_class.new(blocks_hash, build(:landing_page)) } + subject(:hero) { described_class.new(blocks_hash, build(:landing_page)) } + let(:blocks_hash) do { "type" => "hero", "image" => { @@ -25,21 +26,21 @@ describe "#image" do it "returns the properties of the image" do - expect(subject.image.alt).to eq "some alt text" - expect(subject.image.sources.desktop).to eq "landing_page/desktop.jpeg" - expect(subject.image.sources.desktop_2x).to eq "landing_page/desktop_2x.jpeg" - expect(subject.image.sources.mobile).to eq "landing_page/mobile.jpeg" - expect(subject.image.sources.mobile_2x).to eq "landing_page/mobile_2x.jpeg" - expect(subject.image.sources.tablet).to eq "landing_page/tablet.jpeg" - expect(subject.image.sources.tablet_2x).to eq "landing_page/tablet_2x.jpeg" + expect(hero.image.alt).to eq "some alt text" + expect(hero.image.sources.desktop).to eq "landing_page/desktop.jpeg" + expect(hero.image.sources.desktop_2x).to eq "landing_page/desktop_2x.jpeg" + expect(hero.image.sources.mobile).to eq "landing_page/mobile.jpeg" + expect(hero.image.sources.mobile_2x).to eq "landing_page/mobile_2x.jpeg" + expect(hero.image.sources.tablet).to eq "landing_page/tablet.jpeg" + expect(hero.image.sources.tablet_2x).to eq "landing_page/tablet_2x.jpeg" end end describe "#hero_content" do it "returns an array of instantiated blocks" do - expect(subject.hero_content.size).to eq 2 - expect(subject.hero_content.first.data).to eq("type" => "govspeak", "content" => "

Some content!

") - expect(subject.hero_content.second.data).to eq("type" => "govspeak", "content" => "

More content!

") + expect(hero.hero_content.size).to eq 2 + expect(hero.hero_content.first.data).to eq("type" => "govspeak", "content" => "

Some content!

") + expect(hero.hero_content.second.data).to eq("type" => "govspeak", "content" => "

More content!

") end it "returns nil if hero_content isn't provided" do @@ -66,29 +67,29 @@ describe "#theme" do it "defaults to default" do - expect(subject.theme).to eq("default") + expect(hero.theme).to eq("default") end it "returns the theme from config" do blocks_hash["theme"] = "middle_left" - expect(subject.theme).to eq("middle_left") + expect(hero.theme).to eq("middle_left") end end describe "#theme_colour" do it "defaults to nil" do - expect(subject.theme_colour).to be_nil + expect(hero.theme_colour).to be_nil end it "returns the theme_colour from config" do blocks_hash["theme_colour"] = 2 - expect(subject.theme_colour).to eq(2) + expect(hero.theme_colour).to eq(2) end end describe "#full_width?" do it "is true" do - expect(subject.full_width?).to be(true) + expect(hero.full_width?).to be(true) end end end diff --git a/spec/models/landing_page/block/image_spec.rb b/spec/models/landing_page/block/image_spec.rb index 77a7dbc916..8f860b41fe 100644 --- a/spec/models/landing_page/block/image_spec.rb +++ b/spec/models/landing_page/block/image_spec.rb @@ -1,5 +1,6 @@ RSpec.describe LandingPage::Block::Image do - let(:subject) { described_class.new(blocks_hash, build(:landing_page)) } + subject(:image) { described_class.new(blocks_hash, build(:landing_page)) } + let(:blocks_hash) do { "type" => "image", "image" => { @@ -19,13 +20,13 @@ describe "#image" do it "returns the properties of the image" do - expect(subject.image.alt).to eq "some alt text" - expect(subject.image.sources.desktop).to eq "landing_page/desktop.jpeg" - expect(subject.image.sources.desktop_2x).to eq "landing_page/desktop_2x.jpeg" - expect(subject.image.sources.mobile).to eq "landing_page/mobile.jpeg" - expect(subject.image.sources.mobile_2x).to eq "landing_page/mobile_2x.jpeg" - expect(subject.image.sources.tablet).to eq "landing_page/tablet.jpeg" - expect(subject.image.sources.tablet_2x).to eq "landing_page/tablet_2x.jpeg" + expect(image.image.alt).to eq "some alt text" + expect(image.image.sources.desktop).to eq "landing_page/desktop.jpeg" + expect(image.image.sources.desktop_2x).to eq "landing_page/desktop_2x.jpeg" + expect(image.image.sources.mobile).to eq "landing_page/mobile.jpeg" + expect(image.image.sources.mobile_2x).to eq "landing_page/mobile_2x.jpeg" + expect(image.image.sources.tablet).to eq "landing_page/tablet.jpeg" + expect(image.image.sources.tablet_2x).to eq "landing_page/tablet_2x.jpeg" end end end diff --git a/spec/models/landing_page/block/layout_base_spec.rb b/spec/models/landing_page/block/layout_base_spec.rb index 7550caf15f..a03710fbe8 100644 --- a/spec/models/landing_page/block/layout_base_spec.rb +++ b/spec/models/landing_page/block/layout_base_spec.rb @@ -1,5 +1,7 @@ RSpec.describe LandingPage::Block::LayoutBase do describe "#blocks" do + subject(:layout_base) { described_class.new(blocks_hash, build(:landing_page)) } + let(:blocks_hash) do { "blocks" => [ @@ -9,14 +11,13 @@ ], } end - let(:subject) { described_class.new(blocks_hash, build(:landing_page)) } it "builds all of the blocks" do - expect(subject.blocks.count).to eq 3 + expect(layout_base.blocks.count).to eq 3 end it "builds blocks of the correct type" do - expect(subject.blocks.first.type).to eq("govspeak") + expect(layout_base.blocks.first.type).to eq("govspeak") end end end diff --git a/spec/models/landing_page/block/main_navigation_spec.rb b/spec/models/landing_page/block/main_navigation_spec.rb index 62f77deb4a..eb79bf7a1a 100644 --- a/spec/models/landing_page/block/main_navigation_spec.rb +++ b/spec/models/landing_page/block/main_navigation_spec.rb @@ -1,5 +1,6 @@ RSpec.describe LandingPage::Block::MainNavigation do - let(:subject) { described_class.new(blocks_hash, build(:landing_page_with_navigation_groups)) } + subject(:main_navigation) { described_class.new(blocks_hash, build(:landing_page_with_navigation_groups)) } + let(:blocks_hash) do { "type" => "main_navigation", @@ -11,10 +12,10 @@ describe "#links" do it "returns an array of links from the specified navigation group" do - expect(subject.links.size).to eq 3 - expect(subject.links.first).to eq({ "text" => "Service Name", "href" => "https://www.gov.uk" }) - expect(subject.links.second).to eq({ "text" => "Test 1", "href" => "/hello" }) - expect(subject.links.third).to eq({ + expect(main_navigation.links.size).to eq 3 + expect(main_navigation.links.first).to eq({ "text" => "Service Name", "href" => "https://www.gov.uk" }) + expect(main_navigation.links.second).to eq({ "text" => "Test 1", "href" => "/hello" }) + expect(main_navigation.links.third).to eq({ "text" => "Test 2", "href" => "/goodbye", "links" => [ @@ -33,13 +34,13 @@ describe "#name" do it "returns the name of the navigation group" do - expect(subject.name).to eq("Some navigation group name") + expect(main_navigation.name).to eq("Some navigation group name") end end describe "#full_width?" do it "is true" do - expect(subject.full_width?).to be(true) + expect(main_navigation.full_width?).to be(true) end end diff --git a/spec/models/landing_page/block/share_links_spec.rb b/spec/models/landing_page/block/share_links_spec.rb index fdea42de39..d5a58e91dc 100644 --- a/spec/models/landing_page/block/share_links_spec.rb +++ b/spec/models/landing_page/block/share_links_spec.rb @@ -1,5 +1,6 @@ RSpec.describe LandingPage::Block::ShareLinks do - let(:subject) { described_class.new(blocks_hash, build(:landing_page)) } + subject(:share_links) { described_class.new(blocks_hash, build(:landing_page)) } + let(:blocks_hash) do { "type" => "share_links", @@ -23,10 +24,10 @@ it_behaves_like "it is a landing-page block" it "returns all of the side navigation links" do - expect(subject.links.count).to eq(2) - expect(subject.links.first[:text]).to eq("Twitter") - expect(subject.links.first[:href]).to eq("/twitter-profile") - expect(subject.links.first[:icon]).to eq("twitter") - expect(subject.links.first[:hidden_text]).to eq("Follow us on") + expect(share_links.links.count).to eq(2) + expect(share_links.links.first[:text]).to eq("Twitter") + expect(share_links.links.first[:href]).to eq("/twitter-profile") + expect(share_links.links.first[:icon]).to eq("twitter") + expect(share_links.links.first[:hidden_text]).to eq("Follow us on") end end diff --git a/spec/models/landing_page/block/side_navigation_spec.rb b/spec/models/landing_page/block/side_navigation_spec.rb index 74b68c957b..bf8cc65c5a 100644 --- a/spec/models/landing_page/block/side_navigation_spec.rb +++ b/spec/models/landing_page/block/side_navigation_spec.rb @@ -1,5 +1,6 @@ RSpec.describe LandingPage::Block::SideNavigation do - let(:subject) { described_class.new(blocks_hash, build(:landing_page_with_navigation_groups)) } + subject(:side_navigation) { described_class.new(blocks_hash, build(:landing_page_with_navigation_groups)) } + let(:blocks_hash) do { "type" => "side_navigation", @@ -11,9 +12,9 @@ describe "#links" do it "returns all of the navigation links from the specified navigation group" do - expect(subject.links.count).to eq(2) - expect(subject.links.first["text"]).to eq("Child a") - expect(subject.links.first["href"]).to eq("/a") + expect(side_navigation.links.count).to eq(2) + expect(side_navigation.links.first["text"]).to eq("Child a") + expect(side_navigation.links.first["href"]).to eq("/a") end end diff --git a/spec/models/landing_page/block/two_column_layout_spec.rb b/spec/models/landing_page/block/two_column_layout_spec.rb index e3f9c6580f..deb9303979 100644 --- a/spec/models/landing_page/block/two_column_layout_spec.rb +++ b/spec/models/landing_page/block/two_column_layout_spec.rb @@ -1,5 +1,6 @@ RSpec.describe LandingPage::Block::TwoColumnLayout do - let(:subject) { described_class.new(blocks_hash, build(:landing_page)) } + subject(:two_column_layout) { described_class.new(blocks_hash, build(:landing_page)) } + let(:blocks_hash) do { "type" => "two_column_layout", "theme" => "two_thirds_one_third", @@ -13,35 +14,35 @@ describe "#left_column_class" do it "returns two thirds when the theme is two_thirds_one_third" do - expect(subject.left_column_class).to eq "govuk-grid-column-two-thirds-from-desktop" + expect(two_column_layout.left_column_class).to eq "govuk-grid-column-two-thirds-from-desktop" end it "returns one third when the theme is one_third_two_thirds" do blocks_hash["theme"] = "one_third_two_thirds" - expect(subject.left_column_class).to eq "govuk-grid-column-one-third" + expect(two_column_layout.left_column_class).to eq "govuk-grid-column-one-third" end it "returns offset column when the theme is two_thirds_right" do blocks_hash["theme"] = "two_thirds_right" - expect(subject.left_column_class).to eq "govuk-grid-column-one-third grid-column-one-third-offset" - expect(subject.left).to be_nil + expect(two_column_layout.left_column_class).to eq "govuk-grid-column-one-third grid-column-one-third-offset" + expect(two_column_layout.left).to be_nil end end describe "#right_column_class" do it "returns two thirds when the theme is one_third_two_thirds" do blocks_hash["theme"] = "one_third_two_thirds" - expect(subject.right_column_class).to eq "govuk-grid-column-two-thirds-from-desktop" + expect(two_column_layout.right_column_class).to eq "govuk-grid-column-two-thirds-from-desktop" end it "returns one third when the theme is two_thirds_one_third" do blocks_hash["theme"] = "two_thirds_one_third" - expect(subject.right_column_class).to eq "govuk-grid-column-one-third" + expect(two_column_layout.right_column_class).to eq "govuk-grid-column-one-third" end it "returns two thirds when the theme is two_thirds_right" do blocks_hash["theme"] = "two_thirds_right" - expect(subject.right_column_class).to eq "govuk-grid-column-two-thirds-from-desktop" + expect(two_column_layout.right_column_class).to eq "govuk-grid-column-two-thirds-from-desktop" end end end diff --git a/spec/models/travel_advice_spec.rb b/spec/models/travel_advice_spec.rb index 05c7499139..b821ca6a20 100644 --- a/spec/models/travel_advice_spec.rb +++ b/spec/models/travel_advice_spec.rb @@ -1,27 +1,25 @@ RSpec.describe TravelAdvice do - before do - @content_store_response = GovukSchemas::Example.find("travel_advice", example_name: "full-country") - end + let(:content_store_response) { GovukSchemas::Example.find("travel_advice", example_name: "full-country") } it_behaves_like "it has parts", "travel_advice", "full-country" describe "#alert_status" do it "adds allowed statuses" do - @content_store_response["details"]["alert_status"] = [described_class::ALERT_STATUSES.first] + content_store_response["details"]["alert_status"] = [described_class::ALERT_STATUSES.first] - alert_statuses = described_class.new(@content_store_response).alert_status + alert_statuses = described_class.new(content_store_response).alert_status expect(alert_statuses).to eq([described_class::ALERT_STATUSES.first]) end it "removes unexpected statuses" do - @content_store_response["details"]["alert_status"] = %w[unexpected-status] + content_store_response["details"]["alert_status"] = %w[unexpected-status] - alert_statuses = described_class.new(@content_store_response).alert_status + alert_statuses = described_class.new(content_store_response).alert_status expect(alert_statuses).to be_empty end it "returns nothing if there are no alerts" do - alert_statuses = described_class.new(@content_store_response).alert_status + alert_statuses = described_class.new(content_store_response).alert_status expect(alert_statuses).to be_empty end end From 254f999823b2d47ccc919a01bae3b1bfe25113c1 Mon Sep 17 00:00:00 2001 From: Keith Lawrence Date: Thu, 12 Dec 2024 16:33:27 +0000 Subject: [PATCH 08/19] lint: fix spec/presenters --- spec/presenter/get_involved_spec.rb | 32 ++++++++++++++--------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/spec/presenter/get_involved_spec.rb b/spec/presenter/get_involved_spec.rb index 4cde85122d..864566ca22 100644 --- a/spec/presenter/get_involved_spec.rb +++ b/spec/presenter/get_involved_spec.rb @@ -1,5 +1,5 @@ RSpec.describe GetInvolved do - let(:consultation_1) do + let(:consultation_closing_november) do { "end_date" => "2024-11-28T23:59:00.000+00:00", "title" => "Consultation on the International Council for Harmonisation (ICH) M14", @@ -10,7 +10,7 @@ } end - let(:consultation_2) do + let(:consultation_closing_december) do { "end_date" => "2024-12-28T23:59:00.000+00:00", "title" => "Consultation on the International Council", @@ -23,9 +23,9 @@ ## Tests - context "recently opened consultations" do + describe "#recently_opened" do let(:content_item) do - double("ContentItem", recently_opened_consultations: [consultation_1, consultation_2]) + instance_double(described_class, recently_opened_consultations: [consultation_closing_november, consultation_closing_december]) end it "returns formatted recently opened consultation links" do @@ -34,23 +34,23 @@ { link: { description: "Closes 28 November 2024", - path: consultation_1["link"], - text: consultation_1["title"], + path: consultation_closing_november["link"], + text: consultation_closing_november["title"], }, metadata: { - document_type: consultation_1["organisations"].first["acronym"], - public_updated_at: consultation_1["organisations"].first["public_timestamp"], + document_type: consultation_closing_november["organisations"].first["acronym"], + public_updated_at: consultation_closing_november["organisations"].first["public_timestamp"], }, }, { link: { description: "Closes 28 December 2024", - path: consultation_2["link"], - text: consultation_2["title"], + path: consultation_closing_december["link"], + text: consultation_closing_december["title"], }, metadata: { - document_type: consultation_2["organisations"].first["acronym"], - public_updated_at: consultation_2["organisations"].first["public_timestamp"], + document_type: consultation_closing_december["organisations"].first["acronym"], + public_updated_at: consultation_closing_december["organisations"].first["public_timestamp"], }, }, ] @@ -61,7 +61,7 @@ context "when the consultation is closed" do let(:content_item) do - double("ContentItem", next_closing_consultation: { "end_date" => (Time.zone.now.to_date - 1).strftime("%Y-%m-%dT23:59:00") }) + instance_double(described_class, next_closing_consultation: { "end_date" => (Time.zone.now.to_date - 1).strftime("%Y-%m-%dT23:59:00") }) end it "returns closed message" do @@ -73,7 +73,7 @@ context "when the consultation is closing today" do let(:content_item) do - double("ContentItem", next_closing_consultation: { "end_date" => Time.zone.now.to_date.strftime("%Y-%m-%dT23:59:00") }) + instance_double(described_class, next_closing_consultation: { "end_date" => Time.zone.now.to_date.strftime("%Y-%m-%dT23:59:00") }) end it "returns closing today message" do @@ -85,7 +85,7 @@ context "when the consultation is closing tomorrow" do let(:content_item) do - double("ContentItem", next_closing_consultation: { "end_date" => (Time.zone.now.to_date + 1).strftime("%Y-%m-%dT23:59:00") }) + instance_double(described_class, next_closing_consultation: { "end_date" => (Time.zone.now.to_date + 1).strftime("%Y-%m-%dT23:59:00") }) end it "returns closing tomorrow message" do @@ -97,7 +97,7 @@ context "when the consultation is closing in more than 1 day" do let(:content_item) do - double("ContentItem", next_closing_consultation: { "end_date" => (Time.zone.now.to_date + 2).strftime("%Y-%m-%dT23:59:00") }) + instance_double(described_class, next_closing_consultation: { "end_date" => (Time.zone.now.to_date + 2).strftime("%Y-%m-%dT23:59:00") }) end it "returns number of days left for closing" do From c953089372f53063e8d35d80cc97b84bd837c3c6 Mon Sep 17 00:00:00 2001 From: Keith Lawrence Date: Mon, 16 Dec 2024 19:59:16 +0000 Subject: [PATCH 09/19] Convert method to matcher - will be used in the following commits, allows us to include expectations in methods that would otherwise appear erroneously to have no expectations, fooling rubocop - But is also a more consistent call. --- spec/support/content_store_helpers.rb | 4 ---- spec/support/matchers/honours_content_store_ttl_matcher.rb | 5 +++++ 2 files changed, 5 insertions(+), 4 deletions(-) create mode 100644 spec/support/matchers/honours_content_store_ttl_matcher.rb diff --git a/spec/support/content_store_helpers.rb b/spec/support/content_store_helpers.rb index 3afb9bd762..30fb4f4c12 100644 --- a/spec/support/content_store_helpers.rb +++ b/spec/support/content_store_helpers.rb @@ -45,10 +45,6 @@ def content_store_throws_exception_for(path, exception) allow(GdsApi).to receive(:content_store).and_return(content_store) end - def honours_content_store_ttl - expect(response.headers["Cache-Control"]).to eq("max-age=#{15.minutes.to_i}, public") - end - def stub_homepage_content_item(links: []) content_item = GovukSchemas::Example.find("homepage", example_name: "homepage_with_popular_links_on_govuk") base_path = content_item.fetch("base_path") diff --git a/spec/support/matchers/honours_content_store_ttl_matcher.rb b/spec/support/matchers/honours_content_store_ttl_matcher.rb new file mode 100644 index 0000000000..bfce19bbeb --- /dev/null +++ b/spec/support/matchers/honours_content_store_ttl_matcher.rb @@ -0,0 +1,5 @@ +RSpec::Matchers.define :honour_content_store_ttl do + match do |actual| + actual.headers["Cache-Control"] == "max-age=#{15.minutes.to_i}, public" + end +end From 189a41b812500de214e613f1ad294c422d225137 Mon Sep 17 00:00:00 2001 From: Keith Lawrence Date: Mon, 16 Dec 2024 19:59:32 +0000 Subject: [PATCH 10/19] lint: fix spec/requests --- spec/requests/calendars_spec.rb | 37 ++--- ...tact_electoral_registration_office_spec.rb | 15 +- .../requests/content_loading_problems_spec.rb | 4 +- spec/requests/find_local_council_spec.rb | 2 +- spec/requests/get_involved_spec.rb | 2 +- spec/requests/help_spec.rb | 14 +- spec/requests/homepage_spec.rb | 4 +- spec/requests/landing_page_spec.rb | 15 +- spec/requests/licence_transactions_spec.rb | 16 +- spec/requests/local_transactions_spec.rb | 46 +++--- spec/requests/placeholder_spec.rb | 2 +- spec/requests/places_spec.rb | 12 +- spec/requests/roadmap_spec.rb | 2 +- spec/requests/sessions_spec.rb | 144 +++++++----------- spec/requests/simple_smart_answers_spec.rb | 29 ++-- spec/requests/static_error_pages_spec.rb | 2 +- spec/requests/take_part_spec.rb | 2 +- spec/requests/transactions_spec.rb | 32 ++-- spec/requests/travel_advice_spec.rb | 80 +++++----- .../simple_smart_answer_presenter_spec.rb | 2 +- 20 files changed, 216 insertions(+), 246 deletions(-) diff --git a/spec/requests/calendars_spec.rb b/spec/requests/calendars_spec.rb index af13a26cf6..fb963870d3 100644 --- a/spec/requests/calendars_spec.rb +++ b/spec/requests/calendars_spec.rb @@ -1,5 +1,5 @@ RSpec.describe "Calendars" do - context "GET 'calendar'" do + describe "GET 'calendar'" do before do allow(Calendar).to receive(:find).and_return(Calendar.new("bank-holidays", "title" => "Brilliant holidays!", "divisions" => [])) stub_content_store_has_item("/bank-holidays", { @@ -11,7 +11,7 @@ stub_content_store_has_item("/when-do-the-clocks-change", schema_name: "calendar") end - context "HTML request (no format)" do + context "when requesting HTML (with no format)" do it "loads the calendar and show it" do get "/bank-holidays" @@ -27,11 +27,11 @@ it "sets the expiry headers" do get "/bank-holidays" - honours_content_store_ttl + expect(response).to honour_content_store_ttl end end - context "for a welsh language content item" do + context "when the content item is in Welsh" do it "sets the I18n locale" do stub_content_store_has_item("/gwyliau-banc", { schema_name: "calendar", @@ -46,9 +46,9 @@ end end - context "json request" do + describe "json request" do it "loads the calendar and return its json representation" do - expect(Calendar).to receive(:find).with("bank-holidays").and_return(instance_double(Calendar, to_json: "json_calendar")) + allow(Calendar).to receive(:find).with("bank-holidays").and_return(instance_double(Calendar, to_json: "json_calendar")) get "/bank-holidays.json" expect(response.body).to eq("json_calendar") @@ -83,11 +83,12 @@ end end - context "GET 'division'" do + describe "GET 'division'" do + let(:division) { instance_double(Calendar::Division, to_json: "", events: []) } + let(:calendar) { instance_double(Calendar, division: division) } + before do - @division = instance_double(Calendar::Division, to_json: "", events: []) - @calendar = instance_double(Calendar, division: @division) - allow(Calendar).to receive(:find).and_return(@calendar) + allow(Calendar).to receive(:find).and_return(calendar) stub_content_store_has_item("/a-calendar", { schema_name: "calendar", title: "A calendar with divisions", @@ -97,19 +98,19 @@ end it "returns the json representation of the division" do - expect(@division).to receive(:to_json).and_return("json_division") - expect(@calendar).to receive(:division).with("a-division").and_return(@division) - allow(Calendar).to receive(:find).with("a-calendar").and_return(@calendar) + allow(division).to receive(:to_json).and_return("json_division") + allow(calendar).to receive(:division).with("a-division").and_return(division) + allow(Calendar).to receive(:find).with("a-calendar").and_return(calendar) get "/a-calendar/a-division.json" expect(response.body).to eq("json_division") end it "returns the ics representation of the division" do - expect(@division).to receive(:events).and_return(:some_events) - expect(@calendar).to receive(:division).with("a-division").and_return(@division) - allow(Calendar).to receive(:find).with("a-calendar").and_return(@calendar) - expect(IcsRenderer).to receive(:new).with(:some_events, "/a-calendar/a-division.ics", :en).and_return(instance_double(IcsRenderer, render: "ics_division")) + allow(division).to receive(:events).and_return(:some_events) + allow(calendar).to receive(:division).with("a-division").and_return(division) + allow(Calendar).to receive(:find).with("a-calendar").and_return(calendar) + allow(IcsRenderer).to receive(:new).with(:some_events, "/a-calendar/a-division.ics", :en).and_return(instance_double(IcsRenderer, render: "ics_division")) get "/a-calendar/a-division.ics" expect(response.body).to eq("ics_division") @@ -132,7 +133,7 @@ end it "returns 404 with an invalid division" do - allow(@calendar).to receive(:division).and_raise(Calendar::CalendarNotFound) + allow(calendar).to receive(:division).and_raise(Calendar::CalendarNotFound) get "/a-calendar/foo.json" expect(response).to have_http_status(:not_found) diff --git a/spec/requests/contact_electoral_registration_office_spec.rb b/spec/requests/contact_electoral_registration_office_spec.rb index 9b0f3a02dd..87faea1f7a 100644 --- a/spec/requests/contact_electoral_registration_office_spec.rb +++ b/spec/requests/contact_electoral_registration_office_spec.rb @@ -12,12 +12,12 @@ expect(response).to have_http_status(:ok) expect(response).to render_template("local_transaction/index") expect(response).to render_template(partial: "application/_location_form") - honours_content_store_ttl + expect(response).to honour_content_store_ttl end end context "with postcode params" do - context "that map to a single address" do + context "when they map to a single address" do it "GET show renders results page" do elections_api_stub = stub_api_postcode_lookup("LS11UR", response: api_response) with_electoral_api_url do @@ -26,20 +26,22 @@ expect(response).to have_http_status(:ok) expect(response).to render_template(:results) expect(elections_api_stub).to have_been_requested - honours_content_store_ttl + expect(response).to honour_content_store_ttl end end end - context "that maps to multiple addresses" do + context "when they map to multiple addresses" do before do + allow(ElectoralPresenter).to receive(:new).and_return(presenter) response = { "address_picker" => true, "addresses" => [] }.to_json stub_api_postcode_lookup("IP224DN", response:) end context "and there are no contact details" do + let(:presenter) { instance_double(ElectoralPresenter, show_picker?: true, addresses: []) } + it "GET show renders the address picker template" do - allow_any_instance_of(ElectoralPresenter).to receive(:show_picker?).and_return(true) with_electoral_api_url do get "/contact-electoral-registration-office", params: { postcode: "IP224DN" } @@ -50,8 +52,9 @@ end context "but contact details are present" do + let(:presenter) { instance_double(ElectoralPresenter, show_picker?: false, electoral_services: nil, use_electoral_services_contact_details?: false, use_registration_contact_details?: false) } + it "GET show doesn't render the address picker template" do - allow_any_instance_of(ElectoralPresenter).to receive(:show_picker?).and_return(false) with_electoral_api_url do get "/contact-electoral-registration-office", params: { postcode: "IP224DN" } diff --git a/spec/requests/content_loading_problems_spec.rb b/spec/requests/content_loading_problems_spec.rb index a21706343c..a4b34b6362 100644 --- a/spec/requests/content_loading_problems_spec.rb +++ b/spec/requests/content_loading_problems_spec.rb @@ -1,5 +1,5 @@ RSpec.describe "Content Loading Problems" do - context "loading the homepage without permission" do + context "when loading a draft page without permission" do before do endpoint = content_store_endpoint(draft: false) stub_request(:get, "#{endpoint}/content/").to_return(status: 403, headers: {}) @@ -12,7 +12,7 @@ end end - context "loading /foreign-travel-advice when the content-store is missing" do + context "when the content-store is missing" do before do stub_content_store_isnt_available end diff --git a/spec/requests/find_local_council_spec.rb b/spec/requests/find_local_council_spec.rb index bb881c5d56..5f52d11be5 100644 --- a/spec/requests/find_local_council_spec.rb +++ b/spec/requests/find_local_council_spec.rb @@ -8,7 +8,7 @@ it "sets correct expiry headers" do get "/find-local-council" - honours_content_store_ttl + expect(response).to honour_content_store_ttl end it "returns a 404 if the local authority can't be found" do diff --git a/spec/requests/get_involved_spec.rb b/spec/requests/get_involved_spec.rb index f0c4faf0b5..37d3d74a7d 100644 --- a/spec/requests/get_involved_spec.rb +++ b/spec/requests/get_involved_spec.rb @@ -4,7 +4,7 @@ stub_request(:get, /\A#{Plek.new.find('search-api')}\/search.json/).to_return(body: { "results" => [], "total" => 83 }.to_json) end - context "GET index" do + describe "GET index" do it "responds with success" do get "/government/get-involved" diff --git a/spec/requests/help_spec.rb b/spec/requests/help_spec.rb index e8808161e7..04cc9c537d 100644 --- a/spec/requests/help_spec.rb +++ b/spec/requests/help_spec.rb @@ -8,7 +8,7 @@ end end - context "GET index" do + describe "GET index" do before do content_store_has_random_item(base_path: "/help", schema: "help_page") end @@ -16,11 +16,11 @@ it "sets the cache expiry headers" do get "/help" - honours_content_store_ttl + expect(response).to honour_content_store_ttl end end - context "loading the cookies setting page" do + context "when loading the cookies setting page" do before do content_store_has_random_item(base_path: "/help/cookies", schema: "help_page") end @@ -34,11 +34,11 @@ it "sets the cache expiry headers" do get "/help/cookies" - honours_content_store_ttl + expect(response).to honour_content_store_ttl end end - context "GET ab-testing" do + describe "GET ab-testing" do %w[A B].each do |variant| it "does not affect non-AB-testing pages with the #{variant} variant" do content_store_has_random_item(base_path: "/help", schema: "help_page") @@ -99,7 +99,7 @@ end end - context "GET /:slug" do + describe "GET /:slug" do before do content_store_has_random_item(base_path: "/help/about-govuk", schema: "help_page") content_store_has_random_item(base_path: "/help/cookies", schema: "help_page") @@ -128,7 +128,7 @@ def stub_at_request request_headers = {} @request = double - allow(@request).to receive(:headers).and_return(request_headers) + allow(@request).to receive(:headers).and_return(request_headers) # rubocop:disable RSpec/InstanceVariable request_headers end end diff --git a/spec/requests/homepage_spec.rb b/spec/requests/homepage_spec.rb index 5ef26648ad..2517a5dc17 100644 --- a/spec/requests/homepage_spec.rb +++ b/spec/requests/homepage_spec.rb @@ -2,7 +2,7 @@ include GovukAbTesting::RspecHelpers include ContentStoreHelpers - context "loading the homepage" do + context "when loading the homepage" do before { stub_homepage_content_item } it "responds with success" do @@ -14,7 +14,7 @@ it "sets correct expiry headers" do get "/" - honours_content_store_ttl + expect(response).to honour_content_store_ttl end end end diff --git a/spec/requests/landing_page_spec.rb b/spec/requests/landing_page_spec.rb index ad877e4478..8c633586b4 100644 --- a/spec/requests/landing_page_spec.rb +++ b/spec/requests/landing_page_spec.rb @@ -3,13 +3,14 @@ RSpec.describe "Landing Page" do include GdsApi::TestHelpers::Search - context "GET show" do + describe "GET show" do context "when a content item does exist" do + let(:content_item) { GovukSchemas::Example.find("landing_page", example_name: "landing_page") } + let(:base_path) { content_item.fetch("base_path") } + before do stub_const("LandingPage::ADDITIONAL_CONTENT_PATH", "spec/fixtures") - @content_item = GovukSchemas::Example.find("landing_page", example_name: "landing_page") - @base_path = @content_item.fetch("base_path") - @content_item["details"]["attachments"] = [ + content_item["details"]["attachments"] = [ { "accessible" => false, "attachment_type" => "document", @@ -22,19 +23,19 @@ "url" => "https://ignored-asset-domain/media/000000000000000000000001/data_one.csv", }, ] - stub_content_store_has_item(@base_path, @content_item) + stub_content_store_has_item(base_path, content_item) stub_content_store_has_item(basic_taxon["base_path"], basic_taxon) stub_any_search_to_return_no_results end it "succeeds" do - get @base_path + get base_path expect(response).to have_http_status(:ok) end it "renders the show template" do - get @base_path + get base_path expect(response).to render_template(:show) end diff --git a/spec/requests/licence_transactions_spec.rb b/spec/requests/licence_transactions_spec.rb index ec0c4ca51c..2e4f30a608 100644 --- a/spec/requests/licence_transactions_spec.rb +++ b/spec/requests/licence_transactions_spec.rb @@ -8,7 +8,7 @@ include GdsApi::TestHelpers::LicenceApplication include LocationHelpers - context "GET start" do + describe "GET start" do before do content_store_has_example_item("/find-licences/new-licence", schema: "specialist_document", example: "licence-transaction") end @@ -16,13 +16,13 @@ it "sets the cache expiry headers" do get "/find-licences/new-licence" - honours_content_store_ttl + expect(response).to honour_content_store_ttl end end - context "POST to find" do + describe "POST to find" do before do - @payload = { + payload = { base_path: "/find-licences/new-licence", document_type: "licence_transaction", schema_name: "specialist_document", @@ -36,10 +36,10 @@ }, }, } - stub_content_store_has_item("/find-licences/new-licence", @payload) + stub_content_store_has_item("/find-licences/new-licence", payload) end - context "loading the licence edition when posting a location" do + context "when loading the licence edition and posting a location" do before do stub_licence_exists( "1071-5-1/00BK", @@ -74,7 +74,7 @@ end end - context "GET authority" do + describe "GET authority" do before do content_store_has_example_item("/find-licences/new-licence", schema: "specialist_document", example: "licence-transaction") end @@ -82,7 +82,7 @@ it "sets the cache expiry headers" do get "/find-licences/new-licence/secret-service" - honours_content_store_ttl + expect(response).to honour_content_store_ttl end end end diff --git a/spec/requests/local_transactions_spec.rb b/spec/requests/local_transactions_spec.rb index 45303f1394..7f23ba845d 100644 --- a/spec/requests/local_transactions_spec.rb +++ b/spec/requests/local_transactions_spec.rb @@ -10,19 +10,19 @@ content_store_has_random_item(base_path: "/a-slug", schema: "local_transaction") get "/a-slug" - expect(@response.headers["X-Frame-Options"]).to eq("DENY") + expect(response.headers["X-Frame-Options"]).to eq("DENY") end it "sets expiry headers for an edition" do content_store_has_random_item(base_path: "/a-slug", schema: "local_transaction") get "/a-slug" - honours_content_store_ttl + expect(response).to honour_content_store_ttl end - context "given a local transaction exists in content store" do + context "when a local transaction exists in content store" do before do - @payload_bear = { + payload_bear = { analytics_identifier: nil, base_path: "/pay-bear-tax", content_id: "d6d6caaf-77db-47e1-8206-30cd4f3d0e3f", @@ -55,7 +55,7 @@ }, external_related_links: [], } - @payload_electoral = { + payload_electoral = { analytics_identifier: nil, base_path: "/get-on-electoral-register", content_id: "d6d6caaf-77db-47e1-8206-30cd4f3hwe78", @@ -81,11 +81,11 @@ }, external_related_links: [], } - stub_content_store_has_item("/send-a-bear-to-your-local-council", @payload_bear) - stub_content_store_has_item("/get-on-electoral-register", @payload_electoral) + stub_content_store_has_item("/send-a-bear-to-your-local-council", payload_bear) + stub_content_store_has_item("/get-on-electoral-register", payload_electoral) end - context "loading the local transaction edition without any location" do + context "when loading the local transaction edition without any location" do it "returns the normal content for a page" do get "/send-a-bear-to-your-local-council" @@ -96,12 +96,12 @@ it "sets correct expiry headers" do get "/send-a-bear-to-your-local-council" - honours_content_store_ttl + expect(response).to honour_content_store_ttl end end - context "loading the local transaction when posting a location" do - context "for an English local authority" do + context "when loading the local transaction when posting a location" do + context "and for an English local authority" do before do configure_locations_api_and_local_authority("ST10 4DB", %w[staffordshire staffordshire-moorlands], 3435) post "/send-a-bear-to-your-local-council", params: { postcode: "ST10-4DB] " } @@ -112,7 +112,7 @@ end end - context "for a Northern Ireland local authority" do + context "and for a Northern Ireland local authority" do before do configure_locations_api_and_local_authority("BT1 4QG", %w[belfast], 8132) post "/send-a-bear-to-your-local-council", params: { postcode: "BT1-4QG] " } @@ -123,7 +123,7 @@ end end - context "for electoral registration for an English local authority" do + context "and for electoral registration for an English local authority" do before do configure_locations_api_and_local_authority("ST10 4DB", %w[staffordshire staffordshire-moorlands], 3435) post "/get-on-electoral-register", params: { postcode: "ST10-4DB] " } @@ -134,7 +134,7 @@ end end - context "for electoral registration for a Northern Ireland local authority" do + context "and for electoral registration for a Northern Ireland local authority" do before do stub_locations_api_has_location("BT1 3QG", [{ "local_custodian_code" => 8132 }]) stub_local_links_manager_has_a_local_authority("belfast", country_name: "Northern Ireland", local_custodian_code: 8132) @@ -147,7 +147,7 @@ end end - context "loading the local transaction when posting an invalid postcode" do + context "when loading the local transaction when posting an invalid postcode" do before do stub_locations_api_does_not_have_a_bad_postcode("BLAH") post "/send-a-bear-to-your-local-council", params: { postcode: "BLAH" } @@ -159,7 +159,7 @@ end end - context "loading the local transaction when posting a postcode with no matching areas" do + context "when loading the local transaction when posting a postcode with no matching areas" do before do stub_locations_api_has_no_location("WC1E 9ZZ") post "/send-a-bear-to-your-local-council", params: { postcode: "WC1E 9ZZ" } @@ -171,7 +171,7 @@ end end - context "loading the local transaction when posting a location that has no matching local authority" do + context "when loading the local transaction when posting a location that has no matching local authority" do before do stub_locations_api_has_location("AB1 2CD", [{ "local_custodian_code" => 123 }]) stub_local_links_manager_does_not_have_a_custodian_code(123) @@ -184,7 +184,7 @@ end end - context "loading the local transaction for an authority" do + context "when loading the local transaction for an authority" do before do configure_locations_api_and_local_authority("ST10 4DB", %w[staffordshire-moorlands], 3435) stub_local_links_manager_has_a_link( @@ -219,9 +219,9 @@ end end - context "loading a local transaction without an interaction that exists in content store" do + context "when loading a local transaction without an interaction that exists in content store" do before do - @payload = { + payload = { analytics_identifier: nil, base_path: "/report-a-bear-on-a-local-road", content_id: "d6d6caaf-77db-47e1-8206-30cd4f3d0e3f", @@ -249,7 +249,7 @@ } configure_locations_api_and_local_authority("ST10 4DB", %w[staffordshire-moorlands], 3435) stub_local_links_manager_has_no_link(authority_slug: "staffordshire-moorlands", lgsl: 1234, lgil: 1, country_name: "England") - stub_content_store_has_item("/report-a-bear-on-a-local-road", @payload) + stub_content_store_has_item("/report-a-bear-on-a-local-road", payload) get "/report-a-bear-on-a-local-road/staffordshire-moorlands" end @@ -281,7 +281,7 @@ country_name: "England", status: "ok", ) - @payload = { + payload = { base_path: "/pay-bear-tax", document_type: "local_transaction", format: "local_transaction", @@ -294,7 +294,7 @@ "Information about paying local tax on owning or looking after a bear.", }, } - stub_content_store_has_item("/pay-bear-tax", @payload) + stub_content_store_has_item("/pay-bear-tax", payload) end it "redirects to the correct authority and pass cache and token as params" do diff --git a/spec/requests/placeholder_spec.rb b/spec/requests/placeholder_spec.rb index fbef987579..636a66dd85 100644 --- a/spec/requests/placeholder_spec.rb +++ b/spec/requests/placeholder_spec.rb @@ -1,5 +1,5 @@ RSpec.describe "Placeholder" do - context "loading the placeholder page" do + context "when loading the placeholder page" do it "responds with success" do stub_content_store_does_not_have_item("/government") stub_content_store_does_not_have_item("/government/placeholder") diff --git a/spec/requests/places_spec.rb b/spec/requests/places_spec.rb index 61d99d4386..4a5e2102e9 100644 --- a/spec/requests/places_spec.rb +++ b/spec/requests/places_spec.rb @@ -36,26 +36,26 @@ stub_places_manager_places_request("slug", query_hash, return_data, 400) end - context "GET 'a place content item'" do + describe "GET 'a place content item'" do it "sets the cache expiry headers" do get "/slug" - honours_content_store_ttl + expect(response).to honour_content_store_ttl end it "does not show location error" do get "/slug" - expect(@controller.view_assigns["location_error"]).to be_nil + expect(controller.view_assigns["location_error"]).to be_nil end end - context "POST 'a postcode'" do + describe "POST 'a postcode'" do context "with valid postcode" do it "does not show location error" do post "/slug", params: { postcode: valid_postcode } - expect(@controller.view_assigns["location_error"]).to be_nil + expect(controller.view_assigns["location_error"]).to be_nil end end @@ -63,7 +63,7 @@ it "shows location error" do post "/slug", params: { postcode: invalid_postcode } - expect(LocationError.new(PlacesManagerResponse::INVALID_POSTCODE).postcode_error).to eq(@controller.view_assigns["location_error"].postcode_error) + expect(controller.view_assigns["location_error"].postcode_error).to eq(LocationError.new(PlacesManagerResponse::INVALID_POSTCODE).postcode_error) end end end diff --git a/spec/requests/roadmap_spec.rb b/spec/requests/roadmap_spec.rb index f73fa45a5c..f785d509e4 100644 --- a/spec/requests/roadmap_spec.rb +++ b/spec/requests/roadmap_spec.rb @@ -1,5 +1,5 @@ RSpec.describe "Roadmap" do - context "GET index" do + describe "GET index" do before do content_store_has_random_item(base_path: "/roadmap", schema: "special_route") end diff --git a/spec/requests/sessions_spec.rb b/spec/requests/sessions_spec.rb index 78d39188a7..a4d1bf558b 100644 --- a/spec/requests/sessions_spec.rb +++ b/spec/requests/sessions_spec.rb @@ -4,7 +4,7 @@ include GdsApi::TestHelpers::AccountApi include GovukPersonalisation::TestHelpers::Requests - context "GET /account" do + describe "GET /account" do before { stub_account_api_get_sign_in_url } it "redirects the user to the GOV.UK Account service domain" do @@ -19,68 +19,62 @@ expect(response).to redirect_to("https://home.account.gov.uk?_ga=ga123") end - context "HTTP Referer" do + context "with an HTTP Referer" do + let!(:stub_without_redirect_path) { stub_account_api_get_sign_in_url } + let!(:stub_with_redirect_path) { stub_account_api_get_sign_in_url(redirect_path:) } + context "with a referer on GOV.UK" do - before do - @redirect_path = "/transition-check/results?c[]=import-wombats&c[]=practice-wizardry" - @referer = "#{Plek.new.website_root}#{@redirect_path}" - setup_referer - end + let(:referer) { "#{Plek.new.website_root}#{redirect_path}" } + let(:redirect_path) { "/transition-check/results?c[]=import-wombats&c[]=practice-wizardry" } it "uses the path from the referer as the redirect_path" do - get "/account", headers: { "Referer" => @referer } + get "/account", headers: { "Referer" => referer } expect(response).to have_http_status(:redirect) - expect(@stub_with_redirect_path).to have_been_requested - expect(@stub_without_redirect_path).not_to have_been_requested + expect(stub_with_redirect_path).to have_been_requested + expect(stub_without_redirect_path).not_to have_been_requested end end context "with a protocol-relative redirect" do - before do - @referer = "//protocol-relative-path" - @redirect_path = @referer - setup_referer - end + let(:referer) { "//protocol-relative-path" } + let(:redirect_path) { referer } it "does not take the redirect_path from the referer" do - get "/account", headers: { "Referer" => @referer } + get "/account", headers: { "Referer" => referer } - expect(@stub_with_redirect_path).not_to have_been_requested + expect(stub_with_redirect_path).not_to have_been_requested expect(response).to redirect_to("https://home.account.gov.uk") end end context "with a referer from outside GOV.UK" do - before do - @referer = "https://www.some-other-website.gov.uk/path" - @redirect_path = "/path" - setup_referer - end + let(:referer) { "https://www.some-other-website.gov.uk/path" } + let(:redirect_path) { "/path" } it "does not take the redirect_path from the referer" do - get "/account", headers: { "Referer" => @referer } + get "/account", headers: { "Referer" => referer } - expect(@stub_with_redirect_path).not_to have_been_requested + expect(stub_with_redirect_path).not_to have_been_requested expect(response).to redirect_to("https://home.account.gov.uk") end end end end - context "GET /sign-in/callback" do + describe "GET /sign-in/callback" do before { @root_path = Plek.new.website_root } it "redirects to the root path if the :code param is not present" do get "/sign-in/callback", params: { state: "state123" } - expect(response).to redirect_to(@response.redirect_url) + expect(response).to redirect_to(response.redirect_url) end it "redirects to the root path if the :state param is not present" do get "/sign-in/callback", params: { code: "code123" } - expect(response).to redirect_to(@response.redirect_url) + expect(response).to redirect_to(response.redirect_url) end it "responds with :bad_request if :code or :state are invalid" do @@ -90,36 +84,41 @@ expect(response).to have_http_status(:bad_request) end - context "the :code and :state are valid" do + context "when the :code and :state are valid" do + let(:govuk_account_session) { "new-session-id" } + let(:redirect_path) { nil } + let(:cookie_consent) { true } + let(:feedback_consent) { true } + before do - @govuk_account_session = "new-session-id" - @redirect_path = nil - @cookie_consent = true - @feedback_consent = true + stub_account_api_validates_auth_response( + govuk_account_session:, + redirect_path:, + cookie_consent:, + feedback_consent:, + ) end context "with no extra parameters" do - before { stub_account_api } - it "sets the 'GOVUK-Account-Session' header" do get "/sign-in/callback", params: { code: "code123", state: "state123" } - expect(@govuk_account_session).to eq(@response.headers["GOVUK-Account-Session"]) + expect(response.headers["GOVUK-Account-Session"]).to eq(govuk_account_session) end it "redirects to the account home path" do get "/sign-in/callback", params: { code: "code123", state: "state123" } expect(response).to have_http_status(:redirect) - expect(account_home_url).to eq(@response.redirect_url) + expect(account_home_url).to eq(response.redirect_url) end - context "cookie consent is passed by query param" do + context "when cookie consent is passed by query param" do it "redirects to the account home path with cookie_consent=accept if given" do get "/sign-in/callback", params: { code: "code123", state: "state123", cookie_consent: "accept" } expect(response).to have_http_status(:redirect) - expect("#{account_home_url}?cookie_consent=accept").to eq(@response.redirect_url) + expect("#{account_home_url}?cookie_consent=accept").to eq(response.redirect_url) end end @@ -128,87 +127,76 @@ get "/sign-in/callback", params: { code: "code123", state: "state123", _ga: underscore_ga } expect(response).to have_http_status(:redirect) - assert_includes(@response.redirect_url, underscore_ga) + expect(response.redirect_url).to include(underscore_ga) end end - context "account-api returns a :redirect_path" do - before do - @redirect_path = "/bank-holiday" - stub_account_api - end + context "when account-api returns a :redirect_path" do + let(:redirect_path) { "/bank-holiday" } it "redirects to the redirect path" do get "/sign-in/callback", params: { code: "code123", state: "state123" } expect(response).to have_http_status(:redirect) - assert_includes(@response.redirect_url, @redirect_path) + expect(response.redirect_url).to include(redirect_path) end - context "the redirect path has a querystring" do - before do - @redirect_path = "/email/subscriptions/account/confirm?frequency=immediately&return_to_url=true&topic_id=some-page-with-notifications" - stub_account_api - end + context "when the redirect path has a querystring" do + let(:redirect_path) { "/email/subscriptions/account/confirm?frequency=immediately&return_to_url=true&topic_id=some-page-with-notifications" } it "preserves the querystring" do get "/sign-in/callback", params: { code: "code123", state: "state123" } expect(response).to have_http_status(:redirect) - assert_includes(@response.redirect_url, @redirect_path) + expect(response.redirect_url).to include(redirect_path) end end end - context "account-api returns a nil :cookie_consent" do - before do - @cookie_consent = nil - @redirect_path = "/bank-holiday" - stub_account_api - end + context "when account-api returns a nil :cookie_consent" do + let(:cookie_consent) { nil } + let(:redirect_path) { "/bank-holiday" } it "signs the user in properly" do get "/sign-in/callback", params: { code: "code123", state: "state123" } - expect(@govuk_account_session).to eq(@response.headers["GOVUK-Account-Session"]) + expect(response.headers["GOVUK-Account-Session"]).to eq(govuk_account_session) end it "redirects to the redirect_path" do get "/sign-in/callback", params: { code: "code123", state: "state123" } expect(response).to have_http_status(:redirect) - assert_includes(@response.redirect_url, @redirect_path) + expect(response.redirect_url).to include(redirect_path) end end - context "account-api returns a nil :feedback_consent" do - before do - @feedback_consent = nil - @redirect_path = "/bank-holiday" - stub_account_api - end + context "when account-api returns a nil :feedback_consent" do + let(:feedback_consent) { nil } + let(:redirect_path) { "/bank-holiday" } it "signs the user in properly" do get "/sign-in/callback", params: { code: "code123", state: "state123" } - expect(@govuk_account_session).to eq(@response.headers["GOVUK-Account-Session"]) + expect(response.headers["GOVUK-Account-Session"]).to eq(govuk_account_session) end it "redirects to the redirect_path" do get "/sign-in/callback", params: { code: "code123", state: "state123" } expect(response).to have_http_status(:redirect) - assert_includes(@response.redirect_url, @redirect_path) + expect(response.redirect_url).to include(redirect_path) end end end end - context "GET /sign-out" do + describe "GET /sign-out" do context "when the user is logged in" do + let(:end_session_uri) { "https://authentication-provider/end-session" } + before do - @end_session_uri = "https://authentication-provider/end-session" - stub_account_api_get_end_session_url(end_session_uri: @end_session_uri) + stub_account_api_get_end_session_url(end_session_uri:) end # TODO: The request module for govuk_personalisation doesn't support requests correctly, @@ -224,22 +212,8 @@ get "/sign-out" expect(response).to have_http_status(:redirect) - expect(@end_session_uri).to eq(@response.redirect_url) + expect(response.redirect_url).to eq(end_session_uri) end end end - - def setup_referer - @stub_without_redirect_path = stub_account_api_get_sign_in_url - @stub_with_redirect_path = stub_account_api_get_sign_in_url(redirect_path: @redirect_path) - end - - def stub_account_api - stub_account_api_validates_auth_response( - govuk_account_session: @govuk_account_session, - redirect_path: @redirect_path, - cookie_consent: @cookie_consent, - feedback_consent: @feedback_consent, - ) - end end diff --git a/spec/requests/simple_smart_answers_spec.rb b/spec/requests/simple_smart_answers_spec.rb index 5d7cb933ff..93735af609 100644 --- a/spec/requests/simple_smart_answers_spec.rb +++ b/spec/requests/simple_smart_answers_spec.rb @@ -10,21 +10,21 @@ def simple_smart_answer_content_item } end - context "GET 'start page'" do + describe "GET 'start page'" do before do content_store_has_random_item(base_path: "/the-bridge-of-death", schema: "simple_smart_answer") end it "sets the cache expiry headers" do get "/the-bridge-of-death" - honours_content_store_ttl + expect(response).to honour_content_store_ttl end end - context "GET 'question flow'" do - context "for a simple_smart_answer slug" do - before do - @node_details = [ + describe "GET 'question flow'" do + context "with a simple_smart_answer slug" do + let(:node_details) do + [ { "kind" => "question", "slug" => "question-1", @@ -79,22 +79,25 @@ def simple_smart_answer_content_item "body" => "

This is outcome 2

", }, ] + end + + before do payload = simple_smart_answer_content_item.merge( details: { start_button_text: "Start here", body: "Hello", - nodes: @node_details, + nodes: node_details, }, ) stub_content_store_has_item("/the-bridge-of-death", payload) end it "calculates the flow state with no responses" do - flow = SimpleSmartAnswers::Flow.new(@node_details) + flow = SimpleSmartAnswers::Flow.new(node_details) state = flow.state_for_responses([]) allow(SimpleSmartAnswers::Flow).to receive(:new).with( - @node_details, + node_details, ).and_return(flow) allow(flow).to receive(:state_for_responses).with([]).and_return(state) get "/the-bridge-of-death/y" @@ -103,10 +106,10 @@ def simple_smart_answer_content_item end it "calculates the flow state for the given responses" do - flow = SimpleSmartAnswers::Flow.new(@node_details) + flow = SimpleSmartAnswers::Flow.new(node_details) state = flow.state_for_responses(%w[option-1 option-2]) allow(SimpleSmartAnswers::Flow).to receive(:new).with( - @node_details, + node_details, ).and_return(flow) allow(flow).to receive(:state_for_responses).with( %w[option-1 option-2], @@ -125,7 +128,7 @@ def simple_smart_answer_content_item it "sets cache control headers" do get "/the-bridge-of-death/y/option-1/option-2" - honours_content_store_ttl + expect(response).to honour_content_store_ttl end context "with form submission params" do @@ -145,7 +148,7 @@ def simple_smart_answer_content_item it "sets cache control headers when redirecting" do get "/the-bridge-of-death/y/option-1", params: { response: "option-2" } - honours_content_store_ttl + expect(response).to honour_content_store_ttl end it "does not redirect if the form submission results in an error" do diff --git a/spec/requests/static_error_pages_spec.rb b/spec/requests/static_error_pages_spec.rb index a8af3d5c20..29535d86f0 100644 --- a/spec/requests/static_error_pages_spec.rb +++ b/spec/requests/static_error_pages_spec.rb @@ -1,5 +1,5 @@ RSpec.describe "Static Error Pages" do - context "When asked for an unrecognised error" do + context "when asked for an unrecognised error" do it "returns a 404 with no body" do get "/static-error-pages/555.html" diff --git a/spec/requests/take_part_spec.rb b/spec/requests/take_part_spec.rb index 68cec2e2a0..99c939d2fc 100644 --- a/spec/requests/take_part_spec.rb +++ b/spec/requests/take_part_spec.rb @@ -3,7 +3,7 @@ content_store_has_example_item("/government/get-involved/take-part/tp1", schema: :take_part) end - context "GET index" do + describe "GET index" do it "returns 200" do get "/government/get-involved/take-part/tp1" diff --git a/spec/requests/transactions_spec.rb b/spec/requests/transactions_spec.rb index 8f419a2674..74b7f8cf9c 100644 --- a/spec/requests/transactions_spec.rb +++ b/spec/requests/transactions_spec.rb @@ -1,27 +1,25 @@ RSpec.describe "Transactions" do - context "GET show" do + describe "GET show" do before do - @content_item = content_store_has_example_item("/foo", schema: "transaction") + content_store_has_example_item("/foo", schema: "transaction") end it "sets the cache expiry headers" do get "/foo" - honours_content_store_ttl + expect(response).to honour_content_store_ttl end it "does not allow framing of transaction pages" do content_store_has_example_item("/foo", schema: "transaction") get "/foo" - expect(@response.headers["X-Frame-Options"]).to eq("DENY") + expect(response.headers["X-Frame-Options"]).to eq("DENY") end end - context "loading the jobsearch page" do - before do - @content_item = content_store_has_example_item("/jobsearch", schema: "transaction", example: "jobsearch") - end + context "when loading the jobsearch page" do + let!(:content_item) { content_store_has_example_item("/jobsearch", schema: "transaction", example: "jobsearch") } it "responds with success" do get "/jobsearch" @@ -32,17 +30,17 @@ it "loads the correct details" do get "/jobsearch" - expect(assigns(:content_item).title).to eq(@content_item["title"]) + expect(assigns(:content_item).title).to eq(content_item["title"]) end it "sets correct expiry headers" do get "/jobsearch" - honours_content_store_ttl + expect(response).to honour_content_store_ttl end end - context "given a welsh version exists" do + context "when a welsh version exists" do before do content_store_has_example_item("/chwilio-am-swydd", schema: "transaction", example: "chwilio-am-swydd") end @@ -55,22 +53,20 @@ end end - context "given a variant exists" do - before do - @content_item = content_store_has_example_item("/foo", schema: "transaction", example: "transaction-with-variants") - end + context "when a variant exists" do + let!(:content_item) { content_store_has_example_item("/foo", schema: "transaction", example: "transaction-with-variants") } it "displays variant specific values where present" do get "/foo", params: { variant: "council-tax-bands-2-staging" } - expect(assigns(:content_item).title).to eq(@content_item.dig("details", "variants", 0, "title")) - expect(assigns(:content_item).transaction_start_link).to eq(@content_item.dig("details", "variants", 0, "transaction_start_link")) + expect(assigns(:content_item).title).to eq(content_item.dig("details", "variants", 0, "title")) + expect(assigns(:content_item).transaction_start_link).to eq(content_item.dig("details", "variants", 0, "transaction_start_link")) end it "displays normal value where variant does not specify value" do get "/foo", params: { variant: "council-tax-bands-2-staging" } - expect(assigns(:content_item).more_information).to eq(@content_item.dig("details", "more_information")) + expect(assigns(:content_item).more_information).to eq(content_item.dig("details", "more_information")) end end end diff --git a/spec/requests/travel_advice_spec.rb b/spec/requests/travel_advice_spec.rb index 5d76ffcf76..1895d6dfb3 100644 --- a/spec/requests/travel_advice_spec.rb +++ b/spec/requests/travel_advice_spec.rb @@ -1,33 +1,34 @@ RSpec.describe "Travel Advice" do - context "GET index" do - context "given countries exist" do + describe "GET index" do + context "when countries exist" do + let(:content_item) { GovukSchemas::Example.find("travel_advice_index", example_name: "index") } + let(:base_path) { content_item.fetch("base_path") } + before do - @content_item = GovukSchemas::Example.find("travel_advice_index", example_name: "index") - @base_path = @content_item.fetch("base_path") - stub_content_store_has_item(@base_path, @content_item) + stub_content_store_has_item(base_path, content_item) end it "succeeds" do - get @base_path + get base_path expect(response).to have_http_status(:ok) end it "renders the index template" do - get @base_path + get base_path expect(response).to render_template(:index) end it "sets cache-control headers" do - get @base_path - honours_content_store_ttl + get base_path + expect(response).to honour_content_store_ttl end - context "requesting atom" do + context "when requesting atom" do before do - atom_base_path = "#{@base_path}.atom" - stub_content_store_has_item(atom_base_path, @content_item) + atom_base_path = "#{base_path}.atom" + stub_content_store_has_item(atom_base_path, content_item) get atom_base_path end @@ -47,75 +48,66 @@ end end - context "GET show" do - context "first part" do - before do - @content_item = GovukSchemas::Example.find("travel_advice", example_name: "full-country") - @country_path = @content_item.fetch("base_path") - stub_content_store_has_item(@country_path, @content_item) - end + describe "GET show" do + let(:content_item) { GovukSchemas::Example.find("travel_advice", example_name: "full-country") } + let(:country_path) { content_item.fetch("base_path") } + before do + stub_content_store_has_item(country_path, content_item) + end + + context "when viewing the first part" do it "succeeds" do - get @country_path + get country_path expect(response).to have_http_status(:ok) end it "renders the show template" do - get @country_path + get country_path expect(response).to render_template(:show) end it "renders the print variant" do - get "#{@country_path}/print" + get "#{country_path}/print" expect(response).to render_template(:show) end it "sets cache-control headers" do - get @country_path - honours_content_store_ttl + get country_path + + expect(response).to honour_content_store_ttl end end - context "second part" do - before do - @content_item = GovukSchemas::Example.find("travel_advice", example_name: "full-country") - @country_path = @content_item.fetch("base_path") - @part_slug = @content_item.dig("details", "parts").last["slug"] - stub_content_store_has_item(@country_path, @content_item) - end + context "when viewing the second part" do + let(:part_slug) { content_item.dig("details", "parts").last["slug"] } it "succeeds" do - get "#{@country_path}/#{@part_slug}" + get "#{country_path}/#{part_slug}" expect(response).to have_http_status(:ok) end it "renders the show template" do - get "#{@country_path}/#{@part_slug}" + get "#{country_path}/#{part_slug}" expect(response).to render_template(:show) end it "sets cache-control headers" do - get "#{@country_path}/#{@part_slug}" - honours_content_store_ttl + get "#{country_path}/#{part_slug}" + expect(response).to honour_content_store_ttl end end - context "missing part" do - before do - @content_item = GovukSchemas::Example.find("travel_advice", example_name: "full-country") - @country_path = @content_item.fetch("base_path") - stub_content_store_has_item(@country_path, @content_item) - end - + context "when viewing a missing part" do it "redirects to the base_path if the part doesn't exist" do - get "#{@country_path}/i-dont-exist" + get "#{country_path}/i-dont-exist" - expect(response).to redirect_to(@country_path) + expect(response).to redirect_to(country_path) end end end diff --git a/spec/unit/presenters/simple_smart_answer_presenter_spec.rb b/spec/unit/presenters/simple_smart_answer_presenter_spec.rb index cdfa9103e8..780f710112 100644 --- a/spec/unit/presenters/simple_smart_answer_presenter_spec.rb +++ b/spec/unit/presenters/simple_smart_answer_presenter_spec.rb @@ -1,6 +1,6 @@ RSpec.describe SimpleSmartAnswerPresenter do def subject(content_item) - described_class.new(content_item.deep_stringify_keys!) + described_class.new(content_item) end before do From 52668ef3e00af86485151aeb6b006d416d6c9be9 Mon Sep 17 00:00:00 2001 From: Keith Lawrence Date: Mon, 16 Dec 2024 10:14:44 +0000 Subject: [PATCH 11/19] lint: fix spec/routing --- spec/routing/simple_smart_answers_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/routing/simple_smart_answers_spec.rb b/spec/routing/simple_smart_answers_spec.rb index 346abb3b4d..dec179437e 100644 --- a/spec/routing/simple_smart_answers_spec.rb +++ b/spec/routing/simple_smart_answers_spec.rb @@ -5,13 +5,13 @@ content_store_has_random_item(base_path: "/fooey", schema: "simple_smart_answer") end - context "for the start page" do + context "when accessing the start page" do it "routes the start page to the SimpleSmartAnswer controller" do expect(get("/fooey")).to route_to(controller: "simple_smart_answers", action: "show", slug: "fooey") end end - context "routes in a flow" do + context "with routes in a flow" do it "routes to the start of a flow" do expect(get("/fooey/y")).to route_to(controller: "simple_smart_answers", action: "flow", slug: "fooey") end From 3c8251ce9c2afdfd17d5423fe4340e8f19997da6 Mon Sep 17 00:00:00 2001 From: Keith Lawrence Date: Mon, 16 Dec 2024 10:46:23 +0000 Subject: [PATCH 12/19] lint: fix spec/support - Also rename withdrawal.rb to withdrawable.rb to match the actual concern name. --- spec/support/concerns/parts.rb | 50 ++++++++++++--------------- spec/support/concerns/withdrawable.rb | 7 ++++ spec/support/concerns/withdrawal.rb | 9 ----- spec/support/meta_tags.rb | 8 ++--- 4 files changed, 33 insertions(+), 41 deletions(-) create mode 100644 spec/support/concerns/withdrawable.rb delete mode 100644 spec/support/concerns/withdrawal.rb diff --git a/spec/support/concerns/parts.rb b/spec/support/concerns/parts.rb index bbd75593ac..1f08f69305 100644 --- a/spec/support/concerns/parts.rb +++ b/spec/support/concerns/parts.rb @@ -1,59 +1,53 @@ RSpec.shared_examples "it has parts" do |document_type, example_name| - before do - @content_store_response = GovukSchemas::Example.find(document_type, example_name:) - @part_slug = @content_store_response.dig("details", "parts").last["slug"] - end + subject(:content_item_with_parts) { described_class.new(content_store_response) } + + let(:content_store_response) { GovukSchemas::Example.find(document_type, example_name:) } + let(:part_slug) { content_store_response.dig("details", "parts").last["slug"] } it "gets all parts" do - expected_parts_count = @content_store_response.dig("details", "parts").count - expect(described_class.new(@content_store_response).parts.count).to eq(expected_parts_count) + expected_parts_count = content_store_response.dig("details", "parts").count + expect(content_item_with_parts.parts.count).to eq(expected_parts_count) end it "gets the current part title" do - content_item = described_class.new(@content_store_response) - content_item.set_current_part(@part_slug) - expected_title = @content_store_response.dig("details", "parts").last["title"] + content_item_with_parts.set_current_part(part_slug) + expected_title = content_store_response.dig("details", "parts").last["title"] - expect(content_item.current_part_title).to eq(expected_title) + expect(content_item_with_parts.current_part_title).to eq(expected_title) end it "gets the current part body" do - content_item = described_class.new(@content_store_response) - content_item.set_current_part(@part_slug) - expected_body = @content_store_response.dig("details", "parts").last["body"] + content_item_with_parts.set_current_part(part_slug) + expected_body = content_store_response.dig("details", "parts").last["body"] - expect(content_item.current_part_body).to eq(expected_body) + expect(content_item_with_parts.current_part_body).to eq(expected_body) end it "gets the next part" do - content_item = described_class.new(@content_store_response) - part_slug = @content_store_response.dig("details", "parts").first["slug"] - content_item.set_current_part(part_slug) + first_part_slug = content_store_response.dig("details", "parts").first["slug"] + content_item_with_parts.set_current_part(first_part_slug) - expect(content_item.next_part["slug"]).not_to eq(part_slug) + expect(content_item_with_parts.next_part["slug"]).not_to eq(first_part_slug) end it "gets the previous part" do - content_item = described_class.new(@content_store_response) - content_item.set_current_part(@part_slug) + content_item_with_parts.set_current_part(part_slug) - expect(content_item.previous_part["slug"]).not_to eq(@part_slug) + expect(content_item_with_parts.previous_part["slug"]).not_to eq(part_slug) end describe "#first_part?" do it "returns true if the current part is the first part" do - content_item = described_class.new(@content_store_response) - part_slug = @content_store_response.dig("details", "parts").first["slug"] - content_item.set_current_part(part_slug) + first_part_slug = content_store_response.dig("details", "parts").first["slug"] + content_item_with_parts.set_current_part(first_part_slug) - expect(content_item.first_part?).to be true + expect(content_item_with_parts.first_part?).to be true end it "returns false if the current part is the first part" do - content_item = described_class.new(@content_store_response) - content_item.set_current_part(@part_slug) + content_item_with_parts.set_current_part(part_slug) - expect(content_item.first_part?).to be false + expect(content_item_with_parts.first_part?).to be false end end end diff --git a/spec/support/concerns/withdrawable.rb b/spec/support/concerns/withdrawable.rb new file mode 100644 index 0000000000..df14f635ed --- /dev/null +++ b/spec/support/concerns/withdrawable.rb @@ -0,0 +1,7 @@ +RSpec.shared_examples "it can be withdrawn" do |document_type, example_name| + let(:content_store_response) { GovukSchemas::Example.find(document_type, example_name:) } + + it "knows it's withdrawn" do + expect(described_class.new(content_store_response).withdrawn?).to be true + end +end diff --git a/spec/support/concerns/withdrawal.rb b/spec/support/concerns/withdrawal.rb deleted file mode 100644 index 4c61cc52fb..0000000000 --- a/spec/support/concerns/withdrawal.rb +++ /dev/null @@ -1,9 +0,0 @@ -RSpec.shared_examples "it can be withdrawn" do |document_type, example_name| - before do - @content_store_response = GovukSchemas::Example.find(document_type, example_name:) - end - - it "knows it's withdrawn" do - expect(described_class.new(@content_store_response).withdrawn?).to be true - end -end diff --git a/spec/support/meta_tags.rb b/spec/support/meta_tags.rb index ef90b2e75a..8c72a78656 100644 --- a/spec/support/meta_tags.rb +++ b/spec/support/meta_tags.rb @@ -13,7 +13,7 @@ it "renders the correct meta tags for the title" do visit path - expect(page).to have_css("meta[property='og:title'][content='Zhe title']", visible: false) + expect(page).to have_css("meta[property='og:title'][content='Zhe title']", visible: :hidden) end end @@ -34,8 +34,8 @@ it "renders image tags" do visit path - expect(page).to have_css("meta[name='twitter:card'][content='summary_large_image']", visible: false) - expect(page).to have_css("meta[name='twitter:image'][content='https://example.org/image.jpg']", visible: false) - expect(page).to have_css("meta[property='og:image'][content='https://example.org/image.jpg']", visible: false) + expect(page).to have_css("meta[name='twitter:card'][content='summary_large_image']", visible: :hidden) + expect(page).to have_css("meta[name='twitter:image'][content='https://example.org/image.jpg']", visible: :hidden) + expect(page).to have_css("meta[property='og:image'][content='https://example.org/image.jpg']", visible: :hidden) end end From 35df408c7f70a943e658cfb9ffd1ab1096d8164a Mon Sep 17 00:00:00 2001 From: Keith Lawrence Date: Wed, 8 Jan 2025 15:34:44 +0000 Subject: [PATCH 13/19] lint: fix spec/system --- spec/system/account_home_spec.rb | 2 +- spec/system/bank_holidays_spec.rb | 30 +++++++++---------- spec/system/electoral_look_up_spec.rb | 18 ++++++----- spec/system/find_local_council_spec.rb | 16 +++++----- spec/system/gwyliau_banc_spec.rb | 24 +++++++-------- spec/system/help_cookies_spec.rb | 2 +- spec/system/help_page_spec.rb | 2 +- spec/system/help_spec.rb | 2 +- spec/system/homepage_spec.rb | 2 +- spec/system/icalendar_spec.rb | 2 +- spec/system/json_spec.rb | 2 +- spec/system/landing_page_spec.rb | 11 +++---- spec/system/licence_transaction_spec.rb | 16 +++++----- spec/system/local_transactions_spec.rb | 6 ++-- spec/system/phase_banner_spec.rb | 4 +-- spec/system/place_spec.rb | 14 ++++----- spec/system/sessions_spec.rb | 2 +- spec/system/sign_in_spec.rb | 2 +- spec/system/simple_smart_answers_spec.rb | 2 +- spec/system/static_error_page_spec.rb | 4 +-- spec/system/transaction_spec.rb | 18 +++++------ spec/system/travel_advice_atom_spec.rb | 4 +-- spec/system/travel_advice_spec.rb | 12 ++++---- spec/system/when_do_the_clocks_change_spec.rb | 10 +++---- 24 files changed, 106 insertions(+), 101 deletions(-) diff --git a/spec/system/account_home_spec.rb b/spec/system/account_home_spec.rb index 54d717f519..c247802cb8 100644 --- a/spec/system/account_home_spec.rb +++ b/spec/system/account_home_spec.rb @@ -3,7 +3,7 @@ before { stub_homepage_content_item } - context "/account/home" do + describe "/account/home" do it "redirects users to One Login's Your Services page" do visit account_home_path diff --git a/spec/system/bank_holidays_spec.rb b/spec/system/bank_holidays_spec.rb index 25026cf7bd..e0bd10f665 100644 --- a/spec/system/bank_holidays_spec.rb +++ b/spec/system/bank_holidays_spec.rb @@ -11,7 +11,7 @@ mock_calendar_fixtures end - context "AB testing spacing" do + context "when AB testing spacing" do it "has spacing for the B variant" do Timecop.travel("2012-12-14") with_variant(BankHolidaysTest: "B") do @@ -39,17 +39,17 @@ Timecop.travel("2012-12-14") visit "/bank-holidays" - within("head", visible: false) do - expect(page).to have_selector("title", text: "UK bank holidays - GOV.UK", visible: false) - desc = page.find("meta[name=description]", visible: false) + within("head", visible: :hidden) do + expect(page).to have_selector("title", text: "UK bank holidays - GOV.UK", visible: :hidden) + desc = page.find("meta[name=description]", visible: :hidden) expect(desc["content"]).to eq("Find out when bank holidays are in England, Wales, Scotland and Northern Ireland - including past and future bank holidays") - expect(page).to have_selector("link[rel=alternate][type='application/json'][href='/bank-holidays.json']", visible: false) - expect(page).to have_selector("link[rel=alternate][type='application/json'][href='/bank-holidays/england-and-wales.json']", visible: false) - expect(page).to have_selector("link[rel=alternate][type='text/calendar'][href='/bank-holidays/england-and-wales.ics']", visible: false) - expect(page).to have_selector("link[rel=alternate][type='application/json'][href='/bank-holidays/scotland.json']", visible: false) - expect(page).to have_selector("link[rel=alternate][type='text/calendar'][href='/bank-holidays/scotland.ics']", visible: false) - expect(page).to have_selector("link[rel=alternate][type='application/json'][href='/bank-holidays/northern-ireland.json']", visible: false) - expect(page).to have_selector("link[rel=alternate][type='text/calendar'][href='/bank-holidays/northern-ireland.ics']", visible: false) + expect(page).to have_selector("link[rel=alternate][type='application/json'][href='/bank-holidays.json']", visible: :hidden) + expect(page).to have_selector("link[rel=alternate][type='application/json'][href='/bank-holidays/england-and-wales.json']", visible: :hidden) + expect(page).to have_selector("link[rel=alternate][type='text/calendar'][href='/bank-holidays/england-and-wales.ics']", visible: :hidden) + expect(page).to have_selector("link[rel=alternate][type='application/json'][href='/bank-holidays/scotland.json']", visible: :hidden) + expect(page).to have_selector("link[rel=alternate][type='text/calendar'][href='/bank-holidays/scotland.ics']", visible: :hidden) + expect(page).to have_selector("link[rel=alternate][type='application/json'][href='/bank-holidays/northern-ireland.json']", visible: :hidden) + expect(page).to have_selector("link[rel=alternate][type='text/calendar'][href='/bank-holidays/northern-ireland.ics']", visible: :hidden) end within("#content") do @@ -199,7 +199,7 @@ end end - context "showing bunting on bank holidays" do + context "when showing bunting on bank holidays" do it "shows bunting when today is a buntable bank holiday" do Timecop.travel(Date.parse("9th April 2012")) do visit "/bank-holidays" @@ -258,7 +258,7 @@ end end - context "last updated" do + describe "last updated" do it "is formatted correctly" do Timecop.travel(Date.parse("5th Dec 2012")) do visit "/bank-holidays" @@ -270,8 +270,8 @@ end end - context "GA4 tracking" do - it "has GA4 tracking on the .ics file links" do + describe ".ics file links" do + it "has GA4 tracking" do visit "/bank-holidays" link_parents = page.all(".app-c-subscribe") link_parents.each do |link_parent| diff --git a/spec/system/electoral_look_up_spec.rb b/spec/system/electoral_look_up_spec.rb index bd61365b3a..95c5a69ee5 100644 --- a/spec/system/electoral_look_up_spec.rb +++ b/spec/system/electoral_look_up_spec.rb @@ -14,7 +14,7 @@ def search_for(postcode:) click_button("Find") end - context "visiting the homepage" do + describe "local electoral office page" do it "contains a form for entering a postcode" do visit electoral_services_path @@ -25,7 +25,7 @@ def search_for(postcode:) end end - context "searching by postcode" do + context "when searching by postcode" do context "when a valid postcode is entered which matches a single address" do it "displays the electoral service (council) address if it's different to the registration office address" do with_different_address = JSON.parse(api_response) @@ -122,19 +122,23 @@ def search_for(postcode:) end end - context "API errors" do - context "400 and 404" do + describe "API errors" do + context "when the API returns 404" do + before { stub_api_postcode_lookup("XM45HQ", status: 404) } + it "displays unfindable postcode message" do - stub_api_postcode_lookup("XM45HQ", status: 404) with_electoral_api_url do search_for(postcode: "XM4 5HQ") expect(page).to have_text("We couldn't find this postcode") end end + end + + context "when the API returns 400" do + before { stub_api_address_lookup("1234", status: 400) } - it "displays unfindable address message" do - stub_api_address_lookup("1234", status: 400) + it "displays unfindable address message for a 400" do with_electoral_api_url do visit electoral_services_path(uprn: "1234") diff --git a/spec/system/find_local_council_spec.rb b/spec/system/find_local_council_spec.rb index a510bea0ca..4f68dc69ba 100644 --- a/spec/system/find_local_council_spec.rb +++ b/spec/system/find_local_council_spec.rb @@ -31,7 +31,7 @@ end it "adds the description as meta tag for SEO purposes" do - description = page.find("meta[name=\"description\"]", visible: false)["content"] + description = page.find("meta[name=\"description\"]", visible: :hidden)["content"] expect(description).to eq("Find your local authority in England, Wales, Scotland and Northern Ireland") end @@ -43,8 +43,8 @@ end context "when entering a postcode in the search form" do - context "for successful postcode lookup" do - context "for unitary local authority" do + context "with a successful postcode lookup" do + context "and with a unitary local authority" do before do configure_locations_api_and_local_authority("SW1A 1AA", %w[westminster], 5990) visit "/find-local-council" @@ -97,7 +97,7 @@ end end - context "for district local authority" do + context "and with a district local authority" do before do stub_locations_api_has_location("HP20 1UG", [{ "latitude" => 51.5010096, "longitude" => -0.141587, "local_custodian_code" => 440 }]) stub_local_links_manager_has_a_district_and_county_local_authority("aylesbury", "buckinghamshire", local_custodian_code: 440) @@ -161,7 +161,7 @@ # and a unitary (rather than district and council), which can happen during merge periods # where it is useful for a short period of time to mark a county as a unitary if it is # becoming one. See find_local_council_controller#result for more explanation - context "for district local authority with a unitary instead of county upper tier" do + context "and for a district local authority with a unitary instead of county upper tier" do before do stub_locations_api_has_location("HP20 1UG", [{ "latitude" => 51.5010096, "longitude" => -0.141587, "local_custodian_code" => 440 }]) stub_local_links_manager_has_a_district_and_unitary_local_authority("aylesbury", "buckinghamshire", local_custodian_code: 440) @@ -210,7 +210,7 @@ end end - context "for unsuccessful postcode lookup" do + context "with an unsuccessful postcode lookup" do context "with invalid postcode" do before do stub_locations_api_does_not_have_a_bad_postcode("NO POSTCODE") @@ -226,7 +226,7 @@ end it "adds \"Error:\" to the beginning of the page title" do - expect(page).to have_selector("title", text: "Error: Find your local council - GOV.UK", visible: false) + expect(page).to have_selector("title", text: "Error: Find your local council - GOV.UK", visible: :hidden) end it "shows an error message" do @@ -263,7 +263,7 @@ end it "adds \"Error:\" to the beginning of the page title" do - expect(page).to have_selector("title", text: "Error: Find your local council - GOV.UK", visible: false) + expect(page).to have_selector("title", text: "Error: Find your local council - GOV.UK", visible: :hidden) end it "shows an error message" do diff --git a/spec/system/gwyliau_banc_spec.rb b/spec/system/gwyliau_banc_spec.rb index 57bc094928..9f810f0407 100644 --- a/spec/system/gwyliau_banc_spec.rb +++ b/spec/system/gwyliau_banc_spec.rb @@ -13,17 +13,17 @@ Timecop.travel("2012-12-14") visit "/gwyliau-banc" - within("head", visible: false) do - expect(page).to have_selector("title", text: "Gwyliau banc y DU - GOV.UK", visible: false) - desc = page.find("meta[name=description]", visible: false) + within("head", visible: :hidden) do + expect(page).to have_selector("title", text: "Gwyliau banc y DU - GOV.UK", visible: :hidden) + desc = page.find("meta[name=description]", visible: :hidden) expect(desc["content"]).to eq("Dysgwch pryd mae gwyliau'r banc yng Nghymru, Lloegr, yr Alban a Gogledd Iwerddon - gan gynnwys gwyliau banc yn y gorffennol a'r dyfodol") - expect(page).to have_selector("link[rel=alternate][type='application/json'][href='/gwyliau-banc.json']", visible: false) - expect(page).to have_selector("link[rel=alternate][type='application/json'][href='/gwyliau-banc/cymru-a-lloegr.json']", visible: false) - expect(page).to have_selector("link[rel=alternate][type='text/calendar'][href='/gwyliau-banc/cymru-a-lloegr.ics']", visible: false) - expect(page).to have_selector("link[rel=alternate][type='application/json'][href='/gwyliau-banc/yr-alban.json']", visible: false) - expect(page).to have_selector("link[rel=alternate][type='text/calendar'][href='/gwyliau-banc/yr-alban.ics']", visible: false) - expect(page).to have_selector("link[rel=alternate][type='application/json'][href='/gwyliau-banc/gogledd-iwerddon.json']", visible: false) - expect(page).to have_selector("link[rel=alternate][type='text/calendar'][href='/gwyliau-banc/gogledd-iwerddon.ics']", visible: false) + expect(page).to have_selector("link[rel=alternate][type='application/json'][href='/gwyliau-banc.json']", visible: :hidden) + expect(page).to have_selector("link[rel=alternate][type='application/json'][href='/gwyliau-banc/cymru-a-lloegr.json']", visible: :hidden) + expect(page).to have_selector("link[rel=alternate][type='text/calendar'][href='/gwyliau-banc/cymru-a-lloegr.ics']", visible: :hidden) + expect(page).to have_selector("link[rel=alternate][type='application/json'][href='/gwyliau-banc/yr-alban.json']", visible: :hidden) + expect(page).to have_selector("link[rel=alternate][type='text/calendar'][href='/gwyliau-banc/yr-alban.ics']", visible: :hidden) + expect(page).to have_selector("link[rel=alternate][type='application/json'][href='/gwyliau-banc/gogledd-iwerddon.json']", visible: :hidden) + expect(page).to have_selector("link[rel=alternate][type='text/calendar'][href='/gwyliau-banc/gogledd-iwerddon.ics']", visible: :hidden) end within("#content") do @@ -171,7 +171,7 @@ end end - context "showing bunting on bank holidays" do + context "when showing bunting on bank holidays" do it "shows bunting when today is a buntable bank holiday" do Timecop.travel(Date.parse("2nd Jan 2012")) do visit "/gwyliau-banc" @@ -197,7 +197,7 @@ end end - context "last updated" do + describe "last updated" do it "is translated and localised" do Timecop.travel(Date.parse("25th Dec 2012")) do visit "/gwyliau-banc" diff --git a/spec/system/help_cookies_spec.rb b/spec/system/help_cookies_spec.rb index 5ff13fa68a..47e1d9c224 100644 --- a/spec/system/help_cookies_spec.rb +++ b/spec/system/help_cookies_spec.rb @@ -1,5 +1,5 @@ RSpec.describe "HelpCookies" do - context "rendering the cookies setting page" do + describe "the cookies settings page" do before do payload = { base_path: "/help/cookies", diff --git a/spec/system/help_page_spec.rb b/spec/system/help_page_spec.rb index 0427e7e0cb..72cfb0eb35 100644 --- a/spec/system/help_page_spec.rb +++ b/spec/system/help_page_spec.rb @@ -20,7 +20,7 @@ it "sets noindex meta tag" do visit "/help/cookie-details" - expect(page).to have_css('meta[name="robots"][content="noindex"]', visible: false) + expect(page).to have_css('meta[name="robots"][content="noindex"]', visible: :hidden) end it "does not render with the single page notification button" do diff --git a/spec/system/help_spec.rb b/spec/system/help_spec.rb index 54f1fb25dd..3278e9a3cb 100644 --- a/spec/system/help_spec.rb +++ b/spec/system/help_spec.rb @@ -1,5 +1,5 @@ RSpec.describe "Help" do - context "rendering the help index page" do + describe "the help index page" do before do payload = { base_path: "/help", diff --git a/spec/system/homepage_spec.rb b/spec/system/homepage_spec.rb index 3b1e39af1b..5e89546b72 100644 --- a/spec/system/homepage_spec.rb +++ b/spec/system/homepage_spec.rb @@ -53,7 +53,7 @@ ClimateControl.modify GOVUK_DISABLE_SEARCH_AUTOCOMPLETE: "1" do visit "/" - expect(page).to_not have_css(".gem-c-search-with-autocomplete") + expect(page).not_to have_css(".gem-c-search-with-autocomplete") expect(page).to have_css(".gem-c-search") end end diff --git a/spec/system/icalendar_spec.rb b/spec/system/icalendar_spec.rb index 5e83876b4d..91aa71867e 100644 --- a/spec/system/icalendar_spec.rb +++ b/spec/system/icalendar_spec.rb @@ -9,7 +9,7 @@ end end - context "getting ICS version" do + describe "ICS downloads" do before do Timecop.freeze(Time.zone.parse("2012-10-17 01:00:00")) end diff --git a/spec/system/json_spec.rb b/spec/system/json_spec.rb index 049ad3bd1c..64ab470c8a 100644 --- a/spec/system/json_spec.rb +++ b/spec/system/json_spec.rb @@ -7,7 +7,7 @@ mock_calendar_fixtures end - context "GET /calendars/.json" do + describe "GET /calendars/.json" do it "contains calendar with division" do visit "/bank-holidays/england-and-wales.json" expected = { diff --git a/spec/system/landing_page_spec.rb b/spec/system/landing_page_spec.rb index bb8968fc22..978741b018 100644 --- a/spec/system/landing_page_spec.rb +++ b/spec/system/landing_page_spec.rb @@ -1,7 +1,7 @@ RSpec.describe "LandingPage" do include SearchHelpers - describe "show" do + describe "GET " do let(:content_item) do { "base_path" => "/landing-page", @@ -125,7 +125,7 @@ expect(page).not_to have_content("Couldn't identify a model class for type: does_not_exist") end - context "when viewed on the draft server" do + context "and is being viewed on the draft server" do before do stub_content_store_has_item(base_path, content_item, draft: true) stub_content_store_has_item(basic_taxon["base_path"], basic_taxon, draft: true) @@ -142,10 +142,11 @@ end end - context "organisation logo" do + describe "organisation logo" do before do stub_content_store_has_item(base_path, content_item.deep_merge({ "details" => { "theme" => "prime-ministers-office-10-downing-street" } })) end + it "has ga4 tracking on the organisation logo" do visit base_path expect(page).to have_selector(".gem-c-organisation-logo[data-module=ga4-link-tracker]") @@ -154,7 +155,7 @@ end end - context "columns layout" do + describe "columns layout" do it "has ga4 tracking on the columns layout" do visit base_path expect(page).to have_selector(".columns-layout[data-module=ga4-link-tracker]") @@ -164,7 +165,7 @@ end end - context "main navigation" do + describe "main navigation" do it "has ga4 tracking on the main navigation" do visit base_path expect(page).to have_selector(".main-nav__nav-container nav[data-module=ga4-link-tracker]") diff --git a/spec/system/licence_transaction_spec.rb b/spec/system/licence_transaction_spec.rb index f76731e0ff..90eaa62e94 100644 --- a/spec/system/licence_transaction_spec.rb +++ b/spec/system/licence_transaction_spec.rb @@ -25,7 +25,7 @@ } end - context "given a location specific licence" do + context "with a location specific licence" do before do configure_locations_api_and_local_authority("SW1A 1AA", %w[westminster], 5990) stub_local_links_manager_does_not_have_an_authority("not-a-valid-council-name") @@ -688,7 +688,7 @@ end end - context "given a non-location specific licence" do + context "with a non-location specific licence" do before do @payload = { base_path: "/find-licences/licence-to-kill", @@ -765,7 +765,7 @@ end end - context "given a licence edition with continuation link" do + context "with a licence edition with continuation link" do before do @payload = { base_path: "/find-licences/artistic-license", @@ -809,7 +809,7 @@ end end - context "given a licence which does not exist in licensify and uses authority url" do + context "with a licence which does not exist in licensify and uses authority url" do before do stub_content_store_has_item("/find-licences/licence-to-kill", @payload) configure_locations_api_and_local_authority("SW1A 1AA", %w[a-council], 5990) @@ -868,7 +868,7 @@ end end - context "given a licence which does not exist in licensify" do + context "with a licence which does not exist in licensify" do before do stub_content_store_has_item("/find-licences/licence-to-kill", @payload) stub_licence_does_not_exist("1071-5-1") @@ -899,7 +899,7 @@ end end - context "given that licensify times out" do + context "when licensify times out" do before do stub_content_store_has_item("/find-licences/licence-to-kill", @payload) stub_licence_times_out("1071-5-1") @@ -911,7 +911,7 @@ end end - context "given that licensify errors" do + context "when licensify returns an error" do before do stub_content_store_has_item("/find-licences/licence-to-kill", @payload) stub_licence_returns_error("1071-5-1") @@ -923,7 +923,7 @@ end end - context "given the usesLicensify parameter" do + context "with the usesLicensify parameter" do before do stub_content_store_has_item("/find-licences/licence-to-kill", @payload) end diff --git a/spec/system/local_transactions_spec.rb b/spec/system/local_transactions_spec.rb index 7e307cf2d9..fd9a5b3241 100644 --- a/spec/system/local_transactions_spec.rb +++ b/spec/system/local_transactions_spec.rb @@ -44,7 +44,7 @@ stub_content_store_has_item("/pay-bear-tax", @payload) end - context "given a local transaction with an interaction present" do + context "with a local transaction with an interaction present" do before do stub_local_links_manager_has_a_link( authority_slug: "westminster", @@ -347,7 +347,7 @@ end end - context "given a local transaction without an interaction present" do + context "with a local transaction but without an interaction present" do before do stub_local_links_manager_has_no_link(authority_slug: "westminster", lgsl: 461, lgil: 8, country_name: "England") end @@ -402,7 +402,7 @@ end end - context "given no interaction present and a missing homepage url" do + context "with no interaction present and a missing homepage url" do before do stub_local_links_manager_has_no_link_and_no_homepage_url(authority_slug: "westminster", lgsl: 461, lgil: 8, country_name: "England") visit "/pay-bear-tax" diff --git a/spec/system/phase_banner_spec.rb b/spec/system/phase_banner_spec.rb index eabb3d675e..9aae6f6487 100644 --- a/spec/system/phase_banner_spec.rb +++ b/spec/system/phase_banner_spec.rb @@ -4,7 +4,7 @@ RSpec.describe "Phase Banner" do let(:base_path) { "/help/about-govuk" } - context "in the live phase" do + context "when in the live phase" do before do content_store_has_example_item(base_path, schema: :help_page, example: "about-govuk") end @@ -16,7 +16,7 @@ end end - context "in the beta phase" do + context "when in the beta phase" do before do content_item = GovukSchemas::Example.find(:help_page, example_name: "about-govuk") content_item["base_path"] = base_path diff --git a/spec/system/place_spec.rb b/spec/system/place_spec.rb index c85f04e59d..f03bea684b 100644 --- a/spec/system/place_spec.rb +++ b/spec/system/place_spec.rb @@ -160,7 +160,7 @@ end end - context "given a valid postcode" do + context "with a valid postcode" do before do stub_places_manager_has_places_for_postcode(@places, "find-passport-offices", "SW1A 1AA", Frontend::PLACES_MANAGER_QUERY_LIMIT, nil) visit "/passport-interview-office" @@ -231,7 +231,7 @@ end end - context "given a valid postcode with no nearby places" do + context "with a valid postcode which has no nearby places" do before do @places = [] stub_places_manager_has_places_for_postcode(@places, "find-passport-offices", "SW1A 1AA", Frontend::PLACES_MANAGER_QUERY_LIMIT, nil) @@ -249,7 +249,7 @@ end end - context "given an empty postcode" do + context "with an empty postcode" do before do visit "/passport-interview-office" click_on("Find results near you") @@ -278,7 +278,7 @@ end end - context "given an invalid postcode" do + context "with an invalid postcode" do before do query_hash = { "postcode" => "BAD POSTCODE", "limit" => Frontend::PLACES_MANAGER_QUERY_LIMIT } return_data = { "error" => "invalidPostcodeError" } @@ -305,7 +305,7 @@ end end - context "given a valid postcode with no locations returned" do + context "with a valid postcode but with no locations returned" do before do query_hash = { "postcode" => "JE4 5TP", "limit" => Frontend::PLACES_MANAGER_QUERY_LIMIT } return_data = { "error" => "validPostcodeNoLocation" } @@ -328,7 +328,7 @@ end end - context "given a postcode which covers multiple authorities (and a local_authority place)" do + context "with a postcode which covers multiple authorities (and a local_authority place)" do before do addresses = [ { "address" => "House 1", "local_authority_slug" => "achester" }, @@ -346,7 +346,7 @@ end end - context "given an internal error response from places manager" do + context "when places manager returns an error" do before do query_hash = { "postcode" => "JE4 5TP", "limit" => Frontend::PLACES_MANAGER_QUERY_LIMIT } stub_places_manager_places_request("find-passport-offices", query_hash, {}, 500) diff --git a/spec/system/sessions_spec.rb b/spec/system/sessions_spec.rb index 883c954bd9..f4e9993797 100644 --- a/spec/system/sessions_spec.rb +++ b/spec/system/sessions_spec.rb @@ -15,7 +15,7 @@ after { Rails.application.reload_routes! } - context "Given a signing in user" do + context "when a user is signing in" do it "Logs the user in and redirect them to manage their account" do given_a_successful_login_attempt visit new_govuk_session_callback_path(code: "code", state: "state") diff --git a/spec/system/sign_in_spec.rb b/spec/system/sign_in_spec.rb index 38e2bd85e0..f2ef53a2d6 100644 --- a/spec/system/sign_in_spec.rb +++ b/spec/system/sign_in_spec.rb @@ -1,5 +1,5 @@ RSpec.describe "Sign in" do - context "rendering the sign-in help page" do + describe "Sign-in help page" do before do payload = { base_path: "/sign-in", diff --git a/spec/system/simple_smart_answers_spec.rb b/spec/system/simple_smart_answers_spec.rb index 523f979555..bd63fa7992 100644 --- a/spec/system/simple_smart_answers_spec.rb +++ b/spec/system/simple_smart_answers_spec.rb @@ -325,7 +325,7 @@ click_on("Start now") expect(page.current_url).to eq("http://www.example.com/the-bridge-of-death/y?token=#{token}") - expect(page).to have_selector("input[value='#{token}']", visible: false) + expect(page).to have_selector("input[value='#{token}']", visible: :hidden) end it "allows changing an answer" do diff --git a/spec/system/static_error_page_spec.rb b/spec/system/static_error_page_spec.rb index 46b7e04f42..959e728b9a 100644 --- a/spec/system/static_error_page_spec.rb +++ b/spec/system/static_error_page_spec.rb @@ -1,5 +1,5 @@ RSpec.describe "Static Error Pages" do - context "When asked for a 4xx page" do + context "when asked for a 4xx page" do it "renders the appropriate page" do visit "/static-error-pages/404.html" @@ -24,7 +24,7 @@ end end - context "When asked for a 5xx page" do + context "when asked for a 5xx page" do it "renders the appropriate page" do visit "/static-error-pages/500.html" diff --git a/spec/system/transaction_spec.rb b/spec/system/transaction_spec.rb index 9b8cfa1418..42795d4d6c 100644 --- a/spec/system/transaction_spec.rb +++ b/spec/system/transaction_spec.rb @@ -1,7 +1,7 @@ RSpec.describe "Transaction" do include SchemaOrgHelpers - context "a transaction with all the optional things" do + context "with a transaction with all the optional things" do before do @payload = { analytics_identifier: nil, @@ -38,8 +38,8 @@ expect(page.status_code).to eq(200) within("head", visible: :all) do - expect(page).to have_selector("title", text: "Carrots - GOV.UK", visible: false) - expect(page).not_to have_selector("meta[name='robots']", visible: false) + expect(page).to have_selector("title", text: "Carrots - GOV.UK", visible: :hidden) + expect(page).not_to have_selector("meta[name='robots']", visible: :hidden) end within("#content") do @@ -90,7 +90,7 @@ end end - context "jobsearch page" do + context "with a jobsearch page" do it "renders ok" do content_store_has_example_item("/jobsearch", schema: "transaction", example: "jobsearch") visit "/jobsearch" @@ -100,7 +100,7 @@ end end - context "start page which should have cross domain analytics" do + context "with a start page which should have cross domain analytics" do it "includes cross domain analytics javascript" do content_store_has_example_item("/foo", schema: "transaction", example: "transaction") visit "/foo" @@ -110,7 +110,7 @@ end end - context "start page format which shouldn't have cross domain analytics" do + context "with a start page format which shouldn't have cross domain analytics" do it "does not include cross domain analytics javascript" do content_store_has_example_item("/foo", schema: "transaction", example: "jobsearch") visit "/foo" @@ -120,7 +120,7 @@ end end - context "locale is 'cy'" do + context "when locale is 'cy'" do before do @payload = { base_path: "/cymraeg", @@ -151,7 +151,7 @@ end end - context "a transaction has variants" do + context "when a transaction has variants" do it "renders correct content including robots meta tag" do content_store_has_example_item("/council-tax-bands-2", schema: "transaction", example: "transaction-with-variants") visit "/council-tax-bands-2/council-tax-bands-2-staging" @@ -164,7 +164,7 @@ end within("head", visible: :all) do - expect(page).to have_selector("meta[name='robots'][content='noindex, nofollow']", visible: false) + expect(page).to have_selector("meta[name='robots'][content='noindex, nofollow']", visible: :hidden) end end end diff --git a/spec/system/travel_advice_atom_spec.rb b/spec/system/travel_advice_atom_spec.rb index 6c8a5916bb..f47445e430 100644 --- a/spec/system/travel_advice_atom_spec.rb +++ b/spec/system/travel_advice_atom_spec.rb @@ -1,7 +1,7 @@ RSpec.describe "TravelAdviceAtom" do include GdsApi::TestHelpers::ContentStore - context "aggregate feed" do + describe "aggregate feed" do it "displays the list of countries as an atom feed" do content_item = GovukSchemas::Example.find("travel_advice_index", example_name: "index") base_path = content_item.fetch("base_path") @@ -41,7 +41,7 @@ end end - context "individual country feed" do + describe "individual country feed" do it "displays a country as an atom feed" do content_store_response = GovukSchemas::Example.find("travel_advice", example_name: "full-country") base_path = content_store_response.fetch("base_path") diff --git a/spec/system/travel_advice_spec.rb b/spec/system/travel_advice_spec.rb index c2cccd454a..083f382fe9 100644 --- a/spec/system/travel_advice_spec.rb +++ b/spec/system/travel_advice_spec.rb @@ -1,5 +1,5 @@ RSpec.describe "TravelAdvice" do - context "index" do + describe "GET /foreign-travel-advice" do before do content_item = GovukSchemas::Example.find("travel_advice_index", example_name: "index") base_path = content_item.fetch("base_path") @@ -14,7 +14,7 @@ within("head", visible: :all) do expect(page).to have_selector("title", text: "Foreign travel advice", visible: :all) expect(page).to have_selector("link[rel=alternate][type='application/atom+xml'][href='/foreign-travel-advice.atom']", visible: :all) - expect(page).to have_selector("meta[name=description][content='Latest travel advice by country including safety and security, entry requirements, travel warnings and health']", visible: false) + expect(page).to have_selector("meta[name=description][content='Latest travel advice by country including safety and security, entry requirements, travel warnings and health']", visible: :hidden) end expect(page).to have_selector("#wrapper.travel-advice") @@ -98,7 +98,7 @@ end end - context "show" do + describe "GET /foreign-travel-advice/" do before do @content_store_response = GovukSchemas::Example.find("travel_advice", example_name: "full-country") @base_path = @content_store_response.fetch("base_path") @@ -150,7 +150,7 @@ expect(page).to have_css(".gem-c-contextual-breadcrumbs") end - context "first part" do + describe "first part" do it "displays latest updates" do visit @base_path @@ -176,7 +176,7 @@ end end - context "other parts" do + describe "other parts" do before do @part = @content_store_response.dig("details", "parts").last end @@ -207,7 +207,7 @@ it "includes a discoverable atom feed link" do visit @base_path - expect(page).to have_css("link[type*='atom'][href='#{@base_path}.atom']", visible: false) + expect(page).to have_css("link[type*='atom'][href='#{@base_path}.atom']", visible: :hidden) end it "does not render with the single page notification button" do diff --git a/spec/system/when_do_the_clocks_change_spec.rb b/spec/system/when_do_the_clocks_change_spec.rb index 7ca2247871..ae59059da3 100644 --- a/spec/system/when_do_the_clocks_change_spec.rb +++ b/spec/system/when_do_the_clocks_change_spec.rb @@ -9,13 +9,13 @@ it "displays the clocks change page" do visit "/when-do-the-clocks-change" - within("head", visible: false) do - expect(page).to have_selector("title", text: "When do the clocks change? - GOV.UK", visible: false) - desc = page.find("meta[name=description]", visible: false) + within("head", visible: :hidden) do + expect(page).to have_selector("title", text: "When do the clocks change? - GOV.UK", visible: :hidden) + desc = page.find("meta[name=description]", visible: :hidden) expect(desc["content"]).to eq("Dates when the clocks go back or forward - includes British Summer Time, Greenwich Mean Time") - expect(page).to have_selector("link[rel=alternate][type='application/json'][href='/when-do-the-clocks-change/united-kingdom.json']", visible: false) - expect(page).to have_selector("link[rel=alternate][type='text/calendar'][href='/when-do-the-clocks-change/united-kingdom.ics']", visible: false) + expect(page).to have_selector("link[rel=alternate][type='application/json'][href='/when-do-the-clocks-change/united-kingdom.json']", visible: :hidden) + expect(page).to have_selector("link[rel=alternate][type='text/calendar'][href='/when-do-the-clocks-change/united-kingdom.ics']", visible: :hidden) end within("#content") do From b6a04e26e3286c3d15c6a1b7264adafa4450c76b Mon Sep 17 00:00:00 2001 From: Keith Lawrence Date: Mon, 16 Dec 2024 16:26:11 +0000 Subject: [PATCH 14/19] lint: fix spec/unit/presenters --- .../content_item_model_presenter_spec.rb | 10 +- .../presenters/electoral_presenter_spec.rb | 17 +-- spec/unit/presenters/faq_presenter_spec.rb | 7 +- .../licence_details_presenter_spec.rb | 114 +++++++++++------- .../simple_smart_answer_presenter_spec.rb | 30 +++-- .../presenters/transaction_presenter_spec.rb | 10 +- .../travel_advice_index_presenter_spec.rb | 2 +- 7 files changed, 109 insertions(+), 81 deletions(-) diff --git a/spec/unit/presenters/content_item_model_presenter_spec.rb b/spec/unit/presenters/content_item_model_presenter_spec.rb index dca552e193..5f6cc442cb 100644 --- a/spec/unit/presenters/content_item_model_presenter_spec.rb +++ b/spec/unit/presenters/content_item_model_presenter_spec.rb @@ -1,19 +1,17 @@ RSpec.describe ContentItemModelPresenter do describe "#page_title" do - before do - @content_store_response = GovukSchemas::Example.find("detailed_guide", example_name: "withdrawn_detailed_guide") - end + let(:content_store_response) { GovukSchemas::Example.find("detailed_guide", example_name: "withdrawn_detailed_guide") } it "includes withdrawn in the page title when the content item is withdrawn" do - content_item = ContentItem.new(@content_store_response) + content_item = ContentItem.new(content_store_response) expect(described_class.new(content_item).page_title).to include("[Withdrawn]") expect(described_class.new(content_item).page_title).to include(content_item.title) end it "only includes the content item title when the page is not withdrawn" do - @content_store_response["withdrawn_notice"] = {} - content_item = ContentItem.new(@content_store_response) + content_store_response["withdrawn_notice"] = {} + content_item = ContentItem.new(content_store_response) expect(described_class.new(content_item).page_title).not_to include("[Withdrawn]") expect(described_class.new(content_item).page_title).to include(content_item.title) diff --git a/spec/unit/presenters/electoral_presenter_spec.rb b/spec/unit/presenters/electoral_presenter_spec.rb index 4aee6ec404..da1059fdd0 100644 --- a/spec/unit/presenters/electoral_presenter_spec.rb +++ b/spec/unit/presenters/electoral_presenter_spec.rb @@ -5,7 +5,7 @@ def api_response JSON.parse(fixture) end - context "exposing attributes from the json payload" do + context "when exposing attributes from the json payload" do described_class::EXPECTED_KEYS.each do |exposed_attribute| it "exposes the value of #{exposed_attribute} from payload via a method" do subject = described_class.new(api_response) @@ -16,7 +16,7 @@ def api_response end end - context "presenting addresses" do + context "when presenting addresses" do context "when duplicate contact details are provided" do it "does not show the electoral services address" do with_duplicate_contact = api_response @@ -42,14 +42,15 @@ def api_response describe "#show_picker?" do context "when address picker is present in the api response" do + let(:with_address_picker) { api_response } + before do - @with_address_picker = api_response - @with_address_picker["address_picker"] = true + with_address_picker["address_picker"] = true end - context "address details are present" do + context "when address details are present" do it "returns true" do - with_no_contact_details = @with_address_picker + with_no_contact_details = with_address_picker with_no_contact_details["registration"] = nil with_no_contact_details["electoral_services"] = nil subject = described_class.new(with_no_contact_details) @@ -58,9 +59,9 @@ def api_response end end - context "address details are missing" do + context "when address details are missing" do it "returns false" do - subject = described_class.new(@with_address_picker) + subject = described_class.new(with_address_picker) expect(subject.show_picker?).to be false end diff --git a/spec/unit/presenters/faq_presenter_spec.rb b/spec/unit/presenters/faq_presenter_spec.rb index d78e8719f6..88b6f07032 100644 --- a/spec/unit/presenters/faq_presenter_spec.rb +++ b/spec/unit/presenters/faq_presenter_spec.rb @@ -1,8 +1,9 @@ RSpec.describe FaqPresenter do include CalendarHelpers + let(:view_context) { ApplicationController.new.view_context } + before do - @view_context = ApplicationController.new.view_context mock_calendar_fixtures end @@ -15,7 +16,7 @@ Timecop.travel(Date.parse("2012-03-24")) do scope = "bank-holidays" calendar = Calendar.find(scope) - presenter = described_class.new(scope, calendar, payload(calendar), @view_context) + presenter = described_class.new(scope, calendar, payload(calendar), view_context) expect(presenter.metadata["mainEntity"]).to eq(expected) end @@ -26,7 +27,7 @@ Timecop.travel(Date.parse("2012-03-24")) do scope = "when-do-the-clocks-change" calendar = Calendar.find(scope) - presenter = described_class.new(scope, calendar, payload(calendar), @view_context) + presenter = described_class.new(scope, calendar, payload(calendar), view_context) expect(presenter.metadata["mainEntity"]).to eq(expected) end diff --git a/spec/unit/presenters/licence_details_presenter_spec.rb b/spec/unit/presenters/licence_details_presenter_spec.rb index adbb9c9bb6..7d6acfff5f 100644 --- a/spec/unit/presenters/licence_details_presenter_spec.rb +++ b/spec/unit/presenters/licence_details_presenter_spec.rb @@ -1,6 +1,6 @@ RSpec.describe LicenceDetailsPresenter do - before do - @local_authority_licence = { + let(:local_authority_licence) do + { "isLocationSpecific" => true, "isOfferedByCounty" => false, "geographicalAvailability" => %w[England Wales], @@ -30,7 +30,10 @@ }, ], } - @the_one_licence_authority = { + end + + let(:the_one_licence_authority) do + { "authorityName" => "The One Licence Authority", "authoritySlug" => "the-one-licence-authority", "authorityInteractions" => { @@ -43,7 +46,10 @@ ], }, } - @the_other_licence_authority = { + end + + let(:the_other_licence_authority) do + { "authorityName" => "The Other Licence Authority", "authoritySlug" => "the-other-licence-authority", "authorityInteractions" => { @@ -56,7 +62,10 @@ ], }, } - @licence_authority_not_using_licensify = { + end + + let(:licence_authority_not_using_licensify) do + { "authorityName" => "The Licence Authority Not Using Licensify", "authoritySlug" => "the-licence-authority-not-using-licensify", "authorityInteractions" => { @@ -69,7 +78,10 @@ ], }, } - @licence_authority_not_using_authority_url = { + end + + let(:licence_authority_not_using_authority_url) do + { "authorityName" => "The Licence Authority Not Using Licensify", "authoritySlug" => "the-licence-authority-not-using-licensify", "authorityInteractions" => { @@ -82,7 +94,10 @@ ], }, } - @licence_authority_without_uses_licensify_param = { + end + + let(:licence_authority_without_uses_licensify_param) do + { "authorityName" => "The Licence Authority Without UsesLicensify Param", "authoritySlug" => "the-licence-authority-without-uses-licensify-param", "authorityInteractions" => { @@ -95,7 +110,10 @@ ], }, } - @licence_authority_without_uses_authority_url_param = { + end + + let(:licence_authority_without_uses_authority_url_param) do + { "authorityName" => "The Licence Authority Without UsesLicensify Param", "authoritySlug" => "the-licence-authority-without-uses-licensify-param", "authorityInteractions" => { @@ -108,24 +126,33 @@ ], }, } - @licence_authority_with_no_actions = { + end + + let(:licence_authority_with_no_actions) do + { "authorityName" => "The Licence Authority with no actions", "authoritySlug" => "the-licence-authority-with-no-actions", "authorityInteractions" => {}, } - @licence_authority_licence = { + end + + let(:licence_authority_licence) do + { "isLocationSpecific" => false, "isOfferedByCounty" => false, "geographicalAvailability" => %w[England Wales], - "issuingAuthorities" => [@the_one_licence_authority], + "issuingAuthorities" => [the_one_licence_authority], } - @multiple_authorities_and_location_specific_licence = { + end + + let(:multiple_authorities_and_location_specific_licence) do + { "isLocationSpecific" => true, "isOfferedByCounty" => false, "geographicalAvailability" => %w[England Wales], "issuingAuthorities" => [ - @the_other_licence_authority, - @the_one_licence_authority, + the_other_licence_authority, + the_one_licence_authority, ], } end @@ -133,13 +160,13 @@ describe "#single_licence_authority_present?" do context "when authority present" do it "returns true for licence authority specific licence" do - subject = described_class.new(@licence_authority_licence) + subject = described_class.new(licence_authority_licence) expect(subject.single_licence_authority_present?).to be true end it "returns false for local authority specific licence" do - subject = described_class.new(@local_authority_licence) + subject = described_class.new(local_authority_licence) expect(subject.single_licence_authority_present?).to be false end @@ -147,16 +174,16 @@ context "when authority non-present" do it "returns false" do - subject = described_class.new(@licence_authority_licence.merge("issuingAuthorities" => [])) + subject = described_class.new(licence_authority_licence.merge("issuingAuthorities" => [])) expect(subject.single_licence_authority_present?).to be false end end end - context "authority look up methods" do - before do - @presented_the_one_licence_authority = { + context "with authority look up methods" do + let(:presented_the_one_licence_authority) do + { "name" => "The One Licence Authority", "slug" => "the-one-licence-authority", "contact" => nil, @@ -173,7 +200,10 @@ ], }, } - @presented_the_other_licence_authority = { + end + + let(:presented_the_other_licence_authority) do + { "name" => "The Other Licence Authority", "slug" => "the-other-licence-authority", "contact" => nil, @@ -195,37 +225,37 @@ describe "#authority" do context "when one authority is present" do it "returns the authority" do - subject = described_class.new(@licence_authority_licence) + subject = described_class.new(licence_authority_licence) - expect(subject.authority).to eq(@presented_the_one_licence_authority) + expect(subject.authority).to eq(presented_the_one_licence_authority) end end context "when more than one authority are present" do it "returns the matched authority if authority slug provided" do - subject = described_class.new(@multiple_authorities_and_location_specific_licence, "the-other-licence-authority") + subject = described_class.new(multiple_authorities_and_location_specific_licence, "the-other-licence-authority") - expect(subject.authority).to eq(@presented_the_other_licence_authority) + expect(subject.authority).to eq(presented_the_other_licence_authority) end it "returns nil if no matched authority found" do - subject = described_class.new(@multiple_authorities_and_location_specific_licence, "a-funky-licence-authority") + subject = described_class.new(multiple_authorities_and_location_specific_licence, "a-funky-licence-authority") expect(subject.authority).to be_nil end - context "no authority_slug is provided" do + context "and no authority_slug is provided" do it "returns the first authority" do - subject = described_class.new(@multiple_authorities_and_location_specific_licence) + subject = described_class.new(multiple_authorities_and_location_specific_licence) - expect(subject.authority).to eq(@presented_the_other_licence_authority) + expect(subject.authority).to eq(presented_the_other_licence_authority) end end end context "when no authority is present" do it "returns nil" do - subject = described_class.new(@licence_authority_licence.merge("issuingAuthorities" => [])) + subject = described_class.new(licence_authority_licence.merge("issuingAuthorities" => [])) expect(subject.authority).to be_nil end @@ -235,59 +265,59 @@ describe "#uses_licensify" do it "is true if the action has a field to confirm that it uses licensify" do - subject = described_class.new(@licence_authority_licence, "the-one-licence-authority") + subject = described_class.new(licence_authority_licence, "the-one-licence-authority") expect(subject.uses_licensify("apply")).to be true end it "is true if the default action has uses_licensify set to true" do - subject = described_class.new(@licence_authority_licence, "the-one-licence-authority", "apply") + subject = described_class.new(licence_authority_licence, "the-one-licence-authority", "apply") expect(subject.uses_licensify("apply")).to be true end it "is false if the action has the uses_licensify field set to false" do - subject = described_class.new(@licence_authority_not_using_licensify) + subject = described_class.new(licence_authority_not_using_licensify) expect(subject.uses_licensify("apply")).to be false end it "is false if the action does not have a usesLicensify field" do - subject = described_class.new(@licence_authority_without_uses_licensify_param) + subject = described_class.new(licence_authority_without_uses_licensify_param) expect(subject.uses_licensify("apply")).to be false end it "is false if authority is nil" do - subject = described_class.new(@the_one_licence_authority) + subject = described_class.new(the_one_licence_authority) expect(subject.uses_licensify("apply")).to be false end end describe "#uses_authority_url" do it "is true if the action has a field to confirm that it uses authority url" do - subject = described_class.new(@licence_authority_licence, "the-one-licence-authority") + subject = described_class.new(licence_authority_licence, "the-one-licence-authority") expect(subject.uses_authority_url("apply")).to be true end it "is true if the default action has uses_authority_url set to true" do - subject = described_class.new(@licence_authority_licence, "the-one-licence-authority", "apply") + subject = described_class.new(licence_authority_licence, "the-one-licence-authority", "apply") expect(subject.uses_authority_url("apply")).to be true end it "is false if the action has the uses_authority_url field set to false" do - subject = described_class.new(@licence_authority_not_using_authority_url) + subject = described_class.new(licence_authority_not_using_authority_url) expect(subject.uses_authority_url("apply")).to be false end it "is false if the action does not have a usesAuthorityUrl field" do - subject = described_class.new(@licence_authority_without_uses_authority_url_param) + subject = described_class.new(licence_authority_without_uses_authority_url_param) expect(subject.uses_authority_url("apply")).to be false end it "is false if authority is nil" do - subject = described_class.new(@the_one_licence_authority) + subject = described_class.new(the_one_licence_authority) expect(subject.uses_authority_url("apply")).to be false end @@ -295,13 +325,13 @@ describe "#action" do it "returns action name if available" do - subject = described_class.new(@local_authority_licence, nil, "apply") + subject = described_class.new(local_authority_licence, nil, "apply") expect(subject.action).to eq("apply") end it "raises RecordNotFound if not available" do - subject = described_class.new(@local_authority_licence, nil, "complain") + subject = described_class.new(local_authority_licence, nil, "complain") expect { subject.action }.to raise_error(RecordNotFound) end diff --git a/spec/unit/presenters/simple_smart_answer_presenter_spec.rb b/spec/unit/presenters/simple_smart_answer_presenter_spec.rb index 780f710112..b862287e26 100644 --- a/spec/unit/presenters/simple_smart_answer_presenter_spec.rb +++ b/spec/unit/presenters/simple_smart_answer_presenter_spec.rb @@ -3,38 +3,36 @@ def subject(content_item) described_class.new(content_item) end - before do - @content_store_response = GovukSchemas::Example.find("simple_smart_answer", example_name: "simple-smart-answer") - end + let(:content_store_response) { GovukSchemas::Example.find("simple_smart_answer", example_name: "simple-smart-answer") } - context "locale is 'cy'" do + context "when locale is 'cy'" do before { I18n.locale = :cy } after { I18n.locale = :en } - context "start_button_text is 'Start now'" do + context "and start_button_text is 'Start now'" do it "returns Welsh translation 'Dechrau nawr'" do - @content_store_response["details"]["start_button_text"] = "Start now" - content_item = SimpleSmartAnswer.new(@content_store_response) + content_store_response["details"]["start_button_text"] = "Start now" + content_item = SimpleSmartAnswer.new(content_store_response) - expect(described_class.new(content_item).start_button_text).to eq("Dechrau nawr") + expect(subject(content_item).start_button_text).to eq("Dechrau nawr") end end - context "start_button_text is 'Continue'" do + context "and start_button_text is 'Continue'" do it "returns Welsh translation 'Parhau'" do - @content_store_response["details"]["start_button_text"] = "Continue" - content_item = SimpleSmartAnswer.new(@content_store_response) + content_store_response["details"]["start_button_text"] = "Continue" + content_item = SimpleSmartAnswer.new(content_store_response) - expect(described_class.new(content_item).start_button_text).to eq("Parhau") + expect(subject(content_item).start_button_text).to eq("Parhau") end end - context "start_button_text is explicitly set" do + context "and start_button_text is explicitly set" do it "returns the set value" do - @content_store_response["details"]["start_button_text"] = "Go for it" - content_item = SimpleSmartAnswer.new(@content_store_response) + content_store_response["details"]["start_button_text"] = "Go for it" + content_item = SimpleSmartAnswer.new(content_store_response) - expect(described_class.new(content_item).start_button_text).to eq("Go for it") + expect(subject(content_item).start_button_text).to eq("Go for it") end end end diff --git a/spec/unit/presenters/transaction_presenter_spec.rb b/spec/unit/presenters/transaction_presenter_spec.rb index 994f3b480b..7ccb193219 100644 --- a/spec/unit/presenters/transaction_presenter_spec.rb +++ b/spec/unit/presenters/transaction_presenter_spec.rb @@ -5,7 +5,7 @@ def subject(content_item) let(:content_store_response) { GovukSchemas::Example.find("transaction", example_name: "transaction") } - context "start_button_text is 'Start now'" do + context "when start_button_text is 'Start now'" do describe "#start_button_text" do it "shows the start_button_text" do content_store_response["details"]["start_button_text"] = "Start now" @@ -15,7 +15,7 @@ def subject(content_item) end end - context "start_button_text is 'Sign in'" do + context "when start_button_text is 'Sign in'" do describe "#start_button_text" do it "shows the custom start button text" do content_store_response["details"]["start_button_text"] = "Sign in" @@ -25,11 +25,11 @@ def subject(content_item) end end - context "locale is 'cy'" do + context "when locale is 'cy'" do before { I18n.locale = :cy } after { I18n.locale = :en } - context "start_button_text is 'Start now'" do + context "and start_button_text is 'Start now'" do it "returns Welsh translation 'Dechrau nawr'" do content_store_response["details"]["start_button_text"] = "Start now" content_item = Transaction.new(content_store_response) @@ -38,7 +38,7 @@ def subject(content_item) end end - context "start_button_text is 'Sign in'" do + context "and start_button_text is 'Sign in'" do it "returns Welsh translation 'Mewngofnodi'" do content_store_response["details"]["start_button_text"] = "Sign in" content_item = Transaction.new(content_store_response) diff --git a/spec/unit/presenters/travel_advice_index_presenter_spec.rb b/spec/unit/presenters/travel_advice_index_presenter_spec.rb index bdf1d626d7..012734f347 100644 --- a/spec/unit/presenters/travel_advice_index_presenter_spec.rb +++ b/spec/unit/presenters/travel_advice_index_presenter_spec.rb @@ -1,5 +1,5 @@ RSpec.describe TravelAdviceIndexPresenter do - context "handling countries" do + context "when handling countries" do let(:attributes) { GovukSchemas::Example.find("travel_advice_index", example_name: "index") } let(:presenter) { described_class.new(attributes) } From 74b29d591b4d48ac8e6d63a5c30015c3aafa64c1 Mon Sep 17 00:00:00 2001 From: Keith Lawrence Date: Mon, 16 Dec 2024 11:49:00 +0000 Subject: [PATCH 15/19] lint: fix spec/unit/services --- .../unit/services/csv_preview_service_spec.rb | 85 +++++++++---------- 1 file changed, 42 insertions(+), 43 deletions(-) diff --git a/spec/unit/services/csv_preview_service_spec.rb b/spec/unit/services/csv_preview_service_spec.rb index 593846c3dc..4cc6c34411 100644 --- a/spec/unit/services/csv_preview_service_spec.rb +++ b/spec/unit/services/csv_preview_service_spec.rb @@ -1,17 +1,10 @@ RSpec.describe CsvPreviewService do - before do - @csv = "header 1,header 2,header 3\ncontent 1,content 2,content 3\n" - @long_csv = @csv.dup - (described_class::MAXIMUM_ROWS + 10).times do - @long_csv = "#{@long_csv}other content 1,other content 2,other content 3\n" - end - @crlf_csv = @csv.gsub(/\n/, "\r\n") - @csv_with_many_columns = "column" - (described_class::MAXIMUM_COLUMNS + 10).times do - @csv_with_many_columns = "#{@csv_with_many_columns},column" - end - (@csv_with_many_columns << "\n") - @parsed_csv = [ + subject(:csv_preview_service) { described_class.new(csv) } + + let(:csv) { "header 1,header 2,header 3\ncontent 1,content 2,content 3\n" } + + let(:parsed_csv) do + [ [ { text: "header 1" }, { text: "header 2" }, @@ -27,118 +20,124 @@ describe "#csv_rows" do context "with normal CSV" do - subject { described_class.new(@csv).csv_rows } - it "parses the CSV correctly" do - expect(subject.first).to eq(@parsed_csv) + expect(csv_preview_service.csv_rows.first).to eq(parsed_csv) end it "indicates that it is not truncated" do - expect(subject.second).to be_falsey + expect(csv_preview_service.csv_rows.second).to be_falsey end end context "with CSV containing CRLF line terminators" do - subject { described_class.new(@crlf_csv).csv_rows } + let(:csv) { "header 1,header 2,header 3\r\ncontent 1,content 2,content 3\r\n" } it "parses the CSV correctly" do - expect(subject.first).to eq(@parsed_csv) + expect(csv_preview_service.csv_rows.first).to eq(parsed_csv) end end context "with long CSV" do - subject { described_class.new(@long_csv).csv_rows } + let(:csv) do + val = "header 1,header 2,header 3\ncontent 1,content 2,content 3\n" + (described_class::MAXIMUM_ROWS + 10).times do + val += "other content 1,other content 2,other content 3\n" + end + val + end it "returns only the header row and less or equal to the maximum number of normal rows" do - expect(subject.first.length).to be <= (described_class::MAXIMUM_ROWS + 1) + expect(csv_preview_service.csv_rows.first.length).to be <= (described_class::MAXIMUM_ROWS + 1) end it "indicates that it is truncated" do - expect(subject.second).to be_truthy + expect(csv_preview_service.csv_rows.second).to be_truthy end end context "with CSV containing many columns" do - subject { described_class.new(@csv_with_many_columns).csv_rows } + let(:csv) do + val = "column" + (described_class::MAXIMUM_COLUMNS + 10).times do + val += ",column" + end + val += "\n" + end it "returns only less than or equal the maximum number of columns" do - expect(subject.first.first.length).to be <= described_class::MAXIMUM_COLUMNS + expect(csv_preview_service.csv_rows.first.first.length).to be <= described_class::MAXIMUM_COLUMNS end it "indicates that it is truncated" do - expect(subject.second).to be_truthy + expect(csv_preview_service.csv_rows.second).to be_truthy end end context "with CSV in UTF-8" do - subject do - described_class.new("za\u017C\u00F3\u0142\u0107 g\u0119\u015Bl\u0105 ja\u017A\u0144\n").csv_rows - end + let(:csv) { "za\u017C\u00F3\u0142\u0107 g\u0119\u015Bl\u0105 ja\u017A\u0144\n" } it "parses the CSV correctly" do - expect(subject.first).to eq([[{ text: "za\u017C\u00F3\u0142\u0107 g\u0119\u015Bl\u0105 ja\u017A\u0144" }]]) + expect(csv_preview_service.csv_rows.first).to eq([[{ text: "za\u017C\u00F3\u0142\u0107 g\u0119\u015Bl\u0105 ja\u017A\u0144" }]]) end end context "with CSV in windows-1252" do - subject do - described_class.new("\xFE\xE6r he feoll his tw\xE6gen gebro\xF0ra\n").csv_rows - end + let(:csv) { "\xFE\xE6r he feoll his tw\xE6gen gebro\xF0ra\n" } it "parses the CSV and convert it to UTF-8" do - expect(subject.first).to eq([[{ text: "\u00FE\u00E6r he feoll his tw\u00E6gen gebro\u00F0ra" }]]) + expect(csv_preview_service.csv_rows.first).to eq([[{ text: "\u00FE\u00E6r he feoll his tw\u00E6gen gebro\u00F0ra" }]]) end end context "with CSV with bytes that cannot be converted to UTF-8" do - subject { described_class.new("F\x8Ed\x8Eration Fran\x8Daise\n") } + let(:csv) { "F\x8Ed\x8Eration Fran\x8Daise\n" } it "raises FileEncodingError" do - expect { subject.csv_rows }.to raise_error(described_class::FileEncodingError) + expect { csv_preview_service.csv_rows }.to raise_error(described_class::FileEncodingError) end end end describe "#newline_or_last_char_index" do - subject { described_class.new("") } + let(:csv) { "" } context "when newline index is less than index of last newline" do it "returns the correct index" do - expect(subject.send(:newline_or_last_char_index, "a\nb\ncdef\ngh\nijk", 2)).to eq(8) + expect(csv_preview_service.send(:newline_or_last_char_index, "a\nb\ncdef\ngh\nijk", 2)).to eq(8) end end context "when newline index is equal to index of last newline" do it "returns the correct index" do - expect(subject.send(:newline_or_last_char_index, "a\nb\ncdef\ngh\n", 3)).to eq(11) + expect(csv_preview_service.send(:newline_or_last_char_index, "a\nb\ncdef\ngh\n", 3)).to eq(11) end end context "when newline index is greater than index of last newline" do it "returns the correct index" do - expect(subject.send(:newline_or_last_char_index, "a\nb\ncdef\ngh\nijk", 10)).to eq(14) + expect(csv_preview_service.send(:newline_or_last_char_index, "a\nb\ncdef\ngh\nijk", 10)).to eq(14) end end end describe "#truncate_to_maximum_number_of_lines" do - subject { described_class.new("") } + let(:csv) { "" } context "when requested number of lines is less than actual number of lines" do it "returns the correct string" do - expect(subject.send(:truncate_to_maximum_number_of_lines, "a\nb\ncdef\ngh\nijk", 3)).to eq("a\nb\ncdef\n") + expect(csv_preview_service.send(:truncate_to_maximum_number_of_lines, "a\nb\ncdef\ngh\nijk", 3)).to eq("a\nb\ncdef\n") end end context "when requested number of lines is equal to actual number of lines" do it "returns the correct string" do - expect(subject.send(:truncate_to_maximum_number_of_lines, "a\nb\ncdef\ngh\n", 4)).to eq("a\nb\ncdef\ngh\n") + expect(csv_preview_service.send(:truncate_to_maximum_number_of_lines, "a\nb\ncdef\ngh\n", 4)).to eq("a\nb\ncdef\ngh\n") end end context "when requested number of lines is greater than actual number of lines" do it "returns the correct string" do - expect(subject.send(:truncate_to_maximum_number_of_lines, "a\nb\ncdef\ngh\nijk", 10)).to eq("a\nb\ncdef\ngh\nijk") + expect(csv_preview_service.send(:truncate_to_maximum_number_of_lines, "a\nb\ncdef\ngh\nijk", 10)).to eq("a\nb\ncdef\ngh\nijk") end end end From 00692de35c81c10f263b1fd5976d6511fb83f112 Mon Sep 17 00:00:00 2001 From: Keith Lawrence Date: Mon, 16 Dec 2024 11:59:15 +0000 Subject: [PATCH 16/19] lint: fix spec/unit/simple_smart_answers --- spec/unit/simple_smart_answers/flow_spec.rb | 236 +++++++++----------- spec/unit/simple_smart_answers/node_spec.rb | 192 ++++++++-------- 2 files changed, 204 insertions(+), 224 deletions(-) diff --git a/spec/unit/simple_smart_answers/flow_spec.rb b/spec/unit/simple_smart_answers/flow_spec.rb index 6c277b8879..50472c23d2 100644 --- a/spec/unit/simple_smart_answers/flow_spec.rb +++ b/spec/unit/simple_smart_answers/flow_spec.rb @@ -1,141 +1,125 @@ -module SimpleSmartAnswers - RSpec.describe Flow do - context "finding nodes" do - before do - @nodes = [ - { - "kind" => "question", - "slug" => "question-1", - "title" => "Question 1", - "body" => "

This is question 1

", - "options" => [], - }, - { - "kind" => "question", - "slug" => "question-2", - "title" => "Question 2", - "body" => "

This is question 2

", - "options" => [], - }, - { - "kind" => "outcome", - "slug" => "outcome-1", - "title" => "Outcome 1", - "body" => "

This is outcome 1

", - }, - { - "kind" => "outcome", - "slug" => "outcome-1", - "title" => "Outcome 1", - "body" => "

This is outcome 1

", - }, - ] - @flow = described_class.new(@nodes) - end +RSpec.describe SimpleSmartAnswers::Flow do + subject(:flow) { described_class.new(nodes) } - it "returns the node matching the slug" do - node = @flow.node_for_slug("question-2") + context "when finding nodes" do + let(:nodes) do + [ + { + "kind" => "question", + "slug" => "question-1", + "title" => "Question 1", + "body" => "

This is question 1

", + "options" => [], + }, + { + "kind" => "question", + "slug" => "question-2", + "title" => "Question 2", + "body" => "

This is question 2

", + "options" => [], + }, + ] + end + + it "returns the node matching the slug" do + node = flow.node_for_slug("question-2") - expect(node).to be_a(Node) - expect(node.title).to eq("Question 2") - end + expect(node).to be_a(SimpleSmartAnswers::Node) + expect(node.title).to eq("Question 2") + end - it "returns nil if none match" do - expect(@flow.node_for_slug("question-3")).to be_nil - end + it "returns nil if none match" do + expect(flow.node_for_slug("question-3")).to be_nil + end - it "returns the first node as the start_node" do - node = @flow.start_node + it "returns the first node as the start_node" do + node = flow.start_node - expect(node).to be_a(Node) - expect(node.title).to eq("Question 1") - end + expect(node).to be_a(SimpleSmartAnswers::Node) + expect(node.title).to eq("Question 1") end + end - describe "#state_for_responses" do - before do - @flow = - described_class.new( - [ - { - "kind" => "question", - "slug" => "question-1", - "title" => "Question 1", - "body" => "

This is question 1

", - "options" => [ - { - "label" => "Option 1", - "slug" => "option-1", - "next_node" => "question-2", - }, - { - "label" => "Option 2", - "slug" => "option-2", - "next_node" => "outcome-1", - }, - { - "label" => "Option 3", - "slug" => "option-3", - "next_node" => "outcome-2", - }, - ], - }, - { - "kind" => "question", - "slug" => "question-2", - "title" => "Question 2", - "body" => "

This is question 2

", - "options" => [ - { - "label" => "Option 1", - "slug" => "option-1", - "next_node" => "outcome-1", - }, - { - "label" => "Option 2", - "slug" => "option-2", - "next_node" => "outcome-2", - }, - ], - }, - { - "kind" => "outcome", - "slug" => "outcome-1", - "title" => "Outcome 1", - "body" => "

This is outcome 1

", - }, - { - "kind" => "outcome", - "slug" => "outcome-2", - "title" => "Outcome 2", - "body" => "

This is outcome 2

", - }, - ], - ) - end + describe "#state_for_responses" do + let(:nodes) do + [ + { + "kind" => "question", + "slug" => "question-1", + "title" => "Question 1", + "body" => "

This is question 1

", + "options" => [ + { + "label" => "Option 1", + "slug" => "option-1", + "next_node" => "question-2", + }, + { + "label" => "Option 2", + "slug" => "option-2", + "next_node" => "outcome-1", + }, + { + "label" => "Option 3", + "slug" => "option-3", + "next_node" => "outcome-2", + }, + ], + }, + { + "kind" => "question", + "slug" => "question-2", + "title" => "Question 2", + "body" => "

This is question 2

", + "options" => [ + { + "label" => "Option 1", + "slug" => "option-1", + "next_node" => "outcome-1", + }, + { + "label" => "Option 2", + "slug" => "option-2", + "next_node" => "outcome-2", + }, + ], + }, + { + "kind" => "outcome", + "slug" => "outcome-1", + "title" => "Outcome 1", + "body" => "

This is outcome 1

", + }, + { + "kind" => "outcome", + "slug" => "outcome-2", + "title" => "Outcome 2", + "body" => "

This is outcome 2

", + }, + ] + end - it "returns a state for the given responses" do - state = @flow.state_for_responses(%w[option-1]) + it "returns a state for the given responses" do + state = flow.state_for_responses(%w[option-1]) - expect(state.current_node.slug).to eq("question-2") - expect(state.current_question_number).to eq(2) - expect(state.completed_questions.size).to eq(1) + expect(state.current_node.slug).to eq("question-2") + expect(state.current_question_number).to eq(2) + expect(state.completed_questions.size).to eq(1) - answer1 = state.completed_questions.first - expect(answer1.label).to eq("Option 1") - expect(answer1.slug).to eq("option-1") - expect(answer1.question.title).to eq("Question 1") - end + answer1 = state.completed_questions.first + expect(answer1.label).to eq("Option 1") + expect(answer1.slug).to eq("option-1") + expect(answer1.question.title).to eq("Question 1") + end - it "stops processing and set error flag on invalid response" do - state = @flow.state_for_responses(%w[option-1 fooey option-2]) + it "stops processing and set error flag on invalid response" do + state = flow.state_for_responses(%w[option-1 fooey option-2]) - expect(state.error?).to be true - expect(state.current_node.slug).to eq("question-2") - expect(state.current_question_number).to eq(2) - expect(state.completed_questions.size).to eq(1) - expect(state.completed_questions.first.slug).to eq("option-1") - end + expect(state.error?).to be true + expect(state.current_node.slug).to eq("question-2") + expect(state.current_question_number).to eq(2) + expect(state.completed_questions.size).to eq(1) + expect(state.completed_questions.first.slug).to eq("option-1") end end end diff --git a/spec/unit/simple_smart_answers/node_spec.rb b/spec/unit/simple_smart_answers/node_spec.rb index bc652b96c1..65a924f2a2 100644 --- a/spec/unit/simple_smart_answers/node_spec.rb +++ b/spec/unit/simple_smart_answers/node_spec.rb @@ -1,114 +1,110 @@ -module SimpleSmartAnswers - RSpec.describe Node do - it "has attribute accessors for basic fields" do - node = - described_class.new( - :a_flow, - "kind" => "question", - "slug" => "question-1", - "title" => "Question 1", - "body" => "

This is question 1

", - "options" => [], - ) - expect(node.kind).to eq("question") - expect(node.title).to eq("Question 1") - expect(node.slug).to eq("question-1") - expect(node.body).to eq("

This is question 1

") - end +RSpec.describe SimpleSmartAnswers::Node do + let(:flow) { instance_double(SimpleSmartAnswers::Flow, node_for_slug: :a_node) } - describe "#outcome?" do - it "is true for an outcome node" do - expect(described_class.new(:a_flow, "kind" => "outcome").outcome?).to be true - end + it "has attribute accessors for basic fields" do + node = + described_class.new( + :a_flow, + "kind" => "question", + "slug" => "question-1", + "title" => "Question 1", + "body" => "

This is question 1

", + "options" => [], + ) + expect(node.kind).to eq("question") + expect(node.title).to eq("Question 1") + expect(node.slug).to eq("question-1") + expect(node.body).to eq("

This is question 1

") + end - it "is false for a question node" do - expect(described_class.new(:a_flow, "kind" => "question").outcome?).to be false - end + describe "#outcome?" do + it "is true for an outcome node" do + expect(described_class.new(:a_flow, "kind" => "outcome").outcome?).to be true end - describe "#options" do - before do - @flow = instance_double(Flow, node_for_slug: :a_node) - @node = - described_class.new( - @flow, - "kind" => "question", - "slug" => "question-1", - "options" => [ - { - "label" => "Option 1", - "slug" => "option-1", - "next_node" => "question-2", - }, - { - "label" => "Option 3", - "slug" => "option-3", - "next_node" => "question-3", - }, - { - "label" => "Option 2", - "slug" => "option-2", - "next_node" => "question-2", - }, - ], - ) - end + it "is false for a question node" do + expect(described_class.new(:a_flow, "kind" => "question").outcome?).to be false + end + end - it "constructs options" do - expect(@node.options.map(&:label)).to eq(["Option 1", "Option 3", "Option 2"]) - expect(@node.options.map(&:slug)).to eq(%w[option-1 option-3 option-2]) - expect(@node.options.map(&:next_node_slug)).to eq(%w[question-2 question-3 question-2]) - expect(@node.options.first.question).to eq(@node) - end + describe "#options" do + let(:node) do + described_class.new( + flow, + "kind" => "question", + "slug" => "question-1", + "options" => [ + { + "label" => "Option 1", + "slug" => "option-1", + "next_node" => "question-2", + }, + { + "label" => "Option 3", + "slug" => "option-3", + "next_node" => "question-3", + }, + { + "label" => "Option 2", + "slug" => "option-2", + "next_node" => "question-2", + }, + ], + ) end - describe "#process_response" do - before do - @flow = instance_double(Flow, node_for_slug: :a_node) - @node = - described_class.new( - @flow, - "kind" => "question", - "slug" => "question-1", - "options" => [ - { - "label" => "Option 1", - "slug" => "option-1", - "next_node" => "question-2", - }, - { - "label" => "Option 3", - "slug" => "option-3", - "next_node" => "question-3", - }, - { - "label" => "Option 2", - "slug" => "option-2", - "next_node" => "question-2", - }, - ], - ) - end + it "constructs options" do + expect(node.options.map(&:label)).to eq(["Option 1", "Option 3", "Option 2"]) + expect(node.options.map(&:slug)).to eq(%w[option-1 option-3 option-2]) + expect(node.options.map(&:next_node_slug)).to eq(%w[question-2 question-3 question-2]) + expect(node.options.first.question).to eq(node) + end + end - it "returns the matching option instance" do - expect(@node.process_response("option-2").label).to eq("Option 2") - end + describe "#process_response" do + let(:node) do + described_class.new( + flow, + "kind" => "question", + "slug" => "question-1", + "options" => [ + { + "label" => "Option 1", + "slug" => "option-1", + "next_node" => "question-2", + }, + { + "label" => "Option 3", + "slug" => "option-3", + "next_node" => "question-3", + }, + { + "label" => "Option 2", + "slug" => "option-2", + "next_node" => "question-2", + }, + ], + ) + end - it "populates the next_node on the returned option" do - allow(@flow).to receive(:node_for_slug).with("question-2").and_return(:question_2_node) + it "returns the matching option instance" do + expect(node.process_response("option-2").label).to eq("Option 2") + end - expect(@node.process_response("option-2").next_node).to eq(:question_2_node) - end + it "populates the next_node on the returned option" do + allow(flow).to receive(:node_for_slug).with("question-2").and_return(:question_2_node) - it "raises InvalidResponse if no option matches" do - expect { @node.process_response("option-4") }.to raise_error(InvalidResponse) - end + expect(node.process_response("option-2").next_node).to eq(:question_2_node) + end + + it "raises InvalidResponse if no option matches" do + expect { node.process_response("option-4") }.to raise_error(SimpleSmartAnswers::InvalidResponse) + end - it "raises InvalidResponse if option points to a non-existent node" do - allow(@flow).to receive(:node_for_slug).with("question-2").and_return(nil) + it "raises InvalidResponse if option points to a non-existent node" do + allow(flow).to receive(:node_for_slug).with("question-2").and_return(nil) - expect { @node.process_response("option-2") }.to raise_error(InvalidResponse) - end + expect { node.process_response("option-2") }.to raise_error(SimpleSmartAnswers::InvalidResponse) end end end From 43cfcf1cd0433ec5312c5b3955e606e8da8e9b80 Mon Sep 17 00:00:00 2001 From: Keith Lawrence Date: Mon, 16 Dec 2024 13:45:04 +0000 Subject: [PATCH 17/19] lint: fix spec/unit/calendar --- .../division_spec.rb} | 128 +++++++++--------- .../event_spec.rb} | 4 +- .../year_spec.rb} | 34 ++--- 3 files changed, 85 insertions(+), 81 deletions(-) rename spec/unit/{calendar_division_spec.rb => calendar/division_spec.rb} (58%) rename spec/unit/{calendar_event_spec.rb => calendar/event_spec.rb} (92%) rename spec/unit/{calendar_year_spec.rb => calendar/year_spec.rb} (84%) diff --git a/spec/unit/calendar_division_spec.rb b/spec/unit/calendar/division_spec.rb similarity index 58% rename from spec/unit/calendar_division_spec.rb rename to spec/unit/calendar/division_spec.rb index 259da3479e..29bbf6b00d 100644 --- a/spec/unit/calendar_division_spec.rb +++ b/spec/unit/calendar/division_spec.rb @@ -25,8 +25,9 @@ it "construct a year for each one in the data" do div = described_class.new("something", "2012" => [1, 2], "2013" => [3, 4]) - expect(Calendar::Year).to receive(:new).with("2012", div, [1, 2]).and_return(:y_2012) - expect(Calendar::Year).to receive(:new).with("2013", div, [3, 4]).and_return(:y_2013) + allow(Calendar::Year).to receive(:new).with("2012", div, [1, 2]).and_return(:y_2012) + allow(Calendar::Year).to receive(:new).with("2013", div, [3, 4]).and_return(:y_2013) + expect(div.years).to eq(%i[y_2012 y_2013]) end @@ -48,124 +49,126 @@ expect(div.years).to eq(%i[y_2012 y_2013]) end - context "finding a year by name" do - before do - @div = described_class.new("something", "title" => "A Division", "2012" => [1, 2], "2013" => [3, 4]) - end + context "when finding a year by name" do + subject(:div) { described_class.new("something", "title" => "A Division", "2012" => [1, 2], "2013" => [3, 4]) } it "returns the year with the matching name" do - y = @div.year("2013") + y = div.year("2013") expect(y.class).to eq(Calendar::Year) expect(y.to_s).to eq("2013") end it "raises an exception when division doesn't exist" do - expect { @div.year("non-existent") }.to raise_error(Calendar::CalendarNotFound) + expect { div.year("non-existent") }.to raise_error(Calendar::CalendarNotFound) end end end describe "#events" do + let(:div) { described_class.new("something") } + let(:years) { [] } + before do - @years = [] - @div = described_class.new("something") - allow(@div).to receive(:years).and_return(@years) + allow(div).to receive(:years).and_return(years) end it "merges events for all years into single array" do - @years << instance_double(Calendar::Year, events: [1, 2]) - @years << instance_double(Calendar::Year, events: [3, 4, 5]) - @years << instance_double(Calendar::Year, events: [6, 7]) + years << instance_double(Calendar::Year, events: [1, 2]) + years << instance_double(Calendar::Year, events: [3, 4, 5]) + years << instance_double(Calendar::Year, events: [6, 7]) - expect(@div.events).to eq([1, 2, 3, 4, 5, 6, 7]) + expect(div.events).to eq([1, 2, 3, 4, 5, 6, 7]) end it "handles years with no events" do - @years << instance_double(Calendar::Year, events: [1, 2]) - @years << instance_double(Calendar::Year, events: []) - @years << instance_double(Calendar::Year, events: [6, 7]) + years << instance_double(Calendar::Year, events: [1, 2]) + years << instance_double(Calendar::Year, events: []) + years << instance_double(Calendar::Year, events: [6, 7]) - expect(@div.events).to eq([1, 2, 6, 7]) + expect(div.events).to eq([1, 2, 6, 7]) end end describe "#upcoming_event" do + let(:div) { described_class.new("something") } + let(:years) { [] } + before do - @years = [] - @div = described_class.new("something") - allow(@div).to receive(:years).and_return(@years) + allow(div).to receive(:years).and_return(years) end it "returns nil with no years" do - expect(@div.upcoming_event).to be_nil + expect(div.upcoming_event).to be_nil end it "returns nil if no years have upcoming_events" do - @years << instance_double(Calendar::Year, upcoming_event: nil) - @years << instance_double(Calendar::Year, upcoming_event: nil) + years << instance_double(Calendar::Year, upcoming_event: nil) + years << instance_double(Calendar::Year, upcoming_event: nil) - expect(@div.upcoming_event).to be_nil + expect(div.upcoming_event).to be_nil end it "returns the upcoming event for the first year that has one" do - @years << instance_double(Calendar::Year, upcoming_event: nil) - @years << instance_double(Calendar::Year, upcoming_event: :event_1) - @years << instance_double(Calendar::Year, upcoming_event: :event_2) + years << instance_double(Calendar::Year, upcoming_event: nil) + years << instance_double(Calendar::Year, upcoming_event: :event_1) + years << instance_double(Calendar::Year, upcoming_event: :event_2) - expect(@div.upcoming_event).to eq(:event_1) + expect(div.upcoming_event).to eq(:event_1) end it "caches the event" do y1 = instance_double(Calendar::Year) y2 = instance_double(Calendar::Year, upcoming_event: :event_1) - @years << y1 - @years << y2 + years << y1 + years << y2 expect(y1).to receive(:upcoming_event).once.and_return(nil) - @div.upcoming_event + div.upcoming_event - expect(@div.upcoming_event).to eq(:event_1) + expect(div.upcoming_event).to eq(:event_1) end end describe "#upcoming_events_by_year" do + let(:div) { described_class.new("something") } + let(:years) { [] } + before do - @years = [] - @div = described_class.new("something") - allow(@div).to receive(:years).and_return(@years) + allow(div).to receive(:years).and_return(years) end it "returns a hash of year => events for upcoming events" do y1 = instance_double(Calendar::Year, upcoming_events: %i[e1 e2]) y2 = instance_double(Calendar::Year, upcoming_events: %i[e3 e4 e5]) - ((@years << y1) << y2) + ((years << y1) << y2) expected = { y1 => %i[e1 e2], y2 => %i[e3 e4 e5] } - expect(@div.upcoming_events_by_year).to eq(expected) + expect(div.upcoming_events_by_year).to eq(expected) end it "does not include any years with no upcoming events" do y1 = instance_double(Calendar::Year, upcoming_events: []) y2 = instance_double(Calendar::Year, upcoming_events: %i[e1 e2 e3]) - ((@years << y1) << y2) + ((years << y1) << y2) expected = { y2 => %i[e1 e2 e3] } - expect(@div.upcoming_events_by_year).to eq(expected) + expect(div.upcoming_events_by_year).to eq(expected) end end describe "#past_events_by_year" do + let(:div) { described_class.new("something") } + let(:years) { [] } + before do - @years = [] - @div = described_class.new("something") - allow(@div).to receive(:years).and_return(@years) + allow(div).to receive(:years).and_return(years) end it "returns a hash of year => reversed events for past events" do y1 = instance_double(Calendar::Year, past_events: %i[e1 e2]) y2 = instance_double(Calendar::Year, past_events: %i[e3 e4 e5]) - ((@years << y1) << y2) + ((years << y1) << y2) expected = { y1 => %i[e2 e1], y2 => %i[e5 e4 e3] } - events_by_year = @div.past_events_by_year + events_by_year = div.past_events_by_year expect(events_by_year).to eq(expected) expect(events_by_year.keys).to eq([y2, y1]) @@ -174,51 +177,52 @@ it "does not include any years with no past events" do y1 = instance_double(Calendar::Year, past_events: %i[e1 e2]) y2 = instance_double(Calendar::Year, past_events: []) - ((@years << y1) << y2) + ((years << y1) << y2) expected = { y1 => %i[e2 e1] } - expect(@div.past_events_by_year).to eq(expected) + expect(div.past_events_by_year).to eq(expected) end end describe "#show_bunting?" do - before { @div = described_class.new("something") } + let(:div) { described_class.new("something") } it "is true if there is a buntable bank holiday today" do - @event = Calendar::Event.new("bunting" => true, "date" => Time.zone.today) - allow(@div).to receive(:upcoming_event).and_return(@event) + event = Calendar::Event.new("bunting" => true, "date" => Time.zone.today) + allow(div).to receive(:upcoming_event).and_return(event) - expect(@div.show_bunting?).to be true + expect(div.show_bunting?).to be true end it "is false if there is a non-buntable bank holiday today" do - @event = Calendar::Event.new("bunting" => false, "date" => Time.zone.today) - allow(@div).to receive(:upcoming_event).and_return(@event) + event = Calendar::Event.new("bunting" => false, "date" => Time.zone.today) + allow(div).to receive(:upcoming_event).and_return(event) - expect(@div.show_bunting?).to be false + expect(div.show_bunting?).to be false end it "is false if there is no bank holiday today" do - @event = Calendar::Event.new("bunting" => true, "date" => Time.zone.today + 1.week) - allow(@div).to receive(:upcoming_event).and_return(@event) + event = Calendar::Event.new("bunting" => true, "date" => Time.zone.today + 1.week) + allow(div).to receive(:upcoming_event).and_return(event) - expect(@div.show_bunting?).to be false + expect(div.show_bunting?).to be false end end describe "#as_json" do - before { @div = described_class.new("something") } + let(:div) { described_class.new("something") } it "returns division slug" do - hash = @div.as_json + hash = div.as_json + expect(hash["division"]).to eq("something") end it "returns all events from all years" do y1 = instance_double(Calendar::Year, events: [1, 2]) y2 = instance_double(Calendar::Year, events: [3, 4]) - allow(@div).to receive(:years).and_return([y1, y2]) - hash = @div.as_json + allow(div).to receive(:years).and_return([y1, y2]) + hash = div.as_json expect(hash["events"]).to eq([1, 2, 3, 4]) end diff --git a/spec/unit/calendar_event_spec.rb b/spec/unit/calendar/event_spec.rb similarity index 92% rename from spec/unit/calendar_event_spec.rb rename to spec/unit/calendar/event_spec.rb index 1bdf9f0829..e7633bb44e 100644 --- a/spec/unit/calendar_event_spec.rb +++ b/spec/unit/calendar/event_spec.rb @@ -1,5 +1,5 @@ RSpec.describe Calendar::Event do - context "construction" do + describe "#date" do it "parses a date given as a string" do e = described_class.new("date" => "2012-02-04") @@ -13,7 +13,7 @@ end end - context "as_json in English" do + describe "#as_json" do it "returns a hash representation" do I18n.locale = :en e = described_class.new("title" => "bank_holidays.new_year", "date" => "02/01/2012", "notes" => "common.substitute_day", "bunting" => true) diff --git a/spec/unit/calendar_year_spec.rb b/spec/unit/calendar/year_spec.rb similarity index 84% rename from spec/unit/calendar_year_spec.rb rename to spec/unit/calendar/year_spec.rb index 1497b17f69..12bcda1dbc 100644 --- a/spec/unit/calendar_year_spec.rb +++ b/spec/unit/calendar/year_spec.rb @@ -4,8 +4,8 @@ end describe "#events" do - before do - @y = described_class.new("1234", :a_division, [ + let(:y) do + described_class.new("1234", :a_division, [ { "title" => "bank_holidays.new_year", "date" => "02/01/2012" }, { "title" => "bank_holidays.good_friday", "date" => "27/08/2012" }, ]) @@ -15,14 +15,14 @@ foo = Calendar::Event.new("title" => "bank_holidays.new_year", "date" => Date.civil(2012, 1, 2)) bar = Calendar::Event.new("title" => "bank_holidays.good_friday", "date" => Date.civil(2012, 8, 27)) - expect(@y.events).to eq([foo, bar]) + expect(y.events).to eq([foo, bar]) end it "caches the constructed instances" do - first = @y.events + first = y.events expect(Calendar::Event).not_to receive(:new) - expect(@y.events).to eq(first) + expect(y.events).to eq(first) end end @@ -81,8 +81,8 @@ end describe "#upcoming_events" do - before do - @year = described_class.new("1234", :a_division, [ + let(:year) do + described_class.new("1234", :a_division, [ { "title" => "bank_holidays.new_year", "date" => "02/01/2012" }, { "title" => "bank_holidays.good_friday", "date" => "27/08/2012" }, { "title" => "bank_holidays.easter_monday", "date" => "16/10/2012" }, @@ -91,22 +91,22 @@ it "returns all future events including today" do Timecop.travel("2012-08-27") do - expect(@year.upcoming_events.map(&:title)).to eq(["Good Friday", "Easter Monday"]) + expect(year.upcoming_events.map(&:title)).to eq(["Good Friday", "Easter Monday"]) end end it "caches the result" do - @year.upcoming_events + year.upcoming_events - expect(@year).not_to receive(:events) + expect(year).not_to receive(:events) - @year.upcoming_events + year.upcoming_events end end describe "#past_events" do - before do - @year = described_class.new("1234", :a_division, [ + let(:year) do + described_class.new("1234", :a_division, [ { "title" => "bank_holidays.new_year", "date" => "02/01/2012" }, { "title" => "bank_holidays.good_friday", "date" => "27/08/2012" }, { "title" => "bank_holidays.easter_monday", "date" => "16/10/2012" }, @@ -115,16 +115,16 @@ it "returns all past events excluding today" do Timecop.travel("2012-08-27") do - expect(@year.past_events.map(&:title)).to eq(["New Year’s Day"]) + expect(year.past_events.map(&:title)).to eq(["New Year’s Day"]) end end it "caches the result" do - @year.past_events + year.past_events - expect(@year).not_to receive(:events) + expect(year).not_to receive(:events) - @year.past_events + year.past_events end end end From 028b41c858921b7681e27d708e23b371d7413e72 Mon Sep 17 00:00:00 2001 From: Keith Lawrence Date: Mon, 16 Dec 2024 14:17:41 +0000 Subject: [PATCH 18/19] lint: fix spec/unit/sanitiser - rename file into sanitiser folder to match module tree. --- spec/unit/{ => sanitiser}/strategy_spec.rb | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename spec/unit/{ => sanitiser}/strategy_spec.rb (100%) diff --git a/spec/unit/strategy_spec.rb b/spec/unit/sanitiser/strategy_spec.rb similarity index 100% rename from spec/unit/strategy_spec.rb rename to spec/unit/sanitiser/strategy_spec.rb From 4d85fb205a4a6088ddba650fa0174133126ae6d0 Mon Sep 17 00:00:00 2001 From: Keith Lawrence Date: Wed, 8 Jan 2025 13:37:34 +0000 Subject: [PATCH 19/19] lint: fix spec/unit - One lint (RSpec/SubjectStub) is disabled in two classes rather than being fixed, because fixing it really requires a rethink about the actual classes, and this set of commits is scoped to just making the lint pass. We should revisit how these two classes work later (especially the call out to mtime in ics_renderer) --- .../unit/api_error_routing_constraint_spec.rb | 9 +- spec/unit/calendar_spec.rb | 104 +++++++++--------- spec/unit/content_item_loader_spec.rb | 27 ++--- spec/unit/format_routing_constraint_spec.rb | 2 +- ...ull_path_format_routing_constraint_spec.rb | 2 +- spec/unit/ics_renderer_spec.rb | 79 ++++++------- spec/unit/locales_validation_spec.rb | 10 +- spec/unit/postcode_sanitizer_spec.rb | 10 +- 8 files changed, 121 insertions(+), 122 deletions(-) diff --git a/spec/unit/api_error_routing_constraint_spec.rb b/spec/unit/api_error_routing_constraint_spec.rb index a00ea9d48a..56a8bcbcad 100644 --- a/spec/unit/api_error_routing_constraint_spec.rb +++ b/spec/unit/api_error_routing_constraint_spec.rb @@ -1,18 +1,19 @@ RSpec.describe ApiErrorRoutingConstraint do include ContentStoreHelpers - let(:subject) { described_class.new } - let(:request) { double(path: "/slug", env: {}) } + subject(:api_error_routing_constraint) { described_class.new } + + let(:request) { instance_double(ActionDispatch::Request, path: "/slug", env: {}) } it "returns true if there's a cached error" do stub_content_store_does_not_have_item("/slug") - expect(subject.matches?(request)).to be true + expect(api_error_routing_constraint.matches?(request)).to be true end it "returns false if there was no error in API calls" do stub_content_store_has_item("/slug") - expect(subject.matches?(request)).to be false + expect(api_error_routing_constraint.matches?(request)).to be false end end diff --git a/spec/unit/calendar_spec.rb b/spec/unit/calendar_spec.rb index 6cf29e5e58..d2b1fb1e46 100644 --- a/spec/unit/calendar_spec.rb +++ b/spec/unit/calendar_spec.rb @@ -5,10 +5,10 @@ mock_calendar_fixtures end - context "finding a calendar by slug" do + context "when finding a calendar by slug" do it "constructs a calendar with the slug and data from the corresponding JSON file" do data_from_json = JSON.parse(File.read(Rails.root.join(Calendar::REPOSITORY_PATH, "single-calendar.json"))) - expect(described_class).to receive(:new).with("single-calendar", data_from_json).and_return(:a_calendar) + allow(described_class).to receive(:new).with("single-calendar", data_from_json).and_return(:a_calendar) cal = described_class.find("single-calendar") expect(cal).to eq(:a_calendar) @@ -28,8 +28,8 @@ end describe "#divisions" do - before do - @cal = described_class.new( + subject(:calendar) do + described_class.new( "a-calendar", "title" => "UK bank holidays", "divisions" => { @@ -41,114 +41,116 @@ end it "constructs a division for each one in the data" do - expect(Calendar::Division).to receive(:new).with("england-and-wales", { "2012" => [1], "2013" => [3] }).and_return(:england_and_wales) - expect(Calendar::Division).to receive(:new).with("scotland", { "2012" => [1, 2], "2013" => [3, 4] }).and_return(:scotland) - expect(Calendar::Division).to receive(:new).with("northern-ireland", { "2012" => [2], "2013" => [4] }).and_return(:northern_ireland) - expect(@cal.divisions).to eq(%i[england_and_wales scotland northern_ireland]) + allow(Calendar::Division).to receive(:new).with("england-and-wales", { "2012" => [1], "2013" => [3] }).and_return(:england_and_wales) + allow(Calendar::Division).to receive(:new).with("scotland", { "2012" => [1, 2], "2013" => [3, 4] }).and_return(:scotland) + allow(Calendar::Division).to receive(:new).with("northern-ireland", { "2012" => [2], "2013" => [4] }).and_return(:northern_ireland) + + expect(calendar.divisions).to eq(%i[england_and_wales scotland northern_ireland]) end it "caches the constructed instances" do - first = @cal.divisions + first = calendar.divisions expect(Calendar::Division).not_to receive(:new) - expect(@cal.divisions).to eq(first) + expect(calendar.divisions).to eq(first) end - context "finding a division by slug" do + context "when finding a division by slug" do it "returns the division with the matching slug" do - div = @cal.division("england-and-wales") + div = calendar.division("england-and-wales") expect(div.class).to eq(Calendar::Division) expect(div.title).to eq("England and wales") end it "raises exception when division doesn't exist" do - expect { @cal.division("non-existent") }.to raise_error(Calendar::CalendarNotFound) + expect { calendar.division("non-existent") }.to raise_error(Calendar::CalendarNotFound) end end end describe "#events" do + subject(:calendar) { described_class.new("a-calendar") } + + let(:divisions) { [] } + before do - @divisions = [] - @calendar = described_class.new("a-calendar") - allow(@calendar).to receive(:divisions).and_return(@divisions) + allow(calendar).to receive(:divisions).and_return(divisions) # rubocop:disable RSpec/SubjectStub end it "merges events for all years into single array" do - @divisions << instance_double(Calendar::Division, events: [1, 2]) - @divisions << instance_double(Calendar::Division, events: [3, 4, 5]) - @divisions << instance_double(Calendar::Division, events: [6, 7]) + divisions << instance_double(Calendar::Division, events: [1, 2]) + divisions << instance_double(Calendar::Division, events: [3, 4, 5]) + divisions << instance_double(Calendar::Division, events: [6, 7]) - expect(@calendar.events).to eq([1, 2, 3, 4, 5, 6, 7]) + expect(calendar.events).to eq([1, 2, 3, 4, 5, 6, 7]) end it "handles years with no events" do - @divisions << instance_double(Calendar::Division, events: [1, 2]) - @divisions << instance_double(Calendar::Division, events: []) - @divisions << instance_double(Calendar::Division, events: [6, 7]) + divisions << instance_double(Calendar::Division, events: [1, 2]) + divisions << instance_double(Calendar::Division, events: []) + divisions << instance_double(Calendar::Division, events: [6, 7]) - expect(@calendar.events).to eq([1, 2, 6, 7]) + expect(calendar.events).to eq([1, 2, 6, 7]) end end - context "attribute accessors" do - before do - @cal = described_class.new("a-calendar", "title" => "bank_holidays.calendar.title", "description" => "bank_holidays.calendar.description") - end + describe "attribute accessors" do + subject(:calendar) { described_class.new("a-calendar", "title" => "bank_holidays.calendar.title", "description" => "bank_holidays.calendar.description") } it "has an accessor for the title" do - expect(@cal.title).to eq("UK bank holidays") + expect(calendar.title).to eq("UK bank holidays") end it "has an accessor for the description" do - expect(@cal.description).to eq("Find out when bank holidays are in England, Wales, Scotland and Northern Ireland - including past and future bank holidays") + expect(calendar.description).to eq("Find out when bank holidays are in England, Wales, Scotland and Northern Ireland - including past and future bank holidays") end end describe "#show_bunting?" do - before { @cal = described_class.new("a-calendar") } + subject(:calendar) { described_class.new("a-calendar") } it "is true when one division is buntable" do - @div1 = instance_double(Calendar::Division, show_bunting?: true) - @div2 = instance_double(Calendar::Division, show_bunting?: false) - @div3 = instance_double(Calendar::Division, show_bunting?: false) - allow(@cal).to receive(:divisions).and_return([@div1, @div2, @div3]) + div1 = instance_double(Calendar::Division, show_bunting?: true) + div2 = instance_double(Calendar::Division, show_bunting?: false) + div3 = instance_double(Calendar::Division, show_bunting?: false) + allow(calendar).to receive(:divisions).and_return([div1, div2, div3]) # rubocop:disable RSpec/SubjectStub - expect(@cal.show_bunting?).to be true + expect(calendar.show_bunting?).to be true end it "is true when more than one division is buntable" do - @div1 = instance_double(Calendar::Division, show_bunting?: true) - @div2 = instance_double(Calendar::Division, show_bunting?: true) - @div3 = instance_double(Calendar::Division, show_bunting?: false) - allow(@cal).to receive(:divisions).and_return([@div1, @div2, @div3]) + div1 = instance_double(Calendar::Division, show_bunting?: true) + div2 = instance_double(Calendar::Division, show_bunting?: true) + div3 = instance_double(Calendar::Division, show_bunting?: false) + allow(calendar).to receive(:divisions).and_return([div1, div2, div3]) # rubocop:disable RSpec/SubjectStub - expect(@cal.show_bunting?).to be true + expect(calendar.show_bunting?).to be true end it "is false when no divisions are buntable" do - @div1 = instance_double(Calendar::Division, show_bunting?: false) - @div2 = instance_double(Calendar::Division, show_bunting?: false) - @div3 = instance_double(Calendar::Division, show_bunting?: false) - allow(@cal).to receive(:divisions).and_return([@div1, @div2, @div3]) + div1 = instance_double(Calendar::Division, show_bunting?: false) + div2 = instance_double(Calendar::Division, show_bunting?: false) + div3 = instance_double(Calendar::Division, show_bunting?: false) + allow(calendar).to receive(:divisions).and_return([div1, div2, div3]) # rubocop:disable RSpec/SubjectStub - expect(@cal.show_bunting?).to be false + expect(calendar.show_bunting?).to be false end end describe "#as_json" do + subject(:calendar) { described_class.new("a-calendar") } + before do - @div1 = instance_double(Calendar::Division, slug: "division-1", as_json: "div1 json") - @div2 = instance_double(Calendar::Division, slug: "division-2", as_json: "div2 json") - @cal = described_class.new("a-calendar") - allow(@cal).to receive(:divisions).and_return([@div1, @div2]) + div1 = instance_double(Calendar::Division, slug: "division-1", as_json: "div1 json") + div2 = instance_double(Calendar::Division, slug: "division-2", as_json: "div2 json") + allow(calendar).to receive(:divisions).and_return([div1, div2]) # rubocop:disable RSpec/SubjectStub end it "construct a hash representation of all divisions" do expected = { "division-1" => "div1 json", "division-2" => "div2 json" } - expect(@cal.as_json).to eq(expected) + expect(calendar.as_json).to eq(expected) end end end diff --git a/spec/unit/content_item_loader_spec.rb b/spec/unit/content_item_loader_spec.rb index a2617f4953..e23ad21f13 100644 --- a/spec/unit/content_item_loader_spec.rb +++ b/spec/unit/content_item_loader_spec.rb @@ -1,13 +1,14 @@ RSpec.describe ContentItemLoader do include ContentStoreHelpers - let(:subject) { described_class.new } + subject(:content_item_loader) { described_class.new } + let!(:item_request) { stub_content_store_has_item("/my-random-item") } describe ".for_request" do it "returns a new object per request" do - request_1 = double(path: "/my-random-item", env: {}) - request_2 = double(path: "/my-random-item", env: {}) + request_1 = instance_double(ActionDispatch::Request, path: "/my-random-item", env: {}) + request_2 = instance_double(ActionDispatch::Request, path: "/my-random-item", env: {}) loader_1 = described_class.for_request(request_1) loader_2 = described_class.for_request(request_2) @@ -16,7 +17,7 @@ end it "returns the same object for the same request" do - request_1 = double(path: "/my-random-item", env: {}) + request_1 = instance_double(ActionDispatch::Request, path: "/my-random-item", env: {}) loader_1 = described_class.for_request(request_1) loader_2 = described_class.for_request(request_1) @@ -27,15 +28,15 @@ describe "#load" do it "caches calls to the content store" do - subject.load("/my-random-item") - subject.load("/my-random-item") + content_item_loader.load("/my-random-item") + content_item_loader.load("/my-random-item") expect(item_request).to have_been_made.once end it "restricts cache to the specific instance of the class, so does not cache across requests" do - request_1 = double(path: "/my-random-item", env: {}) - request_2 = double(path: "/my-random-item", env: {}) + request_1 = instance_double(ActionDispatch::Request, path: "/my-random-item", env: {}) + request_2 = instance_double(ActionDispatch::Request, path: "/my-random-item", env: {}) loader_1 = described_class.for_request(request_1) loader_2 = described_class.for_request(request_2) @@ -52,11 +53,11 @@ end it "returns (but does not raise) the original exception" do - expect(subject.load("/my-missing-item")).to be_a(GdsApi::HTTPErrorResponse) + expect(content_item_loader.load("/my-missing-item")).to be_a(GdsApi::HTTPErrorResponse) end end - context "With ALLOW_LOCAL_CONTENT_ITEM_OVERRIDE=true" do + context "with ALLOW_LOCAL_CONTENT_ITEM_OVERRIDE=true" do before do ENV["ALLOW_LOCAL_CONTENT_ITEM_OVERRIDE"] = "true" stub_const("ContentItemLoader::LOCAL_ITEMS_PATH", "spec/fixtures/local-content-items") @@ -70,7 +71,7 @@ let!(:item_request) { stub_content_store_has_item("/my-json-item") } it "loads content from the JSON file instead of the content store" do - response = subject.load("/my-json-item") + response = content_item_loader.load("/my-json-item") expect(item_request).not_to have_been_made expect(ContentItemFactory.build(response).schema_name).to eq("json_page") @@ -81,7 +82,7 @@ let!(:item_request) { stub_content_store_has_item("/my-yaml-item") } it "loads content from the YAML file instead of the content store" do - response = subject.load("/my-yaml-item") + response = content_item_loader.load("/my-yaml-item") expect(item_request).not_to have_been_made expect(ContentItemFactory.build(response).schema_name).to eq("yaml_page") @@ -92,7 +93,7 @@ let!(:item_request) { stub_content_store_has_item("/my-remote-item") } it "returns to loading from the content store" do - subject.load("/my-remote-item") + content_item_loader.load("/my-remote-item") expect(item_request).to have_been_made.once end diff --git a/spec/unit/format_routing_constraint_spec.rb b/spec/unit/format_routing_constraint_spec.rb index d85a6c6d5a..6703a7937a 100644 --- a/spec/unit/format_routing_constraint_spec.rb +++ b/spec/unit/format_routing_constraint_spec.rb @@ -2,7 +2,7 @@ include ContentStoreHelpers describe "#matches?" do - let(:request) { double(params: { slug: "slug" }, env: {}) } + let(:request) { instance_double(ActionDispatch::Request, params: { slug: "slug" }, env: {}) } context "when the content_store returns a document" do before do diff --git a/spec/unit/full_path_format_routing_constraint_spec.rb b/spec/unit/full_path_format_routing_constraint_spec.rb index 6acbdb36be..1aadd5c082 100644 --- a/spec/unit/full_path_format_routing_constraint_spec.rb +++ b/spec/unit/full_path_format_routing_constraint_spec.rb @@ -2,7 +2,7 @@ include ContentStoreHelpers describe "#matches?" do - let(:request) { double(path: "/format/routing/test", env: {}) } + let(:request) { instance_double(ActionDispatch::Request, path: "/format/routing/test", env: {}) } context "when the content_store returns a document" do before do diff --git a/spec/unit/ics_renderer_spec.rb b/spec/unit/ics_renderer_spec.rb index c060798c1f..1fa43d88dd 100644 --- a/spec/unit/ics_renderer_spec.rb +++ b/spec/unit/ics_renderer_spec.rb @@ -1,30 +1,33 @@ require "ics_renderer" RSpec.describe IcsRenderer do - context "generating complete ics file" do - it "generates correct ics header and footer" do - r = described_class.new([], "/foo/ics", :en) + subject(:ics_renderer) { described_class.new([], "/foo/ics", :en) } + context "when generating complete ics file" do + it "generates correct ics header and footer" do expected = "BEGIN:VCALENDAR\r\n" (expected << "VERSION:2.0\r\n") (expected << "METHOD:PUBLISH\r\n") (expected << "PRODID:-//uk.gov/GOVUK calendars//EN\r\n") (expected << "CALSCALE:GREGORIAN\r\n") (expected << "END:VCALENDAR\r\n") - expect(r.render).to eq(expected) + expect(ics_renderer.render).to eq(expected) end - it "handles locale correctly" do - r = described_class.new([], "/foo/ics", :cy) + context "and renderer is for Welsh" do + subject(:ics_renderer) { described_class.new([], "/foo/ics", :cy) } - expect(r.render).to match("PRODID:-//uk.gov/GOVUK calendars//CY") + it "handles locale correctly" do + expect(ics_renderer.render).to match("PRODID:-//uk.gov/GOVUK calendars//CY") + end end it "generates an event for each given event" do - r = described_class.new(%i[e1 e2], "/foo/ics", :en) + ics_renderer = described_class.new(%i[e1 e2], "/foo/ics", :en) + + allow(ics_renderer).to receive(:render_event).with(:e1).and_return("Event1 ics\r\n") + allow(ics_renderer).to receive(:render_event).with(:e2).and_return("Event2 ics\r\n") - expect(r).to receive(:render_event).with(:e1).and_return("Event1 ics\r\n") - expect(r).to receive(:render_event).with(:e2).and_return("Event2 ics\r\n") expected = "BEGIN:VCALENDAR\r\n" (expected << "VERSION:2.0\r\n") (expected << "METHOD:PUBLISH\r\n") @@ -33,21 +36,19 @@ (expected << "Event1 ics\r\n") (expected << "Event2 ics\r\n") (expected << "END:VCALENDAR\r\n") - expect(r.render).to eq(expected) + expect(ics_renderer.render).to eq(expected) end end - context "generating an event" do + context "when generating an event" do before do - @path = "/foo/ics" - @r = described_class.new([], @path, :en) - allow_any_instance_of(described_class).to receive(:dtstamp).and_return("20121017T0100Z") + allow(ics_renderer).to receive(:dtstamp).and_return("20121017T0100Z") # rubocop:disable RSpec/SubjectStub end it "generates an event" do e = Calendar::Event.new("title" => "bank_holidays.good_friday", "date" => "2012-04-14") - expect(Digest::MD5).to receive(:hexdigest).with(@path).once.and_return("hash") + expect(Digest::MD5).to receive(:hexdigest).with("/foo/ics").once.and_return("hash") expected = "BEGIN:VEVENT\r\n" (expected << "DTEND;VALUE=DATE:20120415\r\n") (expected << "DTSTART;VALUE=DATE:20120414\r\n") @@ -56,49 +57,51 @@ (expected << "SEQUENCE:0\r\n") (expected << "DTSTAMP:20121017T0100Z\r\n") (expected << "END:VEVENT\r\n") - expect(@r.render_event(e)).to eq(expected) + expect(ics_renderer.render_event(e)).to eq(expected) end end - context "generating a uid" do - before do - @path = "/foo/bar.ics" - @r = described_class.new([], @path, :en) - @hash = Digest::MD5.hexdigest(@path) - @first_event = Calendar::Event.new("title" => "bank_holidays.new_year", "date" => Date.new(1982, 5, 28)) - @second_event = Calendar::Event.new("title" => "bank_holidays.good_friday", "date" => Date.new(1984, 1, 16)) - end + describe "#uid" do + subject(:ics_renderer) { described_class.new([], path, :en) } + + let(:path) { "/foo/bar.ics" } + let(:hash) { Digest::MD5.hexdigest(path) } + + let(:first_event) { Calendar::Event.new("title" => "bank_holidays.new_year", "date" => Date.new(1982, 5, 28)) } + let(:second_event) { Calendar::Event.new("title" => "bank_holidays.good_friday", "date" => Date.new(1984, 1, 16)) } it "uses calendar path, event title and event date to create a uid" do - expect(@r.uid(@first_event)).to eq("#{@hash}-1982-05-28-NewYearsDay@gov.uk") + expect(ics_renderer.uid(first_event)).to eq("#{hash}-1982-05-28-NewYearsDay@gov.uk") end it "caches the hash generation" do - expect(Digest::MD5).to receive(:hexdigest).with(@path).once.and_return(@hash) - @r.uid(@first_event) - expect(@r.uid(@second_event)).to eq("#{@hash}-1984-01-16-GoodFriday@gov.uk") + expect(Digest::MD5).to receive(:hexdigest).with(path).once.and_return(hash) + + ics_renderer.uid(first_event) + + expect(ics_renderer.uid(second_event)).to eq("#{hash}-1984-01-16-GoodFriday@gov.uk") end end - context "generating dtstamp" do - before { @r = described_class.new([], "/foo/ics", :en) } - + describe "#dstamp" do it "returns the mtime of the REVISION file" do - expect(File).to receive(:mtime).with(Rails.root.join("REVISION")).and_return(Time.zone.parse("2012-04-06 14:53:54Z")) - expect(@r.dtstamp).to eq("20120406T145354Z") + allow(File).to receive(:mtime).with(Rails.root.join("REVISION")).and_return(Time.zone.parse("2012-04-06 14:53:54Z")) + + expect(ics_renderer.dtstamp).to eq("20120406T145354Z") end it "returns now if the file doesn't exist" do Timecop.freeze(Time.zone.parse("2012-11-27 16:13:27")) do - expect(File).to receive(:mtime).with(Rails.root.join("REVISION")).and_raise(Errno::ENOENT) - expect(@r.dtstamp).to eq("20121127T161327Z") + allow(File).to receive(:mtime).with(Rails.root.join("REVISION")).and_raise(Errno::ENOENT) + + expect(ics_renderer.dtstamp).to eq("20121127T161327Z") end end it "caches the result" do expect(File).to receive(:mtime).with(Rails.root.join("REVISION")).once.and_return(Time.zone.parse("2012-04-06 14:53:54Z")) - @r.dtstamp - expect(@r.dtstamp).to eq("20120406T145354Z") + ics_renderer.dtstamp + expect(ics_renderer.dtstamp).to eq("20120406T145354Z") end end end diff --git a/spec/unit/locales_validation_spec.rb b/spec/unit/locales_validation_spec.rb index 9293c00d33..8af4ef1bd6 100644 --- a/spec/unit/locales_validation_spec.rb +++ b/spec/unit/locales_validation_spec.rb @@ -1,15 +1,7 @@ RSpec.describe "Locales Validation" do - # Stop the checker putting it's jaunty message to stdout - around do |ex| - original_stdout = $stdout - $stdout = File.open(File::NULL, "w") - ex.run - $stdout = original_stdout - end - it "validates all locale files" do checker = RailsTranslationManager::LocaleChecker.new("config/locales/*.yml") - expect(checker.validate_locales).to be_truthy + expect { checker.validate_locales }.to output("Locale files are in sync, nice job!\n").to_stdout end end diff --git a/spec/unit/postcode_sanitizer_spec.rb b/spec/unit/postcode_sanitizer_spec.rb index f4d59c2359..d48c354040 100644 --- a/spec/unit/postcode_sanitizer_spec.rb +++ b/spec/unit/postcode_sanitizer_spec.rb @@ -1,5 +1,5 @@ RSpec.describe PostcodeSanitizer do - context "postcodes are sanitized before being sent to LocationsApi" do + describe "#sanitize" do it "strips trailing spaces from entered postcodes" do expect(described_class.sanitize("WC2B 6NH ")).to eq("WC2B 6NH") end @@ -11,11 +11,11 @@ it "transposes O/0 and I/1 if necessary" do expect(described_class.sanitize("WIA OAA")).to eq("W1A 0AA") end - end - context "if the postcode parameter is nil or an empty string" do - it "returns nil and does not try to perform sanitization" do - expect(described_class.sanitize("")).to be_nil + context "when the postcode parameter is nil or an empty string" do + it "returns nil and does not try to perform sanitization" do + expect(described_class.sanitize("")).to be_nil + end end end end