WIP: test: refactored sign_in method in authentication_helper.rb to set set session variables#7962
Open
YheChen wants to merge 5 commits into
Open
WIP: test: refactored sign_in method in authentication_helper.rb to set set session variables#7962YheChen wants to merge 5 commits into
YheChen wants to merge 5 commits into
Conversation
cdd56c3 to
056cd42
Compare
Collaborator
Coverage Report for CI Build 26303551501Coverage remained the same at 90.225%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats💛 - Coveralls |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed Changes
(Describe your changes here. Also describe the motivation for your changes: what problem do they solve, or how do they improve the application or codebase? If this pull request fixes an open issue, use a keyword to link this pull request to the issue.)
Refactored
AuthenticationHelper#sign_in(inspec/support/authentication_helper.rb) to set the relevant session values directly instead of dispatching a realPOST /loginrequest toMainController#login.Motivation:
sign_inis a test helper whose responsibility is to simulate an authenticated session so that other controller tests can exercise their own controller's behavior. It is not responsible for verifying thatMainController#loginworks —MainControllerhas its own spec for that. Routing every test login through the realloginaction conflated those two responsibilities and, as a side effect, made every call tosign_ininvokeUser.authenticate, which spawns a subprocess to run the configuredvalidate_filescript. Becausesign_inis called once perget_as/post_as/put_as/patch_as/delete_as, this added per-test overhead to a large portion of the controller suite.The new
sign_inwrites the four session keys thatMainController#loginwould have written on a successful local login:session[:auth_type] = 'local'session[:real_user_name] = user.user_namesession[:timeout] = Settings.session_timeout.seconds.from_now.to_ssession[:has_warned] = falseSubsequent requests in the same example still pass
SessionHandler#authenticate'sbefore_actionchecks becauselogged_in?andsession_expired?only read from these session keys.Also updated
main_controller_spec.rb: 7 tests were asserting on the redirect produced bysign_in's internalpost :login, which is exactly the conflation this refactor exposes. They now callpost :logindirectly. One test (LTI launch) additionally needed its cookie value memoized vialetto avoid a pre-existing locale-asymmetry bug surfaced by the change.Screenshots of your changes (if applicable)
Associated documentation repository pull request (if applicable)
Type of Change
(Write an
Xor a brief description next to the type or types that best describe your changes.)Checklist
(Complete each of the following items for your pull request. Indicate that you have completed an item by changing the
[ ]into a[x]in the raw text, or by clicking on the checkbox in the rendered description on GitHub.)Before opening your pull request:
After opening your pull request:
Questions and Comments
(Include any questions or comments you have regarding your changes.)