Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,17 @@ gem 'kramdown-parser-gfm', '~> 1.0'
gem 'jekyll-redirect-from'
gem 'jekyll-sitemap'

group :development, :test do
gem 'pry-byebug'
end

group :test do
gem 'capybara'
gem 'html-proofer', '~> 4.0'
gem 'nokogiri', '>= 1.10.5'
gem 'rackup' # required for `Capybara.server = :webrick`
gem 'rack-jekyll'
gem 'rspec'
gem 'rspec_junit_formatter', require: false
gem 'nokogiri', '>= 1.10.5'
gem 'selenium-webdriver'
end
50 changes: 49 additions & 1 deletion Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,19 @@ GEM
specs:
addressable (2.8.7)
public_suffix (>= 2.0.2, < 7.0)
base64 (0.3.0)
bigdecimal (3.1.8)
byebug (12.0.0)
capybara (3.40.0)
addressable
matrix
mini_mime (>= 0.1.3)
nokogiri (~> 1.11)
rack (>= 1.6.0)
rack-test (>= 0.6.3)
regexp_parser (>= 1.5, < 3.0)
xpath (~> 3.2)
coderay (1.1.3)
colorator (1.1.0)
concurrent-ruby (1.3.4)
diff-lcs (1.5.1)
Expand Down Expand Up @@ -62,21 +74,42 @@ GEM
listen (3.9.0)
rb-fsevent (~> 0.10, >= 0.10.3)
rb-inotify (~> 0.9, >= 0.9.10)
logger (1.7.0)
matrix (0.4.2)
mercenary (0.4.0)
method_source (1.1.0)
mini_mime (1.1.5)
mini_portile2 (2.8.9)
nokogiri (1.18.8)
mini_portile2 (~> 2.8.2)
racc (~> 1.4)
parallel (1.26.3)
pathutil (0.16.2)
forwardable-extended (~> 2.6)
pry (0.15.2)
coderay (~> 1.1)
method_source (~> 1.0)
pry-byebug (3.11.0)
byebug (~> 12.0)
pry (>= 0.13, < 0.16)
public_suffix (6.0.1)
racc (1.8.1)
rack (1.6.13)
rack-jekyll (0.5.0)
jekyll (>= 1.3)
listen (>= 1.3)
rack (~> 1.5)
rack-test (2.2.0)
rack (>= 1.3)
rackup (1.0.1)
rack (< 3)
webrick
rainbow (3.1.1)
rake (13.2.1)
rb-fsevent (0.11.2)
rb-inotify (0.11.1)
ffi (~> 1.0)
regexp_parser (2.10.0)
rexml (3.3.9)
rouge (4.4.0)
rspec (3.13.0)
Expand All @@ -94,35 +127,50 @@ GEM
rspec-support (3.13.1)
rspec_junit_formatter (0.6.0)
rspec-core (>= 2, < 4, != 2.12.0)
rubyzip (2.4.1)
safe_yaml (1.0.5)
sass-embedded (1.80.6)
google-protobuf (~> 4.28)
rake (>= 13)
selenium-webdriver (4.33.0)
base64 (~> 0.2)
logger (~> 1.4)
rexml (~> 3.2, >= 3.2.5)
rubyzip (>= 1.2.2, < 3.0)
websocket (~> 1.0)
terminal-table (3.0.2)
unicode-display_width (>= 1.1.1, < 3)
typhoeus (1.4.1)
ethon (>= 0.9.0)
unicode-display_width (2.6.0)
webrick (1.9.0)
websocket (1.2.11)
xpath (3.2.0)
nokogiri (~> 1.8)
yell (2.2.2)
zeitwerk (2.7.1)

PLATFORMS
ruby

DEPENDENCIES
capybara
html-proofer (~> 4.0)
jekyll (~> 4.3.0)
jekyll-redirect-from
jekyll-sass-converter (~> 3.0.0)
jekyll-sitemap
kramdown-parser-gfm (~> 1.0)
nokogiri (>= 1.10.5)
pry-byebug
rack-jekyll
rackup
rspec
rspec_junit_formatter
selenium-webdriver

RUBY VERSION
ruby 3.2.5p208

BUNDLED WITH
2.5.9
2.6.9
1 change: 1 addition & 0 deletions _layouts/base.html
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ <h1 class="usa-display">{{ page.title }}</h1>
<script src="{{ site.baseurl }}/assets/js/toggle_view.js"></script>
<script src="{{ site.baseurl }}/assets/js/main.js"></script>
<script src="{{ site.baseurl }}/assets/js/anchor.js"></script>
<script src="{{ site.baseurl }}/assets/js/anchor_accordion.js"></script>

<script>
anchors.add([
Expand Down
16 changes: 16 additions & 0 deletions assets/js/anchor_accordion.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
(() => {
const onChange = (_event) => {
const urlHash = new URL(document.URL).hash;
const idFromHash = urlHash.match(/^(#[A-Za-z0-9_-]+)$/);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since this came up before I put the code up for review: I deliberately want to keep some input sanitization here. The format I've picked is in line with the general format we use for class and id attributes, so it feels appropriate to me.

if (idFromHash) {
const accordionHeading = document.querySelector(
`${idFromHash[1]}.usa-accordion__heading > button`,
);
if (accordionHeading) {
accordionHeading.click();
}
}
};
document.addEventListener('DOMContentLoaded', onChange);
window.addEventListener('hashchange', onChange);
})();
9 changes: 9 additions & 0 deletions spec/capybara_helper.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
require 'spec_helper'
require 'capybara/rspec'
require 'rack/jekyll'
require 'selenium-webdriver'
require 'yaml'

Capybara.javascript_driver = :selenium_chrome_headless
Capybara.app = Rack::Jekyll.new(force_build: true)
Capybara.server = :webrick
26 changes: 26 additions & 0 deletions spec/integration/accordion_js_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
require 'capybara_helper'
require 'pry-byebug'

RSpec.describe 'accordions on /oidc/authorization/', :js, type: :feature do
Copy link
Contributor Author

@h-m-m h-m-m Jun 6, 2025

Choose a reason for hiding this comment

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

I'd still prefer actual JS tests instead of or in addition to these Capybara tests. That said, I agree with Jack Cody that this was likely the easiest testing to set up.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure actual JS tests are beneficial when the only output is DOM manipulation. If you had a function like getSanitizedHash, you could test that, but as-is this tests everything you can.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do agree that the coverage here is good.

Compared to some JS testing I've done in the past, doing it this way does feels to me like wearing thick, knitted gloves on my fingers when I'm not used to wearing them.

My main concern is that it feels too easy to accidentally break something here in a way that would generate a JS error that this test won't notice. Detecting a JS error like that in a headless browser with Capybara is tricky and fragile. I'm particularly concerned with breaking unrelated behavior on a page where this JS is included but the page doesn't have an accordion (or doesn't have one anymore).

it 'is not expanded by default' do
visit '/oidc/authorization'
expect(page).to have_element('dt')
expect(page).to have_no_element('dd')
accordion_button = find('#service_level button')
expect(accordion_button.native.attribute('aria-expanded')).to eq('false')
end

it 'is expanded with an anchor in the URL' do
visit '/oidc/authorization#service_level'
expect(page).to have_element('dt')
expect(page).to have_element('dd')
definition = find('#service_level ~ dd').text
expect(definition).to include('acr.login.gov:verified')
accordion_button = find('#service_level > button')
expect(accordion_button.native.attribute('aria-expanded')).to eq('true')

visit '/oidc/authorization#aal_values'
next_definition = find('#aal_values ~ dd').text
expect(next_definition).to include('http://idmanagement.gov/ns/assurance/aal/2')
end
end