-
Notifications
You must be signed in to change notification settings - Fork 39
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
Logout tests. #666
Logout tests. #666
Conversation
@jmax-fearless , please update the commit message to include the ticket # |
@MothOnMars - Updated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few changes. For clarity, the full spec
with the changes I'm suggesting would look like:
describe UserSessionsController do
before { activate_authlogic }
# remove additional `before { activate_authlogic }` blocks
...
describe '#destroy' do
subject(:destroy) { delete :destroy }
let(:login_uri) { "#{request.protocol}#{request.host_with_port}/login" }
let(:id_token) { 'fake_id_token' }
let(:expected_login_dot_gov_logout_uri) do
URI::HTTPS.build(
host: 'idp.int.identitysandbox.gov',
path: '/openid_connect/logout',
query: {
id_token_hint: id_token,
post_logout_redirect_uri: login_uri,
state: '1234567890123456789012'
}.to_query
).to_s
end
before { session[:id_token] = id_token }
include_context 'approved user logged in'
it { is_expected.to redirect_to(expected_login_dot_gov_logout_uri) }
end
end
@@ -22,4 +26,38 @@ | |||
it { is_expected.to redirect_to(account_path) } | |||
end | |||
end | |||
|
|||
describe 'logging out' do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use the method in the description: describe '#destroy'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed description to use the method name.
@MothOnMars - All comments have been addressed. Please re-review at your convenience. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. As changes are test-only, you may merge without further ado.
Functional SRCH-1772 validate RoutedQuery#url (#670) SRCH-1915 fix Tweet.expire method (#671) Spec/Test only updates SRCH-1926 update CircleCi config to use updated ruby 2.5.5 image (#665) SRCH-1873 install and configure the searchgov_style gem (#667) SRCH-1552 address cucumber logout tests. (#666) SRCH-1982 merge "IgnoredMethods" in Rubocop config (#673)
The RuboCop comment in
user_sessions_controller_spec.rb
could be replaced by an exclusion in the appropriate config file, since it's disabling the ScatteredSetup cop for the whole file. I judged it better to use the pseudo-comment facility, because the exclusion results from what the specs are doing, rather than from the type (system, controller, etc.) of the spec.