Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert Frontend Test Runner from Minitest to RSpec #4090

Merged
merged 75 commits into from
Jul 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
75 commits
Select commit Hold shift + click to select a range
e9e7b58
Fix bool method returning non-bool values
KludgeKML Jun 4, 2024
6592e7b
Fix roadmap meta tag
KludgeKML Jul 17, 2024
1544ede
Update personalisation method calls to new names
KludgeKML Jun 6, 2024
9b50e9f
Install RSpec and init
KludgeKML Jun 24, 2024
0a3c4ab
Update spec_helper
KludgeKML Jul 18, 2024
a0f14fd
Update CI, Rake to use RSpec
KludgeKML Jun 12, 2024
dc1b989
Move pact helper
KludgeKML Jun 25, 2024
876deaa
Convert component tests
KludgeKML Jun 25, 2024
f155708
Convert account_home integration tests
KludgeKML Jun 25, 2024
aa1e4e2
Convert bank_holiday integration tests
KludgeKML Jun 26, 2024
1083a0f
Convert csv_preview integration tests
KludgeKML Jul 16, 2024
0d1a90c
Convert electoral_look_up integration tests
KludgeKML Jul 16, 2024
990a9b3
Convert error_handling integration tests
KludgeKML Jun 25, 2024
1b7492f
Convert find_local_council integration tests
KludgeKML Jul 16, 2024
e46460f
Convert help integration tests
KludgeKML Jun 24, 2024
694f267
Convert help_cookies integration tests
KludgeKML Jun 24, 2024
257225e
Convert homepage integration tests
KludgeKML Jun 24, 2024
589a51d
Convert icalendar integration tests
KludgeKML Jun 24, 2024
546a0ca
Convert json integration tests
KludgeKML Jun 24, 2024
5991dae
Convert licence_transaction integration tests
KludgeKML Jul 16, 2024
199cfc7
Convert local_transactions integration tests
KludgeKML Jul 16, 2024
a6781a1
Convert place integration tests
KludgeKML Jun 24, 2024
b3378ac
Convert sessions integration tests
KludgeKML Jun 24, 2024
0a36584
Convert simple_smart_answers integration tests
KludgeKML Jul 16, 2024
026da54
Convert transaction integration tests
KludgeKML Jun 26, 2024
f1aa5a3
Convert travel_advice_atom integration tests
KludgeKML Jun 24, 2024
e536150
Convert travel_advice integration tests
KludgeKML Jun 24, 2024
6b7f6b0
Convert when_do_the_clocks_change integration tests
KludgeKML Jun 26, 2024
32c1ea0
Convert calendar_controllers functional tests
KludgeKML Jun 26, 2024
d69a474
Convert electoral_controller functional tests
KludgeKML Jun 26, 2024
4d9f00a
Convert find_local_council_controller functional tests
KludgeKML Jun 24, 2024
f21e641
Convert help_controller functional tests
KludgeKML Jun 24, 2024
8b9aba1
Convert homepage_controller functional tests
KludgeKML Jul 23, 2024
b3d5438
Convert licence_transaction_controller functional tests
KludgeKML Jun 24, 2024
21ef040
Convert local_transaction_controller functional tests
KludgeKML Jun 26, 2024
e1bf6d0
Convert place_controller functional tests
KludgeKML Jun 24, 2024
7502b3b
Convert placeholder_controller functional tests
KludgeKML Jun 24, 2024
99e56e2
Convert random_controller functional tests
KludgeKML Jun 24, 2024
69309c9
Convert roadmap_controller functional tests
KludgeKML Jun 24, 2024
edafffe
Convert session_controller functional tests
KludgeKML Jun 24, 2024
e67028d
Convert simple_smart_answers_controller functional tests
KludgeKML Jun 24, 2024
0ec12fe
Convert transaction_controller functional tests
KludgeKML Jun 24, 2024
357d856
Convert travel_advice_controller functional tests
KludgeKML Jun 24, 2024
5d0da1f
Convert simple_smart_answers routing tests
KludgeKML Jun 24, 2024
7f239ea
Convert application_helper unit tests
KludgeKML Jun 26, 2024
ec0d357
Convert currency_helper unit tests
KludgeKML Jul 24, 2024
380fe7c
Convert error_items_helper unit tests
KludgeKML Jun 26, 2024
8b1c743
Convert phone_number_helper unit tests
KludgeKML Jun 26, 2024
3fc99d6
Convert recruitment_banner_helper unit tests
KludgeKML Jun 26, 2024
f5995a3
Convert election_postcode model unit tests
KludgeKML Jun 26, 2024
90e398d
Convert location_error model unit tests
KludgeKML Jun 26, 2024
71fe74a
Convert uprn model unit tests
KludgeKML Jun 26, 2024
8c92fda
Convert calendar_content_item presenter unit tests
KludgeKML Jun 26, 2024
8bad6b6
Convert content_item presenter unit tests
KludgeKML Jun 26, 2024
9f9b20a
Convert electoral presenter unit tests
KludgeKML Jun 26, 2024
d4c857d
Convert faq presenter unit tests
KludgeKML Jun 26, 2024
ed3466a
Convert licence_details presenter unit tests
KludgeKML Jun 26, 2024
4e33d4b
Convert licence_transaction presenter unit tests
KludgeKML Jul 24, 2024
63b3307
Convert local_authority presenter unit tests
KludgeKML Jun 26, 2024
19f124e
Convert local_transaction presenter unit tests
KludgeKML Jun 26, 2024
eb26771
Convert place presenter unit tests
KludgeKML Jun 26, 2024
e43a10f
Convert simple_smart_answer presenter unit tests
KludgeKML Jun 26, 2024
2d1b947
Convert transaction presenter unit tests
KludgeKML Jun 26, 2024
97ad358
Convert travel_advice_index presenter unit tests
KludgeKML Jun 26, 2024
306355b
Convert all service unit tests
KludgeKML Jun 24, 2024
c2c8645
Convert all simple_smart_answer unit tests
KludgeKML Jul 24, 2024
5b07f63
Convert api_error_routing_constraint unit tests
KludgeKML Jun 26, 2024
944100f
Convert bank_holiday_generator unit tests
KludgeKML Jun 26, 2024
530d057
Convert calendar unit tests
KludgeKML Jun 26, 2024
de8608b
Convert format_routing_constraint unit tests
KludgeKML Jun 26, 2024
11664be
Convert ics_renderer unit tests
KludgeKML Jun 26, 2024
cd6f400
Convert locales_validation unit tests
KludgeKML Jun 26, 2024
9a7a586
Convert postcode sanitizer unit tests
KludgeKML Jun 26, 2024
f050d28
Remove obsolete minitest files
KludgeKML Jun 26, 2024
aaef2fc
Remove obsolete code
KludgeKML Jul 9, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ jobs:
dependency-review:
name: Dependency Review scan
uses: alphagov/govuk-infrastructure/.github/workflows/dependency-review.yml@main

security-analysis:
name: Security Analysis
uses: alphagov/govuk-infrastructure/.github/workflows/brakeman.yml@main
Expand Down Expand Up @@ -54,7 +54,7 @@ jobs:

test-ruby:
name: Test Ruby
uses: ./.github/workflows/minitest.yml
uses: ./.github/workflows/rspec.yml

pact-tests:
name: Run Pact tests
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
name: Run Minitest
name: Run RSpec

on:
workflow_call:
Expand All @@ -15,7 +15,7 @@ on:

jobs:
run-minitest:
name: Run Minitest
name: Run RSpec
runs-on: ubuntu-latest
steps:
- name: Checkout repository
Expand All @@ -42,8 +42,8 @@ jobs:
- name: Precompile assets
uses: alphagov/govuk-infrastructure/.github/actions/precompile-rails-assets@main

- name: Run Minitest
- name: Run RSpec
env:
RAILS_ENV: test
GOVUK_CONTENT_SCHEMAS_PATH: vendor/publishing-api/content_schemas
run: bundle exec rake test
run: bundle exec rake spec
1 change: 1 addition & 0 deletions .rspec
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
--require spec_helper
4 changes: 2 additions & 2 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,11 @@ group :development, :test do
end

group :test do
gem "capybara"
gem "govuk_schemas"
gem "i18n-coverage"
gem "mocha"
KludgeKML marked this conversation as resolved.
Show resolved Hide resolved
gem "rails-controller-testing"
gem "shoulda-context"
gem "rspec-rails"
gem "simplecov"
gem "timecop"
gem "webmock"
Expand Down
14 changes: 10 additions & 4 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -591,6 +591,14 @@ GEM
rspec-mocks (3.12.6)
diff-lcs (>= 1.2.0, < 2.0)
rspec-support (~> 3.12.0)
rspec-rails (6.1.1)
actionpack (>= 6.1)
activesupport (>= 6.1)
railties (>= 6.1)
rspec-core (~> 3.12)
rspec-expectations (~> 3.12)
rspec-mocks (~> 3.12)
rspec-support (~> 3.12)
rspec-support (3.12.1)
rubocop (1.64.1)
json (~> 2.3)
Expand Down Expand Up @@ -624,7 +632,6 @@ GEM
rubocop-rspec (3.0.1)
rubocop (~> 1.61)
ruby-progressbar (1.13.0)
ruby2_keywords (0.0.5)
rubyzip (2.3.2)
sass-embedded (1.69.7)
google-protobuf (~> 3.25)
Expand All @@ -640,7 +647,6 @@ GEM
sentry-ruby (5.18.1)
bigdecimal
concurrent-ruby (~> 1.0, >= 1.0.2)
shoulda-context (2.0.0)
simplecov (0.22.0)
docile (~> 1.1)
simplecov-html (~> 0.11)
Expand Down Expand Up @@ -703,6 +709,7 @@ DEPENDENCIES
better_errors
binding_of_caller
bootsnap
capybara
climate_control
dalli
dartsass-rails
Expand All @@ -718,7 +725,6 @@ DEPENDENCIES
i18n-coverage
invalid_utf8_rejector
listen
mocha
pact
pact_broker-client
plek
Expand All @@ -727,8 +733,8 @@ DEPENDENCIES
rails-controller-testing
rails-i18n
rails_translation_manager
rspec-rails
rubocop-govuk
shoulda-context
simplecov
slimmer
sprockets-rails
Expand Down
13 changes: 4 additions & 9 deletions Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,15 @@

begin
require "pact/tasks"
require "rspec/core/rake_task"
RSpec::Core::RakeTask.new
rescue LoadError
# Pact isn't available in all environments
# Pact/RSpec not available in all environments
end

require File.expand_path("config/application", __dir__)
require "rake/testtask"

Rails.application.load_tasks

Rake::TestTask.new(:test_unit) do |t|
t.libs << "test"
t.test_files = FileList["test/**/*_test.rb"]
t.warning = false
end

Rake::Task[:default].clear if Rake::Task.task_defined?(:default)
task default: %i[test_unit jasmine pact:verify lint]
task default: %i[lint spec jasmine pact:verify]
KludgeKML marked this conversation as resolved.
Show resolved Hide resolved
2 changes: 1 addition & 1 deletion app/controllers/account_home_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@ class AccountHomeController < ApplicationController
include GovukPersonalisation::ControllerConcern

def show
redirect_with_analytics GovukPersonalisation::Urls.your_account
redirect_with_analytics GovukPersonalisation::Urls.one_login_your_services
KludgeKML marked this conversation as resolved.
Show resolved Hide resolved
end
end
4 changes: 0 additions & 4 deletions app/controllers/concerns/previewable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,4 @@ module Previewable
def set_edition_for_viewing_draft_content
@edition = params[:edition]
end

def viewing_draft_content?
params.include?("edition")
end
end
2 changes: 1 addition & 1 deletion app/controllers/sessions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ def create
redirect_path = http_referer_path
redirect_path = nil unless is_valid_redirect_path? redirect_path

return redirect_with_analytics GovukPersonalisation::Urls.your_account unless redirect_path
return redirect_with_analytics GovukPersonalisation::Urls.one_login_your_services unless redirect_path

redirect_with_analytics GdsApi.account_api.get_sign_in_url(redirect_path:)["auth_uri"]
end
Expand Down
7 changes: 0 additions & 7 deletions app/helpers/links_helper.rb

This file was deleted.

2 changes: 1 addition & 1 deletion app/models/calendar.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
class Calendar
REPOSITORY_PATH = Rails.env.test? ? "test/fixtures" : "lib/data"
REPOSITORY_PATH = "lib/data".freeze
KludgeKML marked this conversation as resolved.
Show resolved Hide resolved

class CalendarNotFound < StandardError
end
Expand Down
2 changes: 1 addition & 1 deletion app/presenters/licence_details_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ def offered_by_county?
end

def single_licence_authority_present?
licence_authority_specific? && authority
licence_authority_specific? && authority.present?
end

def authority
Expand Down
2 changes: 1 addition & 1 deletion app/views/roadmap/index.html.erb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<% content_for :title, t('roadmap.page_title') %>
<% content_for :extra_headers do %>
<meta name="description" content="<%= t('roadmap.hero.description') %>" />
<meta name="twitter:card" content="summary"></meta>
<meta name="twitter:card" content="summary" />
<meta property="og:url" content="https://www.gov.uk/roadmap" />
<meta property="og:title" content="Roadmap - GOV.UK" />
<meta property="og:description" content="<%= t('roadmap.hero.description') %>" />
Expand Down
48 changes: 48 additions & 0 deletions spec/components/all_components_spec.rb
KludgeKML marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
RSpec.describe "AllComponents" do
Dir.glob("app/views/components/*.erb").each do |filename|
template = filename.split("/").last
component_name = template.sub("_", "").sub(".html", "").sub(".erb", "").gsub("-", "_")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great, though I had to look up the difference between sub and gsub 😅.

I understand that these tests are copied almost verbatim from the old minitest tests, but something to consider is whether the last clause (the gsub) needed? I'd expect that we'd want all of the component names to only contain _ not -?

The component name is already being dasherised in tests further down this file, which is what the gsub here is doing.
It's also having to be undone in it "has a correctly named spec file" do so that the file name can be checked.

I think the tests would be easier to maintain if only the tests that need the component name dasherised do that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's because the component names are loaded from their partials, so the sub removes the first _, then the gsub at the end converts the rest of the name from kebab case to snake case. I think for the moment we should keep this (because it matches the corresponding code in govuk_publishing_components), but we should look later at making all of these shared examples that are loaded directly from the components gem.


describe(component_name) do
yaml_file = "#{__dir__}/../../app/views/components/docs/#{component_name}.yml"

it "is documented" do
expect(File).to exist(yaml_file)
end

it "has the correct documentation" do
yaml = YAML.safe_load_file(yaml_file, permitted_classes: [Time])
KludgeKML marked this conversation as resolved.
Show resolved Hide resolved
expect(yaml["name"]).not_to be_empty
expect(yaml["description"]).not_to be_empty
expect(yaml["examples"]).not_to be_empty
expect((yaml["accessibility_criteria"] or yaml["shared_accessibility_criteria"])).to be_truthy
end

it "has the correct class in the ERB template" do
erb = File.read(filename)
class_name = "app-c-#{component_name.dasherize}"
expect(erb).to include(class_name)
end

it "has a correctly named template file" do
template_file = "#{__dir__}/../../app/views/components/_#{component_name}.html.erb"
expect(File).to exist(template_file)
end

it "has a correctly named spec file" do
rspec_file = "#{__dir__}/../../spec/components/#{component_name.tr('-', '_')}_spec.rb"
expect(File).to exist(rspec_file)
end

it "has a correctly named SCSS file" do
css_file = "#{__dir__}/../../app/assets/stylesheets/components/_#{component_name.tr('_', '-')}.scss"
expect(File).to exist(css_file)
end

it "does not use `html_safe` in it's partial" do
KludgeKML marked this conversation as resolved.
Show resolved Hide resolved
file = File.read(filename)
expect(file).not_to include("html_safe")
end
end
end
end
47 changes: 47 additions & 0 deletions spec/components/calendar_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
RSpec.describe "CalendarComponent", type: :view do
def component_name
"calendar"
end

it "renders the basic component" do
render_component({})

expect(rendered).to have_css(".app-c-calendar")
end

it "renders the component with correct data passed" do
render_component(
title: "Marvel films",
year: "2008 onwards",
headings: [
{ text: "Date" },
{ text: "Day of the week" },
{ text: "Film" },
],
events: [
{
title: "Iron Man",
date: Date.parse("2-5-2008"),
notes: "first film in the MCU",
},
{
title: "The Incredible Hulk",
date: Date.parse("13-6-2008"),
notes: "",
},
],
)

expect(rendered).to have_css(".govuk-table__caption .govuk-visually-hidden", text: "Marvel films")
expect(rendered).to have_css(".govuk-table__caption", text: "Marvel films 2008 onwards")
expect(rendered).to have_css(".govuk-table__head .govuk-table__header:nth-child(1)", text: "Date")
expect(rendered).to have_css(".govuk-table__head .govuk-table__header:nth-child(2)", text: "Day of the week")
expect(rendered).to have_css(".govuk-table__head .govuk-table__header:nth-child(3)", text: "Film")
expect(rendered).to have_css(".govuk-table__row:nth-child(1) .govuk-table__header", text: "2 May")
expect(rendered).to have_css(".govuk-table__row:nth-child(1) .govuk-table__cell:nth-child(2)", text: "Friday")
expect(rendered).to have_css(".govuk-table__row:nth-child(1) .govuk-table__cell:nth-child(3)", text: "Iron Man (first film in the mcu)")
expect(rendered).to have_css(".govuk-table__row:nth-child(2) .govuk-table__header", text: "13 June")
expect(rendered).to have_css(".govuk-table__row:nth-child(2) .govuk-table__cell:nth-child(2)", text: "Friday")
expect(rendered).to have_css(".govuk-table__row:nth-child(2) .govuk-table__cell:nth-child(3)", text: "The Incredible Hulk")
end
end
34 changes: 34 additions & 0 deletions spec/components/subscribe_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
RSpec.describe "SubscribeComponent", type: :view do
def component_name
"subscribe"
end

it "fails to render when no parameters given" do
expect { render_component({}) }.to raise_error(ActionView::Template::Error)
end

it "renders the component when link data is passed" do
render_component(
label: "label",
url: "https://www.gov.uk",
title: "title",
)

expect(rendered).to have_css(".app-c-subscribe a[href='https://www.gov.uk'][title='title']", text: "label")
end

it "renders the component with data attributes" do
render_component(
label: "label",
url: "https://www.gov.uk",
title: "title",
data: {
module: "test-module",
ok: "go",
},
)

expect(rendered).to have_css(".app-c-subscribe a[href='https://www.gov.uk'][title='title']", text: "label")
expect(rendered).to have_css(".app-c-subscribe a[data-module='test-module'][data-ok='go']")
end
end
File renamed without changes.
File renamed without changes.
51 changes: 51 additions & 0 deletions spec/helpers/application_helper_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
RSpec.describe ApplicationHelper do
include ContentStoreHelpers

def dummy_publication
ContentItemPresenter.new(content_store_has_random_item(base_path: "/dummy"))
end

describe "#page_title" do
it "doesn't contain consecutive pipes" do
expect(page_title(dummy_publication)).not_to match(/\|\s*\|/)
end

it "doesn't fail if the publication titles are nil" do
publication = OpenStruct.new(title: nil)

expect(page_title(publication)).to be_truthy
end
end

describe "#wrapper_class" do
it "marks local transactions as a service" do
local_transaction = OpenStruct.new(format: "local_transaction")

expect(wrapper_class(local_transaction).split.include?("service")).to be true
end
end

it "builds title from content items" do
publication = OpenStruct.new(title: "Title")

expect(page_title(publication)).to eq("Title - GOV.UK")
end

it "omits first part of title if publication is omitted" do
expect(page_title).to eq("GOV.UK")
end

describe "#current_path_without_query_string" do
it "returns the path of the current request" do
allow(self).to receive(:request).and_return(ActionDispatch::TestRequest.new("PATH_INFO" => "/foo/bar"))

expect(current_path_without_query_string).to eq("/foo/bar")
end

it "returns the path of the current request stripping off any query string parameters" do
allow(self).to receive(:request).and_return(ActionDispatch::TestRequest.new("PATH_INFO" => "/foo/bar"))

expect(current_path_without_query_string).to eq("/foo/bar")
end
end
end
Loading