From 74bf0e5535272333b02c255e473ec06e73366857 Mon Sep 17 00:00:00 2001 From: Keith Lawrence Date: Thu, 12 Dec 2024 15:37:33 +0000 Subject: [PATCH 1/2] Add rspec config --- .rubocop.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.rubocop.yml b/.rubocop.yml index 26ed7fe..ec71f49 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -2,6 +2,7 @@ inherit_gem: rubocop-govuk: - config/default.yml + - config/rspec.yml inherit_mode: merge: From e5f105e70998af0c8ae0a8c7a6c43fe9f4916fca Mon Sep 17 00:00:00 2001 From: Keith Lawrence Date: Thu, 12 Dec 2024 15:38:40 +0000 Subject: [PATCH 2/2] Fix rspec lints - Move files into correct directories - Update contexts to desired wording - Simplify fixture changing --- spec/requests/recruitment_banner_spec.rb | 2 +- .../recruitment_banner_spec.rb | 16 +- .../validators/recruitment_banner_spec.rb | 148 ++++++++++++++++++ .../validators/recruitment_banner_spec.rb | 147 ----------------- 4 files changed, 158 insertions(+), 155 deletions(-) rename spec/units/{ => govuk_web_banners}/recruitment_banner_spec.rb (83%) create mode 100644 spec/units/govuk_web_banners/validators/recruitment_banner_spec.rb delete mode 100644 spec/units/validators/recruitment_banner_spec.rb diff --git a/spec/requests/recruitment_banner_spec.rb b/spec/requests/recruitment_banner_spec.rb index 90c3064..f03b91e 100644 --- a/spec/requests/recruitment_banner_spec.rb +++ b/spec/requests/recruitment_banner_spec.rb @@ -4,7 +4,7 @@ allow(YAML).to receive(:load_file).with(original_path).and_return(replacement_file) end - context "getting a path with the banner partial" do + context "when visiting a path with the banner partial" do let(:replacement_file) do YAML.load_file(Rails.root.join(__dir__, "../../spec/fixtures/requests_banners.yml")) end diff --git a/spec/units/recruitment_banner_spec.rb b/spec/units/govuk_web_banners/recruitment_banner_spec.rb similarity index 83% rename from spec/units/recruitment_banner_spec.rb rename to spec/units/govuk_web_banners/recruitment_banner_spec.rb index 89efc7a..19f4c9f 100644 --- a/spec/units/recruitment_banner_spec.rb +++ b/spec/units/govuk_web_banners/recruitment_banner_spec.rb @@ -1,13 +1,15 @@ RSpec.describe GovukWebBanners::RecruitmentBanner do + let(:fixtures_dir) { Rails.root.join(__dir__, "../../fixtures/") } + before do - original_path = Rails.root.join(__dir__, described_class::BANNER_CONFIG_FILE) + original_path = Rails.root.join(__dir__, "../", described_class::BANNER_CONFIG_FILE) allow(YAML).to receive(:load_file).with(original_path).and_return(replacement_file) end describe ".for_path" do context "with banners" do let(:replacement_file) do - YAML.load_file(Rails.root.join(__dir__, "../../spec/fixtures/active_recruitment_banners.yml")) + YAML.load_file(Rails.root.join(fixtures_dir, "active_recruitment_banners.yml")) end it "returns banner that includes the path" do @@ -21,12 +23,12 @@ context "with timed banners" do let(:replacement_file) do - YAML.load_file(Rails.root.join(__dir__, "../../spec/fixtures/timed_recruitment_banners.yml")) + YAML.load_file(Rails.root.join(fixtures_dir, "timed_recruitment_banners.yml")) end after { travel_back } - context "Before timed banners are active" do + context "but before timed banners are active" do before { travel_to Time.local(2024, 12, 31) } it "finds only banners with no start time" do @@ -37,7 +39,7 @@ end end - context "The day banners 1 and 3 become active and banner 2 ends" do + context "and on the day banners 1 and 3 become active and banner 2 ends" do before { travel_to Time.local(2025, 1, 1) } it "finds only banners active on that date" do @@ -52,7 +54,7 @@ end end - context "The day the page-3 banner swaps" do + context "and on the day the page-3 banner swaps" do before { travel_to Time.local(2025, 2, 1) } it "finds only banners active on that date" do @@ -67,7 +69,7 @@ end end - context "After all timed banners end" do + context "but after all timed banners end" do before { travel_to Time.local(2025, 3, 1) } it "finds only banners active on that date" do diff --git a/spec/units/govuk_web_banners/validators/recruitment_banner_spec.rb b/spec/units/govuk_web_banners/validators/recruitment_banner_spec.rb new file mode 100644 index 0000000..ac9278b --- /dev/null +++ b/spec/units/govuk_web_banners/validators/recruitment_banner_spec.rb @@ -0,0 +1,148 @@ +require "govuk_web_banners/validators/recruitment_banner" + +RSpec.describe GovukWebBanners::Validators::RecruitmentBanner do + subject(:recruitment_banner) { described_class.new(GovukWebBanners::RecruitmentBanner.all_banners) } + + let(:fixtures_dir) { Rails.root.join(__dir__, "../../../../spec/fixtures/") } + + before do + original_path = Rails.root.join(__dir__, "../../", GovukWebBanners::RecruitmentBanner::BANNER_CONFIG_FILE) + allow(YAML).to receive(:load_file).with(original_path).and_return(replacement_file) + + stub_request(:get, %r{\Ahttps://www\.gov\.uk/api/content/.*}) + end + + context "with valid banners" do + let(:replacement_file) do + YAML.load_file(Rails.root.join(fixtures_dir, "active_recruitment_banners.yml")) + end + + describe ".valid?" do + it "returns true" do + expect(recruitment_banner.valid?).to be true + end + end + + describe "errors attribute" do + it "returns an empty array" do + expect(recruitment_banner.errors).to be_empty + end + end + end + + context "with invalid banners" do + let(:replacement_file) do + YAML.load_file(Rails.root.join(fixtures_dir, "invalid_recruitment_banners.yml")) + end + + describe ".valid?" do + it "returns false" do + expect(recruitment_banner.valid?).to be false + end + end + + describe "errors attribute" do + it "returns the relevant errors" do + expect(recruitment_banner.errors["Empty Survey URL"]).to eq(["is missing a survey_url"]) + expect(recruitment_banner.errors["Suggestion Link Text Broken"]).to eq(["is missing a suggestion_link_text"]) + expect(recruitment_banner.errors["Page Paths Empty"]).to eq(["is missing any page_paths"]) + expect(recruitment_banner.errors["Suggestion Text Empty"]).to eq(["is missing a suggestion_text"]) + expect(recruitment_banner.errors["Page Paths include invalid path"]).to eq(["page_path views/old should start with a /"]) + end + end + end + + context "with a banner that has expired" do + let(:replacement_file) do + YAML.load_file(Rails.root.join(fixtures_dir, "expired_recruitment_banners.yml")) + end + + before { travel_to Time.local(2024, 1, 2) } + after { travel_back } + + describe ".valid?" do + it "returns true" do + expect(recruitment_banner.valid?).to be true + end + end + + describe ".warnings?" do + it "returns true" do + expect(recruitment_banner.warnings?).to be true + end + end + + describe "warnings attribute" do + it "returns the relevant warnings" do + expect(recruitment_banner.warnings).to eq({ "Banner 1" => ["is expired"] }) + end + end + end + + context "with banners on the same path" do + context "when they aren't active at the same time" do + let(:replacement_file) do + YAML.load_file(Rails.root.join(fixtures_dir, "path_clash_different_times_recruitment_banners.yml")) + end + + describe ".valid?" do + it "returns true" do + expect(recruitment_banner.valid?).to be true + end + end + + describe "errors attribute" do + it "returns an empty array" do + expect(recruitment_banner.errors).to be_empty + end + end + end + + context "when they are active at the same time" do + let(:replacement_file) do + YAML.load_file(Rails.root.join(fixtures_dir, "path_clash_same_time_recruitment_banners.yml")) + end + + describe ".valid?" do + it "returns false" do + expect(recruitment_banner.valid?).to be false + end + end + + describe "errors attribute" do + it "returns the relevant errors" do + expect(recruitment_banner.errors["Banner feb-apr"]).to eq(["is active at the same time as Banner jan-mar and points to the same paths"]) + expect(recruitment_banner.errors["Banner jan-mar"]).to eq(["is active at the same time as Banner feb-apr and points to the same paths"]) + end + end + end + end + + context "with a path that doesn't exist on the live site" do + let(:replacement_file) do + YAML.load_file(Rails.root.join(fixtures_dir, "active_recruitment_banners.yml")) + end + + before do + stub_request(:get, %r{\Ahttps://www\.gov\.uk/api/content/email-signup}).to_return(status: 404) + end + + describe ".valid?" do + it "returns true" do + expect(recruitment_banner.valid?).to be true + end + end + + describe ".warnings?" do + it "returns true" do + expect(recruitment_banner.warnings?).to be true + end + end + + describe "warnings attribute" do + it "returns the relevant warnings" do + expect(recruitment_banner.warnings).to eq({ "Banner 2" => ["refers to a path /email-signup which is not currently live on gov.uk"] }) + end + end + end +end diff --git a/spec/units/validators/recruitment_banner_spec.rb b/spec/units/validators/recruitment_banner_spec.rb deleted file mode 100644 index 9086ed5..0000000 --- a/spec/units/validators/recruitment_banner_spec.rb +++ /dev/null @@ -1,147 +0,0 @@ -require "govuk_web_banners/validators/recruitment_banner" - -RSpec.describe GovukWebBanners::Validators::RecruitmentBanner do - let(:banner_class) { GovukWebBanners::RecruitmentBanner } - let(:subject) { described_class.new(GovukWebBanners::RecruitmentBanner.all_banners) } - - before do - original_path = Rails.root.join(__dir__, "../", banner_class::BANNER_CONFIG_FILE) - allow(YAML).to receive(:load_file).with(original_path).and_return(replacement_file) - - stub_request(:get, %r{\Ahttps://www\.gov\.uk/api/content/.*}) - end - - context "with valid banners" do - let(:replacement_file) do - YAML.load_file(Rails.root.join(__dir__, "../../../spec/fixtures/active_recruitment_banners.yml")) - end - - describe ".valid?" do - it "returns true" do - expect(subject.valid?).to be true - end - end - - describe "errors attribute" do - it "returns an empty array" do - expect(subject.errors).to be_empty - end - end - end - - context "with invalid banners" do - let(:replacement_file) do - YAML.load_file(Rails.root.join(__dir__, "../../../spec/fixtures/invalid_recruitment_banners.yml")) - end - - describe ".valid?" do - it "returns false" do - expect(subject.valid?).to be false - end - end - - describe "errors attribute" do - it "returns the relevant errors" do - expect(subject.errors["Empty Survey URL"]).to eq(["is missing a survey_url"]) - expect(subject.errors["Suggestion Link Text Broken"]).to eq(["is missing a suggestion_link_text"]) - expect(subject.errors["Page Paths Empty"]).to eq(["is missing any page_paths"]) - expect(subject.errors["Suggestion Text Empty"]).to eq(["is missing a suggestion_text"]) - expect(subject.errors["Page Paths include invalid path"]).to eq(["page_path views/old should start with a /"]) - end - end - end - - context "with a banner that has expired" do - let(:replacement_file) do - YAML.load_file(Rails.root.join(__dir__, "../../../spec/fixtures/expired_recruitment_banners.yml")) - end - - before { travel_to Time.local(2024, 1, 2) } - after { travel_back } - - describe ".valid?" do - it "returns true" do - expect(subject.valid?).to be true - end - end - - describe ".warnings?" do - it "returns true" do - expect(subject.warnings?).to be true - end - end - - describe "warnings attribute" do - it "returns the relevant warnings" do - expect(subject.warnings).to eq({ "Banner 1" => ["is expired"] }) - end - end - end - - context "with banners on the same path" do - context "that aren't active at the same time" do - let(:replacement_file) do - YAML.load_file(Rails.root.join(__dir__, "../../../spec/fixtures/path_clash_different_times_recruitment_banners.yml")) - end - - describe ".valid?" do - it "returns true" do - expect(subject.valid?).to be true - end - end - - describe "errors attribute" do - it "returns an empty array" do - expect(subject.errors).to be_empty - end - end - end - - context "that are active at the same time" do - let(:replacement_file) do - YAML.load_file(Rails.root.join(__dir__, "../../../spec/fixtures/path_clash_same_time_recruitment_banners.yml")) - end - - describe ".valid?" do - it "returns false" do - expect(subject.valid?).to be false - end - end - - describe "errors attribute" do - it "returns the relevant errors" do - expect(subject.errors["Banner feb-apr"]).to eq(["is active at the same time as Banner jan-mar and points to the same paths"]) - expect(subject.errors["Banner jan-mar"]).to eq(["is active at the same time as Banner feb-apr and points to the same paths"]) - end - end - end - end - - context "with a path that doesn't exist on the live site" do - let(:replacement_file) do - YAML.load_file(Rails.root.join(__dir__, "../../../spec/fixtures/active_recruitment_banners.yml")) - end - - before do - stub_request(:get, %r{\Ahttps://www\.gov\.uk/api/content/email-signup}).to_return(status: 404) - end - - describe ".valid?" do - it "returns true" do - expect(subject.valid?).to be true - end - end - - describe ".warnings?" do - it "returns true" do - expect(subject.warnings?).to be true - end - end - - describe "warnings attribute" do - it "returns the relevant warnings" do - expect(subject.warnings).to eq({ "Banner 2" => ["refers to a path /email-signup which is not currently live on gov.uk"] }) - end - end - end -end