Rubocop rspec fixes (#264)

.rubocop.yml

@@ -1,11 +1,13 @@
 require:
   - rubocop-performance
   - rubocop-rails
+#  - rubocop-rspec
 
 inherit_gem:
   rubocop-govuk:
     - config/default.yml
     - config/rails.yml
+#    - config/rspec.yml
 
 AllCops:
   Exclude:

spec/components/tab_navigation_component_spec.rb

@@ -7,16 +7,16 @@ RSpec.describe TabNavigationComponent, type: :component do
      { name: "Timeline", url: "#" }]
   end
 
-  context "nav tabs appearing as selected" do
+  context "when the item is 'current' in nav tabs" do
-    it "when the item is 'current' then that tab is selected" do
+    it "then that tab appears as selected" do
       result = render_inline(described_class.new(items: items))
 
       expect(result.css('.app-tab-navigation__link[aria-current="page"]').text).to include("Application")
     end
   end
 
-  context "rendering tabs" do
+  context "when rendering tabs" do
-    it "renders all of the nav tabs specified in the items hash passed to it" do
+    it "all of the nav tabs specified in the items hash are passed to it" do
       result = render_inline(described_class.new(items: items))
 
       expect(result.text).to include("Application")

spec/controllers/admin/admin_users_controller_spec.rb

@@ -6,6 +6,7 @@ describe Admin::AdminUsersController, type: :controller do
   let(:page) { Capybara::Node::Simple.new(response.body) }
   let(:resource_title) { "Admin Users" }
   let(:valid_session) { {} }
+
   login_admin_user
 
   describe "Get admin users" do
@@ -29,7 +30,7 @@ describe Admin::AdminUsersController, type: :controller do
   end
 
   describe "Update admin users" do
-    context "edit form" do
+    context "when editing the form" do
       before do
         get :edit, session: valid_session, params: { id: AdminUser.first.id }
       end
@@ -41,7 +42,7 @@ describe Admin::AdminUsersController, type: :controller do
       end
     end
 
-    context "update" do
+    context "when updating the form" do
       let(:admin_user) { FactoryBot.create(:admin_user) }
       let(:email) { "new_email@example.com" }
       let(:params) { { id: admin_user.id, admin_user: { email: email } } }

spec/controllers/admin/case_logs_controller_spec.rb

@@ -6,14 +6,17 @@ describe Admin::CaseLogsController, type: :controller do
   before do
     RequestHelper.stub_http_requests
   end
+
   render_views
   let(:page) { Capybara::Node::Simple.new(response.body) }
   let(:resource_title) { "Logs" }
   let(:valid_session) { {} }
+
   login_admin_user
 
   describe "Get case logs" do
     let!(:case_log) { FactoryBot.create(:case_log, :in_progress) }
+
     before do
       get :index, session: valid_session
     end
@@ -37,6 +40,7 @@ describe Admin::CaseLogsController, type: :controller do
         },
       }
     end
+
     it "creates a new case log" do
       expect { post :create, session: valid_session, params: params }.to change(CaseLog, :count).by(1)
     end

spec/controllers/admin/dashboard_controller_spec.rb

@@ -6,17 +6,18 @@ describe Admin::DashboardController, type: :controller do
   before do
     RequestHelper.stub_http_requests
   end
+
   render_views
   let(:page) { Capybara::Node::Simple.new(response.body) }
   let(:resource_title) { "Dashboard" }
-  let!(:case_log) { FactoryBot.create(:case_log, :in_progress) }
-  let!(:case_log_2) { FactoryBot.create(:case_log, :in_progress) }
-  let!(:completed_case_log) { FactoryBot.create(:case_log, :completed) }
   let(:valid_session) { {} }
+
   login_admin_user
 
   describe "Get case logs" do
     before do
+      2.times { |_| FactoryBot.create(:case_log, :in_progress) }
+      FactoryBot.create(:case_log, :completed)
       get :index, session: valid_session
     end
 

spec/controllers/admin/organisations_controller_spec.rb

@@ -7,6 +7,7 @@ describe Admin::OrganisationsController, type: :controller do
   let(:resource_title) { "Organisations" }
   let(:valid_session) { {} }
   let!(:organisation) { FactoryBot.create(:organisation) }
+
   login_admin_user
 
   describe "Organisations" do

spec/controllers/admin/users_controller_spec.rb

@@ -8,6 +8,7 @@ describe Admin::UsersController, type: :controller do
   let(:page) { Capybara::Node::Simple.new(response.body) }
   let(:resource_title) { "Users" }
   let(:valid_session) { {} }
+
   login_admin_user
 
   describe "Get users" do
@@ -41,7 +42,7 @@ describe Admin::UsersController, type: :controller do
   end
 
   describe "Update users" do
-    context "update form" do
+    context "when updating the form" do
       before do
         get :edit, session: valid_session, params: { id: user.id }
       end
@@ -56,7 +57,7 @@ describe Admin::UsersController, type: :controller do
       end
     end
 
-    context "update" do
+    context "when updating the user" do
       let(:name) { "Pete" }
       let(:params) { { id: user.id, user: { name: name } } }
 

spec/features/form/check_answers_page_spec.rb

@@ -5,6 +5,8 @@ require_relative "../../request_helper"
 RSpec.describe "Form Check Answers Page" do
   include Helpers
   let(:user) { FactoryBot.create(:user) }
+  let(:subsection) { "household-characteristics" }
+  let(:conditional_subsection) { "conditional-question" }
   let(:case_log) do
     FactoryBot.create(
       :case_log,
@@ -27,16 +29,14 @@ RSpec.describe "Form Check Answers Page" do
     sign_in user
   end
 
-  let(:subsection) { "household-characteristics" }
-  let(:conditional_subsection) { "conditional-question" }
-
   context "when the user needs to check their answers for a subsection" do
+    let(:last_question_for_subsection) { "household-number-of-other-members" }
+
     it "can be visited by URL" do
       visit("/logs/#{id}/#{subsection}/check-answers")
       expect(page).to have_content("#{subsection.tr('-', ' ').humanize} Check your answers")
     end
 
-    let(:last_question_for_subsection) { "household-number-of-other-members" }
     it "redirects to the check answers page when answering the last question and clicking save and continue" do
       fill_in_number_question(id, "other_hhmemb", 0, last_question_for_subsection)
       expect(page).to have_current_path("/logs/#{id}/#{subsection}/check-answers")
@@ -50,7 +50,7 @@ RSpec.describe "Form Check Answers Page" do
       end
     end
 
-    it "should display answers given by the user for the question in the subsection" do
+    it "displays answers given by the user for the question in the subsection" do
       fill_in_number_question(empty_case_log.id, "age1", 28, "person-1-age")
       choose("case-log-sex1-non-binary-field")
       click_button("Save and continue")
@@ -61,14 +61,14 @@ RSpec.describe "Form Check Answers Page" do
 
     # Regex explanation: match the string "Answer" but not if it's follow by "the missing questions"
     # This way only the links in the table will get picked up
-    it "should have an answer link for questions missing an answer" do
+    it "has an answer link for questions missing an answer" do
       visit("/logs/#{empty_case_log.id}/#{subsection}/check-answers")
       assert_selector "a", text: /Answer (?!the missing questions)/, count: 4
       assert_selector "a", text: "Change", count: 0
       expect(page).to have_link("Answer", href: "/logs/#{empty_case_log.id}/person-1-age")
     end
 
-    it "should have a change link for answered questions" do
+    it "has a change link for answered questions" do
       fill_in_number_question(empty_case_log.id, "age1", 28, "person-1-age")
       visit("/logs/#{empty_case_log.id}/#{subsection}/check-answers")
       assert_selector "a", text: /Answer (?!the missing questions)/, count: 3
@@ -76,7 +76,7 @@ RSpec.describe "Form Check Answers Page" do
       expect(page).to have_link("Change", href: "/logs/#{empty_case_log.id}/person-1-age")
     end
 
-    it "should have a change link for answered questions" do
+    it "updates the change/answer link when answers get updated" do
       visit("/logs/#{empty_case_log.id}/household-needs/check-answers")
       assert_selector "a", text: /Answer (?!the missing questions)/, count: 5
       assert_selector "a", text: "Change", count: 0

spec/features/form/conditional_questions_spec.rb

@@ -20,7 +20,7 @@ RSpec.describe "Form Conditional Questions" do
     sign_in user
   end
 
-  context "given a page where some questions are only conditionally shown, depending on how you answer the first question" do
+  context "with a page where some questions are only conditionally shown, depending on how you answer the first question" do
     it "initially hides conditional questions" do
       visit("/logs/#{id}/armed-forces")
       expect(page).not_to have_selector("#armed_forces_injured_div")

spec/features/form/page_routing_spec.rb

@@ -48,7 +48,7 @@ RSpec.describe "Form Page Routing" do
     expect(page).to have_current_path("/logs/#{id}/conditional-question/check-answers")
   end
 
-  context "inferred answers routing", js: true do
+  context "when the answers are inferred", js: true do
     it "shows question if the answer could not be inferred" do
       visit("/logs/#{id}/property-postcode")
       fill_in("case-log-property-postcode-field", with: "PO5 3TE")
@@ -56,7 +56,7 @@ RSpec.describe "Form Page Routing" do
       expect(page).to have_current_path("/logs/#{id}/do-you-know-the-local-authority")
     end
 
-    it "shows question if the answer could not be inferred" do
+    it "shows question if the answer could not be inferred from an empty input" do
       visit("/logs/#{id}/property-postcode")
       click_button("Save and continue")
       expect(page).to have_current_path("/logs/#{id}/do-you-know-the-local-authority")

spec/features/form/progressive_total_field_spec.rb

@@ -21,12 +21,12 @@ RSpec.describe "Accessible Automcomplete" do
 
   it "does not show when js is not enabled" do
     visit("/logs/#{case_log.id}/rent")
-    expect(page).to have_selector("#tcharge_div", visible: false)
+    expect(page).to have_selector("#tcharge_div", visible: :all)
   end
 
   it "does show when js is enabled and calculates the total", js: true do
     visit("/logs/#{case_log.id}/rent")
-    expect(page).to have_selector("#tcharge_div", visible: true)
+    expect(page).to have_selector("#tcharge_div")
     fill_in("case-log-brent-field", with: 5)
     expect(find("#case-log-tcharge-field").value).to eq("5.00")
     fill_in("case-log-pscharge-field", with: 3)

spec/features/form/saving_data_spec.rb

@@ -85,14 +85,13 @@ RSpec.describe "Form Saving Data" do
 
   it "displays checkbox answers in inputs if they are already saved" do
     visit("/logs/#{case_log_with_checkbox_questions_answered.id.to_s.dasherize}/accessibility-requirements")
-    # Something about our styling makes the selenium webdriver think the actual radio buttons are not visible so we pass false here
     expect(page).to have_checked_field(
       "case-log-accessibility-requirements-housingneeds-a-field",
-      visible: false,
+      visible: :all,
     )
     expect(page).to have_unchecked_field(
       "case-log-accessibility-requirements-housingneeds-b-field",
-      visible: false,
+      visible: :all,
     )
   end
 end

spec/features/form/validations_spec.rb

@@ -28,8 +28,8 @@ RSpec.describe "validations" do
   let(:id) { case_log.id }
 
   describe "Question validation" do
-    context "given an invalid tenant age" do
+    context "when the tenant age is invalid" do
-      it " of less than 0 it shows validation" do
+      it "shows validation for under 0" do
         visit("/logs/#{id}/person-1-age")
         fill_in_number_question(empty_case_log.id, "age1", -5, "person-1-age")
         expect(page).to have_selector("#error-summary-title")
@@ -38,7 +38,7 @@ RSpec.describe "validations" do
         expect(page).to have_title("Error")
       end
 
-      it " of greater than 120 it shows validation" do
+      it "shows validation for over 120" do
         visit("/logs/#{id}/person-1-age")
         fill_in_number_question(empty_case_log.id, "age1", 121, "person-1-age")
         expect(page).to have_selector("#error-summary-title")
@@ -103,7 +103,7 @@ RSpec.describe "validations" do
   end
 
   describe "Soft Validation" do
-    context "given a weekly net income that is above the expected amount for the given economic status but below the hard max" do
+    context "when a weekly net income is above the expected amount for the given economic status but below the hard max" do
       let(:case_log) do
         FactoryBot.create(
           :case_log,

spec/features/organisation_spec.rb

@@ -6,7 +6,7 @@ RSpec.describe "User Features" do
   let(:organisation) { user.organisation }
   let(:org_id) { organisation.id }
   let(:set_password_template_id) { DeviseNotifyMailer::SET_PASSWORD_TEMPLATE_ID }
-  let(:notify_client) { double(Notifications::Client) }
+  let(:notify_client) { instance_double(Notifications::Client) }
   let(:reset_password_token) { "MCDH5y6Km-U7CFPgAMVS" }
 
   before do
@@ -17,10 +17,10 @@ RSpec.describe "User Features" do
     sign_in user
   end
 
-  context "User is a data coordinator" do
+  context "when user is a data coordinator" do
     let!(:user) { FactoryBot.create(:user, :data_coordinator) }
 
-    context "Organisation page" do
+    context "when viewing organisation page" do
       it "defaults to organisation details" do
         visit("/logs")
         click_link("Your organisation")
@@ -36,7 +36,7 @@ RSpec.describe "User Features" do
       end
     end
 
-    context "Organisation users" do
+    context "when users are part of organisation" do
       it "users can be added" do
         visit("/organisations/#{org_id}")
         click_link("Users")
@@ -65,10 +65,10 @@ RSpec.describe "User Features" do
     end
   end
 
-  context "User is a data provider" do
+  context "when the user is a data provider" do
-    let!(:user) { FactoryBot.create(:user) }
+    let(:user) { FactoryBot.create(:user) }
 
-    context "Organisation page" do
+    context "when viewing organisation page" do
       it "can only see the details tab" do
         visit("/logs")
         click_link("Your organisation")

spec/features/start_page_spec.rb

@@ -5,7 +5,7 @@ RSpec.describe "Start Page Features" do
   include Helpers
   let(:user) { FactoryBot.create(:user) }
 
-  context "a signed in user" do
+  context "when the user is signed in" do
     before do
       sign_in user
     end
@@ -16,7 +16,7 @@ RSpec.describe "Start Page Features" do
     end
   end
 
-  context "a not signed in user" do
+  context "when the user is not signed in" do
     it "takes you to sign in and then to your logs" do
       visit("/")
       click_link("Start now")

spec/features/test_spec.rb

@@ -7,6 +7,6 @@ RSpec.describe "Test Features" do
 
   it "Responds to a health check" do
     visit("/health")
-    expect(page).to have_http_status(204)
+    expect(page).to have_http_status(:no_content)
   end
 end

spec/features/user_spec.rb

@@ -4,8 +4,9 @@ require_relative "../request_helper"
 RSpec.describe "User Features" do
   let!(:user) { FactoryBot.create(:user, last_sign_in_at: Time.zone.now) }
   let(:reset_password_template_id) { DeviseNotifyMailer::RESET_PASSWORD_TEMPLATE_ID }
-  let(:notify_client) { double(Notifications::Client) }
+  let(:notify_client) { instance_double(Notifications::Client) }
   let(:reset_password_token) { "MCDH5y6Km-U7CFPgAMVS" }
+
   before do
     allow_any_instance_of(DeviseNotifyMailer).to receive(:notify_client).and_return(notify_client)
     allow_any_instance_of(DeviseNotifyMailer).to receive(:host).and_return("test.com")
@@ -13,7 +14,7 @@ RSpec.describe "User Features" do
     allow_any_instance_of(User).to receive(:set_reset_password_token).and_return(reset_password_token)
   end
 
-  context "A user navigating to case logs" do
+  context "when the user navigates to case logs" do
     it " is required to log in" do
       visit("/logs")
       expect(page).to have_current_path("/users/sign-in")
@@ -53,7 +54,7 @@ RSpec.describe "User Features" do
     end
   end
 
-  context "A user who has forgotten their password" do
+  context "when the user has forgotten their password" do
     it " is redirected to the reset password page when they click the reset password link" do
       visit("/logs")
       click_link("reset your password")
@@ -117,7 +118,7 @@ RSpec.describe "User Features" do
     end
   end
 
-  context "If user not logged in" do
+  context "when the user is not logged in" do
     it "'Your account' link does not display" do
       visit("/logs")
       expect(page).to have_no_link("Your account")
@@ -139,7 +140,7 @@ RSpec.describe "User Features" do
     end
   end
 
-  context "Trying to log in with incorrect credentials" do
+  context "when the user is trying to log in with incorrect credentials" do
     it "shows a gov uk error summary and no flash message" do
       visit("/logs")
       fill_in("user[email]", with: user.email)
@@ -170,8 +171,8 @@ RSpec.describe "User Features" do
     end
   end
 
-  context "Your Account " do
+  context "when viewing your account" do
-    before(:each) do
+    before do
       visit("/logs")
       fill_in("user[email]", with: user.email)
       fill_in("user[password]", with: "pAssword1")
@@ -206,8 +207,8 @@ RSpec.describe "User Features" do
     end
   end
 
-  context "Adding a new user" do
+  context "when adding a new user" do
-    before(:each) do
+    before do
       visit("/logs")
       fill_in("user[email]", with: user.email)
       fill_in("user[password]", with: "pAssword1")

spec/helpers/check_answers_helper_spec.rb

@@ -1,20 +1,19 @@
 require "rails_helper"
 
 RSpec.describe CheckAnswersHelper do
-  let(:case_log) { FactoryBot.build(:case_log) }
   let(:form) { case_log.form }
   let(:subsection) { form.get_subsection("household_characteristics") }
   let(:case_log) { FactoryBot.build(:case_log, :in_progress) }
 
   describe "display_answered_questions_summary" do
-    context "given a section that hasn't been completed yet" do
+    context "when a section hasn't been completed yet" do
       it "returns that you have unanswered questions" do
         expect(display_answered_questions_summary(subsection, case_log))
           .to match(/You have answered 2 of 4 questions./)
       end
     end
 
-    context "given a section that has been completed" do
+    context "when a section has been completed" do
       it "returns that you have answered all the questions" do
         case_log.sex1 = "F"
         case_log.other_hhmemb = 0

spec/helpers/conditional_questions_helper_spec.rb

@@ -15,8 +15,9 @@ RSpec.describe ConditionalQuestionsHelper do
   describe "find conditional question" do
     let(:question) { page.questions.find { |q| q.id == "armedforces" } }
     let(:answer_value) { "A current or former regular in the UK Armed Forces (excluding National Service)" }
+
     it "returns the conditional question for a given answer option" do
-      expect(find_conditional_question(page, question, answer_value))
+      expect(find_conditional_question(page, question, answer_value).id).to eq("leftreg")
     end
   end
 

spec/helpers/form_page_error_helper_spec.rb

@@ -1,8 +1,13 @@
 require "rails_helper"
+require_relative "../request_helper"
 
 RSpec.describe FormPageErrorHelper do
+  before do
+    RequestHelper.stub_http_requests
+  end
+
   describe "#remove_other_page_errors" do
-    context "removes non base other questions" do
+    context "when non base other questions are removed" do
       let!(:case_log) { FactoryBot.create(:case_log, :in_progress) }
       let!(:form) { case_log.form }
 

spec/helpers/question_attribute_helper_spec.rb

@@ -21,7 +21,7 @@ RSpec.describe QuestionAttributeHelper do
       })
     end
 
-    context "a question that requires multiple controllers" do
+    context "when a question that requires multiple controllers" do
       let(:question) do
         Form::Question.new("brent", {
           "check_answer_label" => "Basic Rent",
@@ -46,6 +46,7 @@ RSpec.describe QuestionAttributeHelper do
           "data-info": question.conditional_for.to_json,
         }
       end
+
       it "correctly merges html attributes" do
         expect(stimulus_html_attributes(question)).to eq(expected_attribs)
       end

spec/helpers/question_view_helper_spec.rb

@@ -5,70 +5,73 @@ RSpec.describe QuestionViewHelper do
   let(:conditional) { false }
 
   describe "caption" do
-    let(:subject) { caption(caption_text, page_header, conditional) }
+    subject(:header) { caption(caption_text, page_header, conditional) }
+
     let(:caption_text) { "Some text" }
     let(:caption_options_hash) { { text: caption_text.html_safe, size: "l" } }
 
-    context "a page without a header" do
+    context "when viewing a page without a header" do
       let(:page_header) { nil }
 
       it "returns an options hash" do
-        expect(subject).to eq(caption_options_hash)
+        expect(header).to eq(caption_options_hash)
       end
     end
 
-    context "a page with a header" do
+    context "when viewing a page with a header" do
       it "returns nil" do
-        expect(subject).to be_nil
+        expect(header).to be_nil
       end
     end
 
-    context "a conditional question" do
+    context "when viewing a conditional question" do
       let(:conditional) { true }
+
       it "returns nil" do
-        expect(subject).to be_nil
+        expect(header).to be_nil
       end
     end
 
-    context "a question without a caption" do
+    context "when viewig a question without a caption" do
       let(:caption_text) { nil }
 
       it "returns nil" do
-        expect(subject).to be_nil
+        expect(header).to be_nil
       end
     end
   end
 
   describe "legend" do
+    subject(:question_view_helper) { legend(question, page_header, conditional) }
+
     let(:question) { OpenStruct.new(header: "Some question header") }
-    let(:subject) { legend(question, page_header, conditional) }
     let(:size) { "m" }
     let(:tag) { "h2" }
     let(:legend_options_hash) do
       { text: "Some question header".html_safe, size: size, tag: tag }
     end
 
-    context "a page with a header" do
+    context "when viewing a page with a header" do
       it "returns an options hash with a medium question header" do
-        expect(subject).to eq(legend_options_hash)
+        expect(question_view_helper).to eq(legend_options_hash)
       end
     end
 
-    context "a page without a header" do
+    context "when viewing a page without a header" do
       let(:page_header) { nil }
       let(:size) { "l" }
       let(:tag) { "h1" }
 
       it "returns an options hash with a large question header" do
-        expect(subject).to eq(legend_options_hash)
+        expect(question_view_helper).to eq(legend_options_hash)
       end
     end
 
-    context "a conditional question" do
+    context "when viewinng a conditional question" do
       let(:conditional) { true }
 
       it "returns an options hash with a medium question header" do
-        expect(subject).to eq(legend_options_hash)
+        expect(question_view_helper).to eq(legend_options_hash)
       end
     end
   end

spec/helpers/tab_nav_helper_spec.rb

@@ -19,8 +19,9 @@ RSpec.describe TabNavHelper do
   end
 
   describe "#tab_items" do
-    context "user is a data_coordinator" do
+    context "when user is a data_coordinator" do
       let(:user) { FactoryBot.build(:user, :data_coordinator, organisation: organisation) }
+
       it "returns details and user tabs" do
         result = tab_items(user).map { |i| i[:name] }
         expect(result.count).to eq(2)
@@ -29,7 +30,7 @@ RSpec.describe TabNavHelper do
       end
     end
 
-    context "user is a data_provider" do
+    context "when user is a data_provider" do
       it "returns details tab only" do
         result = tab_items(user).map { |i| i[:name] }
         expect(result.count).to eq(1)

spec/helpers/tasklist_helper_spec.rb

@@ -5,6 +5,7 @@ RSpec.describe TasklistHelper do
   before do
     RequestHelper.stub_http_requests
   end
+
   let(:empty_case_log) { FactoryBot.create(:case_log) }
   let(:case_log) { FactoryBot.create(:case_log, :in_progress) }
 
@@ -56,13 +57,13 @@ RSpec.describe TasklistHelper do
   describe "subsection link" do
     let(:subsection) { case_log.form.get_subsection("household_characteristics") }
 
-    context "for a subsection that's enabled" do
+    context "with a subsection that's enabled" do
       it "returns the subsection link url" do
         expect(subsection_link(subsection, case_log)).to match(/household-characteristics/)
       end
     end
 
-    context "for a subsection that cannot be started yet" do
+    context "with a subsection that cannot be started yet" do
       before do
         allow(subsection).to receive(:status).with(case_log).and_return(:cannot_start_yet)
       end

spec/lib/tasks/data_import/organisations_spec.rb

@@ -3,6 +3,7 @@ require "rake"
 
 describe "rake data_import:organisations", type: :task do
   subject(:task) { Rake::Task["data_import:organisations"] }
+
   let(:fixture_path) { "spec/fixtures/softwire_imports/organisations" }
 
   before do

spec/lib/tasks/form_definition_validator_spec.rb

@@ -1,33 +1,35 @@
 require "rails_helper"
 require "rake"
 
-describe "rake form_definition:validate_all", type: :task do
+RSpec.describe "form_definition" do
-  subject(:task) { Rake::Task["form_definition:validate_all"] }
+  describe ":validate_all", type: :task do
+    subject(:task) { Rake::Task["form_definition:validate_all"] }
 
-  before do
+    before do
-    Rake.application.rake_require("tasks/form_definition")
+      Rake.application.rake_require("tasks/form_definition")
-    Rake::Task.define_task(:environment)
+      Rake::Task.define_task(:environment)
-    task.reenable
+      task.reenable
-  end
+    end
 
-  it "runs the validate task for each form definition in the project" do
+    it "runs the validate task for each form definition in the project" do
-    expect(Rake::Task["form_definition:validate"]).to receive(:invoke).exactly(4).times
+      expect(Rake::Task["form_definition:validate"]).to receive(:invoke).exactly(4).times
-    task.invoke
+      task.invoke
+    end
   end
-end
 
-describe "rake form_definition:validate", type: :task do
+  describe ":validate", type: :task do
-  subject(:task) { Rake::Task["form_definition:validate"] }
+    subject(:task) { Rake::Task["form_definition:validate"] }
 
-  before do
+    before do
-    Rake.application.rake_require("tasks/form_definition")
+      Rake.application.rake_require("tasks/form_definition")
-    Rake::Task.define_task(:environment)
+      Rake::Task.define_task(:environment)
-    allow(JSON::Validator).to receive(:validate).and_return(true)
+      allow(JSON::Validator).to receive(:validate).and_return(true)
-    task.reenable
+      task.reenable
-  end
+    end
 
-  it "runs the validate task for the given form definition" do
+    it "runs the validate task for the given form definition" do
-    expect(JSON::Validator).to receive(:validate!).at_least(1).time
+      expect(JSON::Validator).to receive(:validate!).at_least(1).time
-    task.invoke("config/forms/2021_2022.json")
+      task.invoke("config/forms/2021_2022.json")
+    end
   end
 end

spec/models/form/page_spec.rb

@@ -1,6 +1,8 @@
 require "rails_helper"
 
 RSpec.describe Form::Page, type: :model do
+  subject(:page) { described_class.new(page_id, page_definition, subsection) }
+
   let(:case_log) { FactoryBot.build(:case_log) }
   let(:form) { case_log.form }
   let(:section_id) { "rent_and_charges" }
@@ -11,66 +13,65 @@ RSpec.describe Form::Page, type: :model do
   let(:subsection) { Form::Subsection.new(subsection_id, subsection_definition, section) }
   let(:page_id) { "net_income" }
   let(:page_definition) { subsection_definition["pages"][page_id] }
-  subject { Form::Page.new(page_id, page_definition, subsection) }
 
   it "has an id" do
-    expect(subject.id).to eq(page_id)
+    expect(page.id).to eq(page_id)
   end
 
   it "has a header" do
-    expect(subject.header).to eq("Test header")
+    expect(page.header).to eq("Test header")
   end
 
   it "has a description" do
-    expect(subject.description).to eq("Some extra text for the page")
+    expect(page.description).to eq("Some extra text for the page")
   end
 
   it "has questions" do
     expected_questions = %w[earnings incfreq]
-    expect(subject.questions.map(&:id)).to eq(expected_questions)
+    expect(page.questions.map(&:id)).to eq(expected_questions)
   end
 
   it "has soft validations" do
     expected_soft_validations = %w[override_net_income_validation]
-    expect(subject.soft_validations.map(&:id)).to eq(expected_soft_validations)
+    expect(page.soft_validations.map(&:id)).to eq(expected_soft_validations)
   end
 
   it "has a soft_validation helper" do
-    expect(subject.has_soft_validations?).to be true
+    expect(page.has_soft_validations?).to be true
   end
 
   it "has expected form responses" do
     expected_responses = %w[earnings incfreq override_net_income_validation]
-    expect(subject.expected_responses.map(&:id)).to eq(expected_responses)
+    expect(page.expected_responses.map(&:id)).to eq(expected_responses)
   end
 
-  context "page with conditional questions" do
+  context "with a page having conditional questions" do
     let(:page_id) { "housing_benefit" }
 
     it "knows which questions are not conditional" do
       expected_non_conditional_questions = %w[hb]
-      expect(subject.non_conditional_questions.map(&:id))
+      expect(page.non_conditional_questions.map(&:id))
         .to eq(expected_non_conditional_questions)
     end
   end
 
-  context "for a given case log" do
+  context "with a case log" do
     let(:case_log) { FactoryBot.build(:case_log, :in_progress) }
 
     it "knows if it's been routed to" do
-      expect(subject.routed_to?(case_log)).to be true
+      expect(page.routed_to?(case_log)).to be true
     end
 
-    context "given routing conditions" do
+    context "with routing conditions" do
       let(:page_id) { "dependent_page" }
 
       it "evaluates not met conditions correctly" do
-        expect(subject.routed_to?(case_log)).to be false
+        expect(page.routed_to?(case_log)).to be false
       end
 
       it "evaluates not conditions correctly" do
         case_log.incfreq = "Weekly"
-        expect(subject.routed_to?(case_log)).to be true
+        expect(page.routed_to?(case_log)).to be true
       end
     end
 
@@ -81,8 +82,8 @@ RSpec.describe Form::Page, type: :model do
       let(:completed_case_log) { FactoryBot.build(:case_log, :completed, incfreq: "Weekly") }
 
       it "evaluates the sections dependencies" do
-        expect(subject.routed_to?(case_log)).to be false
+        expect(page.routed_to?(case_log)).to be false
-        expect(subject.routed_to?(completed_case_log)).to be true
+        expect(page.routed_to?(completed_case_log)).to be true
       end
     end
   end

spec/models/form/question_spec.rb

@@ -1,6 +1,8 @@
 require "rails_helper"
 
 RSpec.describe Form::Question, type: :model do
+  subject(:question) { described_class.new(question_id, question_definition, page) }
+
   let(:case_log) { FactoryBot.build(:case_log) }
   let(:form) { case_log.form }
   let(:section_id) { "rent_and_charges" }
@@ -14,43 +16,42 @@ RSpec.describe Form::Question, type: :model do
   let(:page) { Form::Page.new(page_id, page_definition, subsection) }
   let(:question_id) { "earnings" }
   let(:question_definition) { page_definition["questions"][question_id] }
-  subject { Form::Question.new(question_id, question_definition, page) }
 
   it "has an id" do
-    expect(subject.id).to eq(question_id)
+    expect(question.id).to eq(question_id)
   end
 
   it "has a header" do
-    expect(subject.header).to eq("What is the tenant’s /and partner’s combined income after tax?")
+    expect(question.header).to eq("What is the tenant’s /and partner’s combined income after tax?")
   end
 
   it "has a check answers label" do
-    expect(subject.check_answer_label).to eq("Income")
+    expect(question.check_answer_label).to eq("Income")
   end
 
   it "has a question type" do
-    expect(subject.type).to eq("numeric")
+    expect(question.type).to eq("numeric")
   end
 
   it "belongs to a page" do
-    expect(subject.page).to eq(page)
+    expect(question.page).to eq(page)
   end
 
   it "belongs to a subsection" do
-    expect(subject.subsection).to eq(subsection)
+    expect(question.subsection).to eq(subsection)
   end
 
   it "has a read only helper" do
-    expect(subject.read_only?).to be false
+    expect(question.read_only?).to be false
   end
 
   context "when type is numeric" do
     it "has a min value" do
-      expect(subject.min).to eq(0)
+      expect(question.min).to eq(0)
     end
 
     it "has a step value" do
-      expect(subject.step).to eq(1)
+      expect(question.step).to eq(1)
     end
   end
 
@@ -59,7 +60,7 @@ RSpec.describe Form::Question, type: :model do
 
     it "has answer options" do
       expected_answer_options = { "0" => "Weekly", "1" => "Monthly", "2" => "Yearly" }
-      expect(subject.answer_options).to eq(expected_answer_options)
+      expect(question.answer_options).to eq(expected_answer_options)
     end
   end
 
@@ -69,7 +70,7 @@ RSpec.describe Form::Question, type: :model do
 
     it "has answer options" do
       expected_answer_options = { "0" => "Option A", "1" => "Option B" }
-      expect(subject.answer_options).to eq(expected_answer_options)
+      expect(question.answer_options).to eq(expected_answer_options)
     end
   end
 
@@ -79,35 +80,35 @@ RSpec.describe Form::Question, type: :model do
     let(:question_id) { "tcharge" }
 
     it "has a read only helper" do
-      expect(subject.read_only?).to be true
+      expect(question.read_only?).to be true
     end
 
     context "when the answer is part of a sum" do
       let(:question_id) { "pscharge" }
 
       it "has a result_field" do
-        expect(subject.result_field).to eq("tcharge")
+        expect(question.result_field).to eq("tcharge")
       end
 
       it "has fields to sum" do
         expected_fields_to_sum = %w[brent scharge pscharge supcharg]
-        expect(subject.fields_to_add).to eq(expected_fields_to_sum)
+        expect(question.fields_to_add).to eq(expected_fields_to_sum)
       end
     end
   end
 
-  context "for a given case log" do
+  context "with a case log" do
     let(:case_log) { FactoryBot.build(:case_log, :in_progress) }
 
     it "has an answer label" do
       case_log.earnings = 100
-      expect(subject.answer_label(case_log)).to eq("100")
+      expect(question.answer_label(case_log)).to eq("100")
     end
 
     it "has an update answer link text helper" do
-      expect(subject.update_answer_link_name(case_log)).to eq("Answer<span class=\"govuk-visually-hidden\"> income</span>")
+      expect(question.update_answer_link_name(case_log)).to eq("Answer<span class=\"govuk-visually-hidden\"> income</span>")
       case_log[question_id] = 5
-      expect(subject.update_answer_link_name(case_log)).to eq("Change<span class=\"govuk-visually-hidden\"> income</span>")
+      expect(question.update_answer_link_name(case_log)).to eq("Change<span class=\"govuk-visually-hidden\"> income</span>")
     end
 
     context "when type is date" do
@@ -118,12 +119,12 @@ RSpec.describe Form::Question, type: :model do
 
       it "displays a formatted answer label" do
         case_log.mrcdate = Time.zone.local(2021, 10, 11)
-        expect(subject.answer_label(case_log)).to eq("11 October 2021")
+        expect(question.answer_label(case_log)).to eq("11 October 2021")
       end
 
       it "can handle nils" do
         case_log.mrcdate = nil
-        expect(subject.answer_label(case_log)).to eq("")
+        expect(question.answer_label(case_log)).to eq("")
       end
     end
 
@@ -137,7 +138,7 @@ RSpec.describe Form::Question, type: :model do
         case_log.housingneeds_a = 1
         case_log.housingneeds_c = 1
         expected_answer_label = "Fully wheelchair accessible housing, Level access housing"
-        expect(subject.answer_label(case_log)).to eq(expected_answer_label)
+        expect(question.answer_label(case_log)).to eq(expected_answer_label)
       end
     end
 
@@ -146,12 +147,12 @@ RSpec.describe Form::Question, type: :model do
       let(:question_id) { "conditional_question" }
 
       it "knows whether it is enabled or not for unmet conditions" do
-        expect(subject.enabled?(case_log)).to be false
+        expect(question.enabled?(case_log)).to be false
       end
 
       it "knows whether it is enabled or not for met conditions" do
         case_log.hb = "Housing benefit"
-        expect(subject.enabled?(case_log)).to be true
+        expect(question.enabled?(case_log)).to be true
       end
     end
   end

spec/models/form/section_spec.rb

@@ -1,22 +1,23 @@
 require "rails_helper"
 
 RSpec.describe Form::Section, type: :model do
+  subject(:section) { described_class.new(section_id, section_definition, form) }
+
   let(:case_log) { FactoryBot.build(:case_log) }
   let(:form) { case_log.form }
   let(:section_id) { "household" }
   let(:section_definition) { form.form_definition["sections"][section_id] }
-  subject { Form::Section.new(section_id, section_definition, form) }
 
   it "has an id" do
-    expect(subject.id).to eq(section_id)
+    expect(section.id).to eq(section_id)
   end
 
   it "has a label" do
-    expect(subject.label).to eq("About the household")
+    expect(section.label).to eq("About the household")
   end
 
   it "has subsections" do
     expected_subsections = %w[household_characteristics household_needs]
-    expect(subject.subsections.map(&:id)).to eq(expected_subsections)
+    expect(section.subsections.map(&:id)).to eq(expected_subsections)
   end
 end

spec/models/form/subsection_spec.rb

@@ -1,6 +1,8 @@
 require "rails_helper"
 
 RSpec.describe Form::Subsection, type: :model do
+  subject(:sub_section) { described_class.new(subsection_id, subsection_definition, section) }
+
   let(:case_log) { FactoryBot.build(:case_log) }
   let(:form) { case_log.form }
   let(:section_id) { "household" }
@@ -8,94 +10,93 @@ RSpec.describe Form::Subsection, type: :model do
   let(:section) { Form::Section.new(section_id, section_definition, form) }
   let(:subsection_id) { "household_characteristics" }
   let(:subsection_definition) { section_definition["subsections"][subsection_id] }
-  subject { Form::Subsection.new(subsection_id, subsection_definition, section) }
 
   it "has an id" do
-    expect(subject.id).to eq(subsection_id)
+    expect(sub_section.id).to eq(subsection_id)
   end
 
   it "has a label" do
-    expect(subject.label).to eq("Household characteristics")
+    expect(sub_section.label).to eq("Household characteristics")
   end
 
   it "has pages" do
     expected_pages = %w[tenant_code person_1_age person_1_gender household_number_of_other_members]
-    expect(subject.pages.map(&:id)).to eq(expected_pages)
+    expect(sub_section.pages.map(&:id)).to eq(expected_pages)
   end
 
   it "has questions" do
     expected_questions = %w[tenant_code age1 sex1 other_hhmemb relat2 age2 sex2 ecstat2]
-    expect(subject.questions.map(&:id)).to eq(expected_questions)
+    expect(sub_section.questions.map(&:id)).to eq(expected_questions)
   end
 
-  context "for a given in progress case log" do
+  context "with an in progress case log" do
     let(:case_log) { FactoryBot.build(:case_log, :in_progress) }
 
     it "has a status" do
-      expect(subject.status(case_log)).to eq(:in_progress)
+      expect(sub_section.status(case_log)).to eq(:in_progress)
     end
 
     it "has a completed status for completed subsection" do
       subsection_definition = section_definition["subsections"]["household_needs"]
-      subject = Form::Subsection.new("household_needs", subsection_definition, section)
+      sub_section = described_class.new("household_needs", subsection_definition, section)
       case_log.armedforces = "No"
       case_log.illness = "No"
       case_log.housingneeds_a = "Yes"
       case_log.la = "York"
       case_log.illness_type_1 = "Yes"
-      expect(subject.status(case_log)).to eq(:completed)
+      expect(sub_section.status(case_log)).to eq(:completed)
     end
 
     it "has status helpers" do
-      expect(subject.is_incomplete?(case_log)).to be(true)
+      expect(sub_section.is_incomplete?(case_log)).to be(true)
-      expect(subject.is_started?(case_log)).to be(true)
+      expect(sub_section.is_started?(case_log)).to be(true)
     end
 
     it "has question helpers for the number of applicable questions" do
       expected_questions = %w[tenant_code age1 sex1 other_hhmemb]
-      expect(subject.applicable_questions(case_log).map(&:id)).to eq(expected_questions)
+      expect(sub_section.applicable_questions(case_log).map(&:id)).to eq(expected_questions)
-      expect(subject.applicable_questions_count(case_log)).to eq(4)
+      expect(sub_section.applicable_questions_count(case_log)).to eq(4)
     end
 
     it "has question helpers for the number of answered questions" do
       subsection_definition = section_definition["subsections"]["household_needs"]
-      subject = Form::Subsection.new("household_needs", subsection_definition, section)
+      sub_section = described_class.new("household_needs", subsection_definition, section)
       expected_questions = %w[armedforces illness accessibility_requirements la condition_effects]
       case_log.armedforces = "No"
       case_log.illness = "No"
       case_log.housingneeds_a = "Yes"
       case_log.la = "York"
       case_log.illness_type_1 = "Yes"
-      expect(subject.answered_questions(case_log).map(&:id)).to eq(expected_questions)
+      expect(sub_section.answered_questions(case_log).map(&:id)).to eq(expected_questions)
-      expect(subject.answered_questions_count(case_log)).to eq(5)
+      expect(sub_section.answered_questions_count(case_log)).to eq(5)
     end
 
     it "has a question helpers for the unanswered questions" do
       expected_questions = %w[sex1 other_hhmemb]
-      expect(subject.unanswered_questions(case_log).map(&:id)).to eq(expected_questions)
+      expect(sub_section.unanswered_questions(case_log).map(&:id)).to eq(expected_questions)
     end
   end
 
-  context "the privacy notice has not been shown" do
+  context "when the privacy notice has not been shown" do
     let(:section_id) { "setup" }
     let(:subsection_id) { "setup" }
     let(:case_log) { FactoryBot.build(:case_log, :about_completed, gdpr_acceptance: "No") }
 
     it "does not mark the section as completed" do
-      expect(subject.status(case_log)).to eq(:in_progress)
+      expect(sub_section.status(case_log)).to eq(:in_progress)
     end
   end
 
-  context "for a given completed case log" do
+  context "with a completed case log" do
     let(:case_log) { FactoryBot.build(:case_log, :completed) }
 
     it "has a status" do
-      expect(subject.status(case_log)).to eq(:completed)
+      expect(sub_section.status(case_log)).to eq(:completed)
     end
 
     it "has status helpers" do
-      expect(subject.is_incomplete?(case_log)).to be(false)
+      expect(sub_section.is_incomplete?(case_log)).to be(false)
-      expect(subject.is_started?(case_log)).to be(true)
+      expect(sub_section.is_started?(case_log)).to be(true)
     end
   end
 end

spec/models/form_handler_spec.rb

@@ -2,9 +2,10 @@ require "rails_helper"
 
 RSpec.describe FormHandler do
   let(:test_form_name) { "2021_2022" }
+
   describe "Get all forms" do
-    it "should be able to load all the forms" do
+    it "is able to load all the forms" do
-      form_handler = FormHandler.instance
+      form_handler = described_class.instance
       all_forms = form_handler.forms
       expect(all_forms.count).to be >= 1
       expect(all_forms[test_form_name]).to be_a(Form)
@@ -12,16 +13,16 @@ RSpec.describe FormHandler do
   end
 
   describe "Get specific form" do
-    it "should be able to load a specific form" do
+    it "is able to load a specific form" do
-      form_handler = FormHandler.instance
+      form_handler = described_class.instance
       form = form_handler.get_form(test_form_name)
       expect(form).to be_a(Form)
       expect(form.pages.count).to eq(29)
     end
   end
 
-  it "should only load the form once at boot time" do
+  it "loads the form once at boot time" do
-    form_handler = FormHandler.instance
+    form_handler = described_class.instance
     expect(Form).not_to receive(:new).with(:any, test_form_name)
     expect(form_handler.get_form(test_form_name)).to be_a(Form)
   end

spec/models/form_spec.rb

@@ -8,6 +8,7 @@ RSpec.describe Form, type: :model do
 
   describe ".next_page" do
     let(:previous_page) { form.get_page("person_1_age") }
+
     it "returns the next page given the previous" do
       expect(form.next_page(previous_page, case_log)).to eq("person_1_gender")
     end
@@ -33,14 +34,15 @@ RSpec.describe Form, type: :model do
   end
 
   describe "invalidated_page_questions" do
-    context "dependencies not met" do
+    context "when dependencies are not met" do
       let(:expected_invalid) { %w[la_known cbl conditional_question_no_second_question dependent_question declaration] }
+
       it "returns an array of question keys whose pages conditions are not met" do
         expect(form.invalidated_page_questions(case_log).map(&:id).uniq).to eq(expected_invalid)
       end
     end
 
-    context "two pages with the same question, only one has dependencies met" do
+    context "with two pages having the same question and only one has dependencies met" do
       let(:expected_invalid) { %w[la_known conditional_question_no_second_question dependent_question declaration] }
 
       it "returns an array of question keys whose pages conditions are not met" do

spec/models/organisation_spec.rb

@@ -5,6 +5,7 @@ RSpec.describe Organisation, type: :model do
   before do
     RequestHelper.stub_http_requests
   end
+
   describe "#new" do
     let(:user) { FactoryBot.create(:user) }
     let(:organisation) { user.organisation }
@@ -17,7 +18,7 @@ RSpec.describe Organisation, type: :model do
       expect(organisation.users.first).to eq(user)
     end
 
-    context "case logs" do
+    context "with case logs" do
       let(:other_organisation) { FactoryBot.create(:organisation) }
       let!(:owned_case_log) do
         FactoryBot.create(

spec/models/user_spec.rb

@@ -5,6 +5,7 @@ RSpec.describe User, type: :model do
   before do
     RequestHelper.stub_http_requests
   end
+
   describe "#new" do
     let(:user) { FactoryBot.create(:user) }
     let(:other_organisation) { FactoryBot.create(:organisation) }

spec/models/validations/local_authority_validations_spec.rb

@@ -1,55 +1,34 @@
 require "rails_helper"
-require_relative "../../request_helper"
-
-RSpec.describe CaseLog do
-  let(:owning_organisation) { FactoryBot.create(:organisation) }
-  let(:managing_organisation) { owning_organisation }
-
-  before do
-    RequestHelper.stub_http_requests
-  end
-
-  describe "#new" do
-    it "raises an error when previous_postcode is present and invalid" do
-      expect {
-        CaseLog.create!(
-          previous_postcode: "invalid_postcode",
-          owning_organisation: owning_organisation,
-          managing_organisation: managing_organisation,
-        )
-      }.to raise_error(ActiveRecord::RecordInvalid, /#{I18n.t("validations.postcode")}/)
-    end
-  end
-end
 
 RSpec.describe Validations::LocalAuthorityValidations do
-  let(:subject) { subject_class.new }
+  subject(:local_auth_validator) { validator_class.new }
-  let(:subject_class) { Class.new { include Validations::LocalAuthorityValidations } }
+
+  let(:validator_class) { Class.new { include Validations::LocalAuthorityValidations } }
   let(:record) { FactoryBot.create(:case_log) }
 
   describe "#validate_previous_accommodation_postcode" do
     it "does not add an error if the record previous_postcode is missing" do
       record.previous_postcode = nil
-      subject.validate_previous_accommodation_postcode(record)
+      local_auth_validator.validate_previous_accommodation_postcode(record)
       expect(record.errors).to be_empty
     end
 
     it "does not add an error if the record previous_postcode is valid (uppercase space)" do
       record.previous_postcode = "M1 1AE"
-      subject.validate_previous_accommodation_postcode(record)
+      local_auth_validator.validate_previous_accommodation_postcode(record)
       expect(record.errors).to be_empty
     end
 
     it "does not add an error if the record previous_postcode is valid (lowercase no space)" do
       record.previous_postcode = "m11ae"
-      subject.validate_previous_accommodation_postcode(record)
+      local_auth_validator.validate_previous_accommodation_postcode(record)
       expect(record.errors).to be_empty
     end
 
     it "does add an error when the postcode is invalid" do
       record.previous_postcode = "invalid"
-      subject.validate_previous_accommodation_postcode(record)
+      local_auth_validator.validate_previous_accommodation_postcode(record)
-      expect(record.errors).to_not be_empty
+      expect(record.errors).not_to be_empty
       expect(record.errors["previous_postcode"]).to include(match I18n.t("validations.postcode"))
     end
   end

spec/models/validations/property_validations_spec.rb

@@ -1,60 +1,40 @@
 require "rails_helper"
 require_relative "../../request_helper"
 
-RSpec.describe CaseLog do
-  let(:owning_organisation) { FactoryBot.create(:organisation) }
-  let(:managing_organisation) { owning_organisation }
-
-  before do
-    RequestHelper.stub_http_requests
-  end
-
-  describe "#new" do
-    it "raises an error when offered is present and invalid" do
-      expect {
-        CaseLog.create!(
-          offered: "random",
-          owning_organisation: owning_organisation,
-          managing_organisation: managing_organisation,
-        )
-      }.to raise_error(ActiveRecord::RecordInvalid)
-    end
-  end
-end
-
 RSpec.describe Validations::PropertyValidations do
-  let(:subject) { subject_class.new }
+  subject(:property_validator) { property_validator_class.new }
-  let(:subject_class) { Class.new { include Validations::PropertyValidations } }
+
+  let(:property_validator_class) { Class.new { include Validations::PropertyValidations } }
   let(:record) { FactoryBot.create(:case_log) }
   let(:expected_error) { I18n.t("validations.property.offered.relet_number") }
 
   describe "#validate_property_number_of_times_relet" do
     it "does not add an error if the record offered is missing" do
       record.offered = nil
-      subject.validate_property_number_of_times_relet(record)
+      property_validator.validate_property_number_of_times_relet(record)
       expect(record.errors).to be_empty
     end
 
     it "does not add an error if offered is valid (number between 0 and 20)" do
       record.offered = 0
-      subject.validate_property_number_of_times_relet(record)
+      property_validator.validate_property_number_of_times_relet(record)
       expect(record.errors).to be_empty
       record.offered = 10
-      subject.validate_property_number_of_times_relet(record)
+      property_validator.validate_property_number_of_times_relet(record)
       expect(record.errors).to be_empty
       record.offered = 20
-      subject.validate_property_number_of_times_relet(record)
+      property_validator.validate_property_number_of_times_relet(record)
       expect(record.errors).to be_empty
     end
 
     it "does add an error when offered is invalid" do
       record.offered = "invalid"
-      subject.validate_property_number_of_times_relet(record)
+      property_validator.validate_property_number_of_times_relet(record)
-      expect(record.errors).to_not be_empty
+      expect(record.errors).not_to be_empty
       expect(record.errors["offered"]).to include(match(expected_error))
       record.offered = 21
-      subject.validate_property_number_of_times_relet(record)
+      property_validator.validate_property_number_of_times_relet(record)
-      expect(record.errors).to_not be_empty
+      expect(record.errors).not_to be_empty
       expect(record.errors["offered"]).to include(match(expected_error))
     end
   end

spec/presenters/error_summary_full_messages_presenter_spec.rb

@@ -1,11 +1,12 @@
 require "rails_helper"
 
 RSpec.describe ErrorSummaryFullMessagesPresenter do
+  subject(:error_summary_presenter) { described_class.new(error_messages) }
+
   let(:error_messages) { { reset_password_token: %w[expired] } }
   let(:formatted_error_messages) { [[:reset_password_token, "Reset password token expired"]] }
-  subject { described_class.new(error_messages) }
 
   it "formats messages to include the attribute name" do
-    expect(subject.formatted_error_messages).to eq(formatted_error_messages)
+    expect(error_summary_presenter.formatted_error_messages).to eq(formatted_error_messages)
   end
 end

spec/requests/auth/passwords_controller_spec.rb

@@ -4,7 +4,7 @@ require_relative "../../support/devise"
 RSpec.describe Auth::PasswordsController, type: :request do
   let(:params) { { user: { email: email } } }
   let(:page) { Capybara::Node::Simple.new(response.body) }
-  let(:notify_client) { double(Notifications::Client) }
+  let(:notify_client) { instance_double(Notifications::Client) }
 
   before do
     allow_any_instance_of(DeviseNotifyMailer).to receive(:notify_client).and_return(notify_client)
@@ -25,7 +25,7 @@ RSpec.describe Auth::PasswordsController, type: :request do
 
   context "when a password reset is requested with an email that doesn't exist in the system" do
     before do
-      allow_any_instance_of(Auth::PasswordsController).to receive(:is_navigational_format?).and_return(false)
+      allow_any_instance_of(described_class).to receive(:is_navigational_format?).and_return(false)
     end
 
     let(:email) { "madeup_email@test.com" }
@@ -38,7 +38,7 @@ RSpec.describe Auth::PasswordsController, type: :request do
     end
   end
 
-  context "#Update - reset password" do
+  describe "#Update - reset password" do
     let(:user) { FactoryBot.create(:user) }
     let(:token) { user.send(:set_reset_password_token) }
     let(:updated_password) { "updated_password_280" }

spec/requests/bulk_upload_controller_spec.rb

@@ -4,31 +4,33 @@ RSpec.describe BulkUploadController, type: :request do
   let(:url) { "/logs/bulk-upload" }
   let(:user) { FactoryBot.create(:user) }
   let(:organisation) { user.organisation }
+  let(:valid_file) { fixture_file_upload("2021_22_lettings_bulk_upload.xlsx", "application/vnd.ms-excel") }
+  let(:invalid_file) { fixture_file_upload("random.txt", "text/plain") }
+  let(:empty_file) { fixture_file_upload("2021_22_lettings_bulk_upload_empty.xlsx", "application/vnd.ms-excel") }
+
   before do
     allow(Organisation).to receive(:find).with(107_242).and_return(organisation)
   end
 
-  context "a not signed in user" do
+  context "when a user is not signed in" do
     describe "GET #show" do
+      before { get url, headers: headers, params: {} }
+
       it "does not let you see the bulk upload page" do
-        get url, headers: headers, params: {}
         expect(response).to redirect_to("/users/sign-in")
       end
     end
 
     describe "POST #bulk upload" do
-      before do
+      before { post url, params: { bulk_upload: { case_log_bulk_upload: valid_file } } }
-        @file = fixture_file_upload("2021_22_lettings_bulk_upload.xlsx", "application/vnd.ms-excel")
-      end
 
       it "does not let you submit bulk uploads" do
-        post url, params: { bulk_upload: { case_log_bulk_upload: @file } }
         expect(response).to redirect_to("/users/sign-in")
       end
     end
   end
 
-  context "a signed in user" do
+  context "when a user is signed in" do
     before do
       sign_in user
     end
@@ -49,40 +51,31 @@ RSpec.describe BulkUploadController, type: :request do
     end
 
     describe "POST #bulk upload" do
-      subject { post url, params: { bulk_upload: { case_log_bulk_upload: @file } } }
+      context "with a valid file based on the upload template" do
-
+        let(:request) { post url, params: { bulk_upload: { case_log_bulk_upload: valid_file } } }
-      context "given a valid file based on the upload template" do
-        before do
-          @file = fixture_file_upload("2021_22_lettings_bulk_upload.xlsx", "application/vnd.ms-excel")
-        end
 
         it "creates case logs for each row in the template" do
-          expect { subject }.to change(CaseLog, :count).by(9)
+          expect { request }.to change(CaseLog, :count).by(9)
         end
 
         it "redirects to the case log index page" do
-          expect(subject).to redirect_to(case_logs_path)
+          expect(request).to redirect_to(case_logs_path)
         end
       end
 
-      context "given an invalid file type" do
+      context "with an invalid file type" do
-        before do
+        before { post url, params: { bulk_upload: { case_log_bulk_upload: invalid_file } } }
-          @file = fixture_file_upload("random.txt", "text/plain")
-          subject
-        end
 
         it "displays an error message" do
           expect(response.body).to match(/Invalid file type/)
         end
       end
 
-      context "given an empty file" do
+      context "with an empty file" do
-        before do
+        let(:request) { post url, params: { bulk_upload: { case_log_bulk_upload: empty_file } } }
-          @file = fixture_file_upload("2021_22_lettings_bulk_upload_empty.xlsx", "application/vnd.ms-excel")
-          subject
-        end
 
         it "displays an error message" do
+          request
           expect(response.body).to match(/No data found/)
         end
       end

spec/requests/case_logs_controller_spec.rb

@@ -64,7 +64,7 @@ RSpec.describe CaseLogsController, type: :request do
       expect(json_response["property_postcode"]).to eq(property_postcode)
     end
 
-    context "invalid json params" do
+    context "with invalid json parameters" do
       let(:age1) { 2000 }
       let(:offered) { 21 }
 
@@ -75,14 +75,14 @@ RSpec.describe CaseLogsController, type: :request do
       end
     end
 
-    context "partial case log submission" do
+    context "with a partial case log submission" do
       it "marks the record as in_progress" do
         json_response = JSON.parse(response.body)
         expect(json_response["status"]).to eq(in_progress)
       end
     end
 
-    context "complete case log submission" do
+    context "with a complete case log submission" do
       let(:org_params) do
         {
           "case_log" => {
@@ -104,7 +104,7 @@ RSpec.describe CaseLogsController, type: :request do
       end
     end
 
-    context "request with invalid credentials" do
+    context "with a request containing invalid credentials" do
       let(:basic_credentials) do
         ActionController::HttpAuthentication::Basic.encode_credentials(api_username, "Oops")
       end
@@ -134,7 +134,7 @@ RSpec.describe CaseLogsController, type: :request do
       )
     end
 
-    context "collection" do
+    context "when displaying a collection of logs" do
       let(:headers) { { "Accept" => "text/html" } }
 
       before do
@@ -151,7 +151,7 @@ RSpec.describe CaseLogsController, type: :request do
       end
     end
 
-    context "member" do
+    context "when requesting a specific case log" do
       let(:completed_case_log) { FactoryBot.create(:case_log, :completed) }
       let(:id) { completed_case_log.id }
 
@@ -168,7 +168,7 @@ RSpec.describe CaseLogsController, type: :request do
         expect(json_response["status"]).to eq(completed_case_log.status)
       end
 
-      context "invalid case log id" do
+      context "when requesting an invalid case log id" do
         let(:id) { (CaseLog.order(:id).last&.id || 0) + 1 }
 
         it "returns 404" do
@@ -176,18 +176,18 @@ RSpec.describe CaseLogsController, type: :request do
         end
       end
 
-      context "edit log" do
+      context "when editing a case log" do
         let(:headers) { { "Accept" => "text/html" } }
 
-        context "a user that is not signed in" do
+        context "with a user that is not signed in" do
           it "does not let the user get case log tasklist pages they don't have access to" do
             get "/logs/#{case_log.id}", headers: headers, params: {}
             expect(response).to redirect_to("/users/sign-in")
           end
         end
 
-        context "a signed in user" do
+        context "with a signed in user" do
-          context "case logs that are owned or managed by your organisation" do
+          context "with case logs that are owned or managed by your organisation" do
             before do
               sign_in user
               get "/logs/#{case_log.id}", headers: headers, params: {}
@@ -205,7 +205,7 @@ RSpec.describe CaseLogsController, type: :request do
             end
           end
 
-          context "case log with a single section complete" do
+          context "with a case log with a single section complete" do
             let(:section_completed_case_log) do
               FactoryBot.create(
                 :case_log,
@@ -227,7 +227,7 @@ RSpec.describe CaseLogsController, type: :request do
             end
           end
 
-          context "case logs that are not owned or managed by your organisation" do
+          context "with case logs that are not owned or managed by your organisation" do
             before do
               sign_in user
               get "/logs/#{unauthorized_case_log.id}", headers: headers, params: {}
@@ -241,7 +241,7 @@ RSpec.describe CaseLogsController, type: :request do
       end
     end
 
-    context "Check answers" do
+    context "when accessing the check answers page" do
       let(:postcode_case_log) do
         FactoryBot.create(:case_log,
                           owning_organisation: organisation,
@@ -308,7 +308,7 @@ RSpec.describe CaseLogsController, type: :request do
       expect(case_log.property_postcode).to eq("M1 1AE")
     end
 
-    context "invalid case log id" do
+    context "with an invalid case log id" do
       let(:id) { (CaseLog.order(:id).last&.id || 0) + 1 }
 
       it "returns 404" do
@@ -316,7 +316,7 @@ RSpec.describe CaseLogsController, type: :request do
       end
     end
 
-    context "invalid case log params" do
+    context "with an invalid case log params" do
       let(:params) { { age1: 200 } }
 
       it "returns 422" do
@@ -329,7 +329,7 @@ RSpec.describe CaseLogsController, type: :request do
       end
     end
 
-    context "request with invalid credentials" do
+    context "with a request containing invalid credentials" do
       let(:basic_credentials) do
         ActionController::HttpAuthentication::Basic.encode_credentials(api_username, "Oops")
       end
@@ -366,7 +366,7 @@ RSpec.describe CaseLogsController, type: :request do
       expect(case_log.property_postcode).to eq("SW1A 2AA")
     end
 
-    context "invalid case log id" do
+    context "with an invalid case log id" do
       let(:id) { (CaseLog.order(:id).last&.id || 0) + 1 }
 
       it "returns 404" do
@@ -374,7 +374,7 @@ RSpec.describe CaseLogsController, type: :request do
       end
     end
 
-    context "request with invalid credentials" do
+    context "with a request containing invalid credentials" do
       let(:basic_credentials) do
         ActionController::HttpAuthentication::Basic.encode_credentials(api_username, "Oops")
       end
@@ -391,7 +391,7 @@ RSpec.describe CaseLogsController, type: :request do
     end
     let(:id) { case_log.id }
 
-    context "expected deletion" do
+    context "when deleting a case log" do
       before do
         delete "/logs/#{id}", headers: headers
       end
@@ -405,7 +405,7 @@ RSpec.describe CaseLogsController, type: :request do
         expect(CaseLog.with_discarded.find(id)).to be_a(CaseLog)
       end
 
-      context "invalid case log id" do
+      context "with an invalid case log id" do
         let(:id) { (CaseLog.order(:id).last&.id || 0) + 1 }
 
         it "returns 404" do
@@ -413,7 +413,7 @@ RSpec.describe CaseLogsController, type: :request do
         end
       end
 
-      context "request with invalid credentials" do
+      context "with a request containing invalid credentials" do
         let(:basic_credentials) do
           ActionController::HttpAuthentication::Basic.encode_credentials(api_username, "Oops")
         end
@@ -424,7 +424,7 @@ RSpec.describe CaseLogsController, type: :request do
       end
     end
 
-    context "deletion fails" do
+    context "when a case log deletion fails" do
       before do
         allow_any_instance_of(CaseLog).to receive(:discard).and_return(false)
         delete "/logs/#{id}", headers: headers

spec/requests/form_controller_spec.rb

@@ -20,7 +20,7 @@ RSpec.describe FormController, type: :request do
   end
   let(:headers) { { "Accept" => "text/html" } }
 
-  context "a not signed in user" do
+  context "when a user is not signed in" do
     describe "GET" do
       it "does not let you get case logs pages you don't have access to" do
         get "/logs/#{case_log.id}/person-1-age", headers: headers, params: {}
@@ -41,14 +41,14 @@ RSpec.describe FormController, type: :request do
     end
   end
 
-  context "a signed in user" do
+  context "when a user is signed in" do
     before do
       sign_in user
     end
 
     describe "GET" do
-      context "form pages" do
+      context "with form pages" do
-        context "forms exist for multiple years" do
+        context "when forms exist for multiple years" do
           let(:case_log_year_1) { FactoryBot.create(:case_log, startdate: Time.zone.local(2021, 5, 1), owning_organisation: organisation) }
           let(:case_log_year_2) { FactoryBot.create(:case_log, :about_completed, startdate: Time.zone.local(2022, 5, 1), owning_organisation: organisation) }
 
@@ -60,14 +60,14 @@ RSpec.describe FormController, type: :request do
           end
         end
 
-        context "case logs that are not owned or managed by your organisation" do
+        context "when case logs are not owned or managed by your organisation" do
           it "does not show form pages for case logs you don't have access to" do
             get "/logs/#{unauthorized_case_log.id}/person-1-age", headers: headers, params: {}
             expect(response).to have_http_status(:not_found)
           end
         end
 
-        context "a form page that has custom guidance" do
+        context "with a form page that has custom guidance" do
           it "displays the correct partial" do
             get "/logs/#{case_log.id}/net-income", headers: headers, params: {}
             expect(response.body).to match("What counts as income?")
@@ -75,8 +75,8 @@ RSpec.describe FormController, type: :request do
         end
       end
 
-      context "check answers pages" do
+      context "when displaying check answers pages" do
-        context "case logs that are not owned or managed by your organisation" do
+        context "when case logs are not owned or managed by your organisation" do
           it "does not show a check answers for case logs you don't have access to" do
             get "/logs/#{unauthorized_case_log.id}/household-characteristics/check-answers", headers: headers, params: {}
             expect(response).to have_http_status(:not_found)
@@ -84,14 +84,14 @@ RSpec.describe FormController, type: :request do
         end
       end
 
-      context "a question that in a section that isn't enabled yet" do
+      context "with a question in a section that isn't enabled yet" do
         it "routes back to the tasklist page" do
           get "/logs/#{case_log.id}/declaration", headers: headers, params: {}
           expect(response).to redirect_to("/logs/#{case_log.id}")
         end
       end
 
-      context "a question that isn't enabled yet" do
+      context "with a question that isn't enabled yet" do
         it "routes back to the tasklist page" do
           get "/logs/#{case_log.id}/conditional-question-no-second-page", headers: headers, params: {}
           expect(response).to redirect_to("/logs/#{case_log.id}")
@@ -100,7 +100,7 @@ RSpec.describe FormController, type: :request do
     end
 
     describe "Submit Form" do
-      context "a form page" do
+      context "with a form page" do
         let(:user) { FactoryBot.create(:user) }
         let(:organisation) { user.organisation }
         let(:case_log) do
@@ -125,7 +125,7 @@ RSpec.describe FormController, type: :request do
           post "/logs/#{case_log.id}/form", params: params
         end
 
-        context "invalid answers" do
+        context "with invalid answers" do
           let(:answer) { 2000 }
 
           it "re-renders the same page with errors if validation fails" do
@@ -133,13 +133,8 @@ RSpec.describe FormController, type: :request do
           end
         end
 
-        context "valid answers" do
+        context "with valid answers" do
           let(:answer) { 20 }
-
-          it "re-renders the same page with errors if validation fails" do
-            expect(response).to have_http_status(:redirect)
-          end
-
           let(:params) do
             {
               id: case_log.id,
@@ -151,6 +146,10 @@ RSpec.describe FormController, type: :request do
             }
           end
 
+          it "re-renders the same page with errors if validation fails" do
+            expect(response).to have_http_status(:redirect)
+          end
+
           it "only updates answers that apply to the page being submitted" do
             case_log.reload
             expect(case_log.age1).to eq(answer)
@@ -159,7 +158,7 @@ RSpec.describe FormController, type: :request do
         end
       end
 
-      context "checkbox questions" do
+      context "with checkbox questions" do
         let(:case_log_form_params) do
           {
             id: case_log.id,
@@ -196,7 +195,7 @@ RSpec.describe FormController, type: :request do
           expect(case_log.housingneeds_c).to eq("Yes")
         end
 
-        context "given a page with checkbox and non-checkbox questions" do
+        context "with a page having checkbox and non-checkbox questions" do
           let(:tenant_code) { "BZ355" }
           let(:case_log_form_params) do
             {
@@ -244,7 +243,7 @@ RSpec.describe FormController, type: :request do
         end
       end
 
-      context "conditional routing" do
+      context "with conditional routing" do
         before do
           allow_any_instance_of(CaseLogValidator).to receive(:validate_pregnancy).and_return(true)
         end
@@ -294,7 +293,7 @@ RSpec.describe FormController, type: :request do
         end
       end
 
-      context "case logs that are not owned or managed by your organisation" do
+      context "with case logs that are not owned or managed by your organisation" do
         let(:answer) { 25 }
         let(:other_organisation) { FactoryBot.create(:organisation) }
         let(:unauthorized_case_log) do

spec/requests/organisations_controller_spec.rb

@@ -9,7 +9,7 @@ RSpec.describe OrganisationsController, type: :request do
   let(:new_value) { "Test Name 35" }
   let(:params) { { id: organisation.id, organisation: { name: new_value } } }
 
-  context "a not signed in user" do
+  context "when user is not signed in" do
     describe "#show" do
       it "does not let you see organisation details from org route" do
         get "/organisations/#{organisation.id}", headers: headers, params: {}
@@ -28,9 +28,9 @@ RSpec.describe OrganisationsController, type: :request do
     end
   end
 
-  context "a signed in user" do
+  context "when user is signed in" do
     describe "#show" do
-      context "organisation that the user belongs to" do
+      context "with an organisation that the user belongs to" do
         before do
           sign_in user
           get "/organisations/#{organisation.id}", headers: headers, params: {}
@@ -41,7 +41,7 @@ RSpec.describe OrganisationsController, type: :request do
         end
       end
 
-      context "organisation that are not in scope for the user, i.e. that they do not belong to" do
+      context "with an organisation that are not in scope for the user, i.e. that they do not belong to" do
         before do
           sign_in user
           get "/organisations/#{unauthorised_organisation.id}", headers: headers, params: {}
@@ -57,9 +57,9 @@ RSpec.describe OrganisationsController, type: :request do
       end
     end
 
-    context "As a data coordinator user" do
+    context "with a data coordinator user" do
-      context "details tab" do
+      context "when we access the details tab" do
-        context "organisation that the user belongs to" do
+        context "with an organisation that the user belongs to" do
           before do
             sign_in user
             get "/organisations/#{organisation.id}/details", headers: headers, params: {}
@@ -87,7 +87,7 @@ RSpec.describe OrganisationsController, type: :request do
           end
         end
 
-        context "organisation that are not in scope for the user, i.e. that they do not belong to" do
+        context "with organisation that are not in scope for the user, i.e. that they do not belong to" do
           before do
             sign_in user
             get "/organisations/#{unauthorised_organisation.id}/details", headers: headers, params: {}
@@ -99,8 +99,8 @@ RSpec.describe OrganisationsController, type: :request do
         end
       end
 
-      context "users tab" do
+      context "when accessing the users tab" do
-        context "organisation that the user belongs to" do
+        context "with an organisation that the user belongs to" do
           before do
             sign_in user
             get "/organisations/#{organisation.id}/users", headers: headers, params: {}
@@ -127,7 +127,7 @@ RSpec.describe OrganisationsController, type: :request do
           end
         end
 
-        context "organisation that are not in scope for the user, i.e. that they do not belong to" do
+        context "with an organisation that are not in scope for the user, i.e. that they do not belong to" do
           before do
             sign_in user
             get "/organisations/#{unauthorised_organisation.id}/users", headers: headers, params: {}
@@ -139,8 +139,8 @@ RSpec.describe OrganisationsController, type: :request do
         end
       end
 
-      context "#edit" do
+      describe "#edit" do
-        context "organisation that the user belongs to" do
+        context "with an organisation that the user belongs to" do
           before do
             sign_in user
             get "/organisations/#{organisation.id}/edit", headers: headers, params: {}
@@ -153,7 +153,7 @@ RSpec.describe OrganisationsController, type: :request do
           end
         end
 
-        context "organisation that the user does not belong to" do
+        context "with an organisation that the user does not belong to" do
           before do
             sign_in user
             get "/organisations/#{unauthorised_organisation.id}/edit", headers: headers, params: {}
@@ -165,8 +165,8 @@ RSpec.describe OrganisationsController, type: :request do
         end
       end
 
-      context "#update" do
+      describe "#update" do
-        context "organisation that the user belongs to" do
+        context "with an organisation that the user belongs to" do
           before do
             sign_in user
             patch "/organisations/#{organisation.id}", headers: headers, params: params
@@ -187,7 +187,7 @@ RSpec.describe OrganisationsController, type: :request do
           end
         end
 
-        context "organisation that the user does not belong to" do
+        context "with an organisation that the user does not belong to" do
           before do
             sign_in user
             patch "/organisations/#{unauthorised_organisation.id}", headers: headers, params: {}
@@ -200,11 +200,11 @@ RSpec.describe OrganisationsController, type: :request do
       end
     end
 
-    context "As a data provider user" do
+    context "with a data provider user" do
       let(:user) { FactoryBot.create(:user) }
 
-      context "details tab" do
+      context "when accessing the details tab" do
-        context "organisation that the user belongs to" do
+        context "with an organisation that the user belongs to" do
           before do
             sign_in user
             get "/organisations/#{organisation.id}/details", headers: headers, params: {}
@@ -232,7 +232,7 @@ RSpec.describe OrganisationsController, type: :request do
           end
         end
 
-        context "organisation that are not in scope for the user, i.e. that they do not belong to" do
+        context "with an organisation that is not in scope for the user, i.e. that they do not belong to" do
           before do
             sign_in user
             get "/organisations/#{unauthorised_organisation.id}/details", headers: headers, params: {}
@@ -244,18 +244,18 @@ RSpec.describe OrganisationsController, type: :request do
         end
       end
 
-      context "users tab" do
+      context "when accessing the users tab" do
         before do
           sign_in user
           get "/organisations/#{organisation.id}/users", headers: headers, params: {}
         end
 
-        it "should return unauthorized 401" do
+        it "returns unauthorized 401" do
           expect(response).to have_http_status(:unauthorized)
         end
       end
 
-      context "#edit" do
+      describe "#edit" do
         before do
           sign_in user
           get "/organisations/#{organisation.id}/edit", headers: headers, params: {}
@@ -266,7 +266,7 @@ RSpec.describe OrganisationsController, type: :request do
         end
       end
 
-      context "#update" do
+      describe "#update" do
         before do
           sign_in user
           patch "/organisations/#{organisation.id}", headers: headers, params: params

spec/requests/soft_validations_controller_spec.rb

@@ -1,4 +1,5 @@
 require "rails_helper"
+require_relative "../request_helper"
 
 RSpec.describe SoftValidationsController, type: :request do
   let(:params) { { case_log_id: case_log.id } }
@@ -9,7 +10,7 @@ RSpec.describe SoftValidationsController, type: :request do
     RequestHelper.stub_http_requests
   end
 
-  context "a not signed in user" do
+  context "when a user is not signed in" do
     let(:case_log) { FactoryBot.create(:case_log, :in_progress) }
 
     describe "GET #show" do
@@ -20,14 +21,14 @@ RSpec.describe SoftValidationsController, type: :request do
     end
   end
 
-  context "a signed in user" do
+  context "when a user is signed in" do
     before do
       sign_in user
       get url, params: {}
     end
 
     describe "GET #show" do
-      context "Soft validation overide required" do
+      context "when a soft validation is triggered" do
         let(:case_log) { FactoryBot.create(:case_log, :soft_validations_triggered) }
 
         it "returns a success response" do
@@ -41,14 +42,14 @@ RSpec.describe SoftValidationsController, type: :request do
         end
       end
 
-      context "Soft validation overide not required" do
+      context "when no soft validation is triggered" do
         let(:case_log) { FactoryBot.create(:case_log, :in_progress) }
 
         it "returns a success response" do
           expect(response).to be_successful
         end
 
-        it "returns a json with the soft validation fields" do
+        it "returns a json without the soft validation fields" do
           json_response = JSON.parse(response.body)
           expect(json_response["show"]).to eq(false)
         end

spec/requests/users_controller_spec.rb

@@ -7,14 +7,14 @@ RSpec.describe UsersController, type: :request do
   let(:page) { Capybara::Node::Simple.new(response.body) }
   let(:new_value) { "new test name" }
   let(:params) { { id: user.id, user: { name: new_value } } }
-  let(:notify_client) { double(Notifications::Client) }
+  let(:notify_client) { instance_double(Notifications::Client) }
 
   before do
     allow_any_instance_of(DeviseNotifyMailer).to receive(:notify_client).and_return(notify_client)
     allow(notify_client).to receive(:send_email).and_return(true)
   end
 
-  context "a not signed in user" do
+  context "when user is not signed in" do
     describe "#show" do
       it "does not let you see user details" do
         get "/users/#{user.id}", headers: headers, params: {}
@@ -50,8 +50,8 @@ RSpec.describe UsersController, type: :request do
         expect(page).to have_css("h1", class: "govuk-heading-l", text: "Reset your password")
       end
 
-      context "update password" do
+      context "when updating a user password" do
-        context "valid reset token" do
+        context "when the reset token is valid" do
           let(:params) do
             {
               id: user.id, user: { password: new_value, password_confirmation: "something_else" }
@@ -70,7 +70,7 @@ RSpec.describe UsersController, type: :request do
           end
         end
 
-        context "reset token more than 3 hours old" do
+        context "when a reset token is more than 3 hours old" do
           let(:raw) { user.send_reset_password_instructions }
           let(:params) do
             {
@@ -107,7 +107,7 @@ RSpec.describe UsersController, type: :request do
   end
 
   describe "#show" do
-    context "current user is user" do
+    context "when the current user matches the user ID" do
       before do
         sign_in user
         get "/users/#{user.id}", headers: headers, params: {}
@@ -118,7 +118,7 @@ RSpec.describe UsersController, type: :request do
       end
     end
 
-    context "current user is another user" do
+    context "when the current user does not matches the user ID" do
       before do
         sign_in user
         get "/users/#{unauthorised_user.id}", headers: headers, params: {}
@@ -135,7 +135,7 @@ RSpec.describe UsersController, type: :request do
   end
 
   describe "#edit" do
-    context "current user is user" do
+    context "when the current user matches the user ID" do
       before do
         sign_in user
         get "/users/#{user.id}/edit", headers: headers, params: {}
@@ -146,7 +146,7 @@ RSpec.describe UsersController, type: :request do
       end
     end
 
-    context "current user is another user" do
+    context "when the current user does not matches the user ID" do
       before do
         sign_in user
         get "/users/#{unauthorised_user.id}/edit", headers: headers, params: {}
@@ -159,7 +159,7 @@ RSpec.describe UsersController, type: :request do
   end
 
   describe "#edit_password" do
-    context "current user is user" do
+    context "when the current user matches the user ID" do
       before do
         sign_in user
         get "/users/#{user.id}/password/edit", headers: headers, params: {}
@@ -170,7 +170,7 @@ RSpec.describe UsersController, type: :request do
       end
     end
 
-    context "current user is another user" do
+    context "when the current user does not matches the user ID" do
       before do
         sign_in user
         get "/users/#{unauthorised_user.id}/edit", headers: headers, params: {}
@@ -183,7 +183,7 @@ RSpec.describe UsersController, type: :request do
   end
 
   describe "#update" do
-    context "current user is user" do
+    context "when the current user matches the user ID" do
       before do
         sign_in user
         patch "/users/#{user.id}", headers: headers, params: params
@@ -195,7 +195,7 @@ RSpec.describe UsersController, type: :request do
       end
     end
 
-    context "update fails to persist" do
+    context "when the update fails to persist" do
       before do
         allow_any_instance_of(User).to receive(:update).and_return(false)
         sign_in user
@@ -207,7 +207,7 @@ RSpec.describe UsersController, type: :request do
       end
     end
 
-    context "current user is another user" do
+    context "when the current user does not matches the user ID" do
       let(:params) { { id: unauthorised_user.id, user: { name: new_value } } }
 
       before do
@@ -220,7 +220,7 @@ RSpec.describe UsersController, type: :request do
       end
     end
 
-    context "update password" do
+    context "when we update the user password" do
       let(:params) do
         {
           id: user.id, user: { password: new_value, password_confirmation: "something_else" }

spec/services/paas_configuration_service_spec.rb

@@ -1,27 +1,27 @@
 require "rails_helper"
 
-RSpec.describe "PaasConfigurationService" do
+RSpec.describe PaasConfigurationService do
-  context "when the paas configuration is unavailable" do
+  subject(:config_service) { described_class.new(logger) }
-    subject { PaasConfigurationService.new(logger) }
+
-    let(:logger) { double("logger") }
+  let(:logger) { instance_double(ActiveSupport::LogSubscriber) }
 
+  context "when the paas configuration is unavailable" do
     before { allow(logger).to receive(:warn) }
 
     it "returns the configuration as not present" do
-      expect(subject.config_present?).to be(false)
+      expect(config_service.config_present?).to be(false)
     end
 
     it "returns the S3 configuration as not present" do
-      expect(subject.s3_config_present?).to be(false)
+      expect(config_service.s3_config_present?).to be(false)
     end
 
     it "does not retrieve any S3 bucket configuration" do
-      expect(subject.s3_buckets).to be_empty
+      expect(config_service.s3_buckets).to be_empty
     end
   end
 
   context "when the paas configuration is present with S3 buckets" do
-    subject { PaasConfigurationService.new(double("logger")) }
     let(:vcap_services) do
       <<-JSON
         {"aws-s3-bucket": [{"instance_name": "bucket_1"},{"instance_name": "bucket_2"}]}
@@ -33,17 +33,17 @@ RSpec.describe "PaasConfigurationService" do
     end
 
     it "returns the configuration as present" do
-      expect(subject.config_present?).to be(true)
+      expect(config_service.config_present?).to be(true)
     end
 
     it "returns the S3 configuration as present" do
-      expect(subject.s3_config_present?).to be(true)
+      expect(config_service.s3_config_present?).to be(true)
     end
 
     it "does retrieve the S3 bucket configurations" do
-      s3_buckets = subject.s3_buckets
+      s3_buckets = config_service.s3_buckets
 
-      expect(s3_buckets).to_not be_empty
+      expect(s3_buckets).not_to be_empty
       expect(s3_buckets.count).to be(2)
       expect(s3_buckets).to have_key(:bucket_1)
       expect(s3_buckets).to have_key(:bucket_2)
@@ -51,22 +51,20 @@ RSpec.describe "PaasConfigurationService" do
   end
 
   context "when the paas configuration is present without S3 buckets" do
-    subject { PaasConfigurationService.new(double("logger")) }
-
     before do
       allow(ENV).to receive(:[]).with("VCAP_SERVICES").and_return("{}")
     end
 
     it "returns the configuration as present" do
-      expect(subject.config_present?).to be(true)
+      expect(config_service.config_present?).to be(true)
     end
 
     it "returns the S3 configuration as not present" do
-      expect(subject.s3_config_present?).to be(false)
+      expect(config_service.s3_config_present?).to be(false)
     end
 
     it "does not retrieve any S3 bucket configuration" do
-      expect(subject.s3_buckets).to be_empty
+      expect(config_service.s3_buckets).to be_empty
     end
   end
 end

spec/support/controller_macros.rb

@@ -1,6 +1,6 @@
 module ControllerMacros
   def login_user
-    before(:each) do
+    before do
       @request.env["devise.mapping"] = Devise.mappings[:user]
       user = FactoryBot.create(:user)
       sign_in user
@@ -8,7 +8,7 @@ module ControllerMacros
   end
 
   def login_admin_user
-    before(:each) do
+    before do
       @request.env["devise.mapping"] = Devise.mappings[:admin_user]
       admin_user = FactoryBot.create(:admin_user)
       sign_in admin_user

spec/views/case_log_index_view_spec.rb

@@ -9,11 +9,14 @@ RSpec.describe "case_logs/index" do
   let(:in_progress_log) { FactoryBot.create(:case_log, :in_progress) }
   let(:completed_log) { FactoryBot.create(:case_log, :completed) }
 
-  context "given an in progress log list" do
+  context "with an in progress log list" do
-    it "renders a table for in progress logs only" do
+    before do
       assign(:in_progress_case_logs, [in_progress_log])
       assign(:completed_case_logs, [])
       render
+    end
+
+    it "renders a table for in progress logs only" do
       expect(rendered).to match(/<table class="govuk-table">/)
       expect(rendered).to match(/Logs you need to complete/)
       expect(rendered).not_to match(/Logs you’ve submitted/)
@@ -22,11 +25,14 @@ RSpec.describe "case_logs/index" do
     end
   end
 
-  context "given a completed log list" do
+  context "with a completed log list" do
-    it "renders a table for in progress logs only" do
+    before do
       assign(:in_progress_case_logs, [])
       assign(:completed_case_logs, [completed_log])
       render
+    end
+
+    it "renders a table for in progress logs only" do
       expect(rendered).to match(/<table class="govuk-table">/)
       expect(rendered).to match(/Logs you’ve submitted/)
       expect(rendered).not_to match(/Logs you need to complete/)
@@ -35,11 +41,14 @@ RSpec.describe "case_logs/index" do
     end
   end
 
-  context "given a completed log list and an in_progress log list" do
+  context "with a completed log list and an in_progress log list" do
-    it "renders two tables, one for each status" do
+    before do
       assign(:in_progress_case_logs, [in_progress_log])
       assign(:completed_case_logs, [completed_log])
       render
+    end
+
+    it "renders two tables, one for each status" do
       expect(rendered).to match(/<table class="govuk-table">/)
       expect(rendered).to match(/Logs you’ve submitted/)
       expect(rendered).to match(/Logs you need to complete/)

spec/views/form/page_view_spec.rb

@@ -2,10 +2,6 @@ require "rails_helper"
 require_relative "../../request_helper"
 
 RSpec.describe "form/page" do
-  before do
-    RequestHelper.stub_http_requests
-  end
-
   let(:case_log) { FactoryBot.create(:case_log, :in_progress) }
   let(:form) { case_log.form }
   let(:subsection) { form.get_subsection("income_and_benefits") }
@@ -23,6 +19,7 @@ RSpec.describe "form/page" do
   end
 
   before do
+    RequestHelper.stub_http_requests
     assign(:case_log, case_log)
     assign(:page, page)
     assign(:subsection, subsection)
@@ -38,7 +35,7 @@ RSpec.describe "form/page" do
     assign_attributes(question, initial_question_attributes)
   end
 
-  context "given a page with a description" do
+  context "with a page containing a description" do
     let(:description) { "Test description <a class=\"govuk-link\" href=\"/files/privacy-notice.pdf\">with link</a>." }
     let(:page_attributes) { { description: description } }
     let(:expected_html) { '<p class="govuk-body govuk-body-m">Test description <a class="govuk-link" href="/files/privacy-notice.pdf">with link</a>.</p>' }
@@ -48,14 +45,14 @@ RSpec.describe "form/page" do
     end
   end
 
-  context "given a page with a header" do
+  context "with a page containing a header" do
     it "renders the header and the subsection label" do
       expect(rendered).to match(page.header)
       expect(rendered).to match(subsection.label)
     end
   end
 
-  context "given a page with a header and hide_subsection_label true" do
+  context "with a page containing a header and hide_subsection_label true" do
     let(:page_attributes) { { hide_subsection_label: true } }
 
     it "renders the header but not the subsection label" do
@@ -64,7 +61,7 @@ RSpec.describe "form/page" do
     end
   end
 
-  context "given a numeric question with prefix and suffix" do
+  context "when rendering a numeric question with prefix and suffix" do
     let(:question_attributes) { { type: "numeric", prefix: "£", suffix: "every week" } }
 
     it "renders prefix and suffix text" do
@@ -75,11 +72,12 @@ RSpec.describe "form/page" do
     end
   end
 
-  context "given a question with extra guidance" do
+  context "with a question containing extra guidance" do
     let(:expected_guidance) { /What counts as income?/ }
 
     context "with radio type" do
       let(:question_attributes) { { type: "radio", answer_options: { "1": "A", "2": "B" } } }
+
       it "renders the guidance partial for radio questions" do
         expect(rendered).to match(expected_guidance)
       end
@@ -87,6 +85,7 @@ RSpec.describe "form/page" do
 
     context "with text type" do
       let(:question_attributes) { { type: "text", answer_options: nil } }
+
       it "renders the guidance partial for text questions" do
         expect(rendered).to match(expected_guidance)
       end
@@ -94,6 +93,7 @@ RSpec.describe "form/page" do
 
     context "with numeric type" do
       let(:question_attributes) { { type: "numeric", answer_options: nil } }
+
       it "renders the guidance partial for numeric questions" do
         expect(rendered).to match(expected_guidance)
       end
@@ -101,6 +101,7 @@ RSpec.describe "form/page" do
 
     context "with select type" do
       let(:question_attributes) { { type: "select", answer_options: { "1": "A", "2": "B" } } }
+
       it "renders the guidance partial for select questions" do
         expect(rendered).to match(expected_guidance)
       end
@@ -108,6 +109,7 @@ RSpec.describe "form/page" do
 
     context "with checkbox type" do
       let(:question_attributes) { { type: "checkbox", answer_options: { "1": "A", "2": "B" } } }
+
       it "renders the guidance partial for checkbox questions" do
         expect(rendered).to match(expected_guidance)
       end
@@ -115,6 +117,7 @@ RSpec.describe "form/page" do
 
     context "with date type" do
       let(:question_attributes) { { type: "date", answer_options: nil } }
+
       it "renders the guidance partial for date questions" do
         expect(rendered).to match(expected_guidance)
       end