Skip to content

Improve reliability of WebauthnSetupForm spec#9963

Merged
aduth merged 3 commits intomainfrom
aduth-webauthn-spec-flakiness
Jan 24, 2024
Merged

Improve reliability of WebauthnSetupForm spec#9963
aduth merged 3 commits intomainfrom
aduth-webauthn-spec-flakiness

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented Jan 24, 2024

🛠 Summary of changes

Revises webauthn_setup_form_spec.rb in hopes of reducing some test flakiness.

Example: https://gsa-tts.slack.com/archives/C0NGESUN5/p1706048041505969

The suspected cause is that let(:user) is lazily evaluated after the form is already submitted, which is likely not the intent of the test setup. Eagerly assigning let!(:user) with existing authenticators should hopefully produce a more reliable result.

📜 Testing Plan

Verify the test passes:

rspec spec/forms/webauthn_setup_form_spec.rb

aduth added 2 commits January 24, 2024 08:33
changelog: Internal, Automated Testing, Improve reliability of tests
@aduth aduth requested a review from kevinsmaster5 January 24, 2024 13:36
:user,
webauthn_configurations: create_list(
:webauthn_configuration,
2,
Copy link
Contributor

Choose a reason for hiding this comment

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

If I'm understanding correctly - this creates the 2 configurations at the same time rather than how they were being created before? Using create_list.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, a little trick I learned recently.

Copy link
Contributor

@kevinsmaster5 kevinsmaster5 left a comment

Choose a reason for hiding this comment

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

It looks good!

@aduth aduth merged commit 50cebd6 into main Jan 24, 2024
@aduth aduth deleted the aduth-webauthn-spec-flakiness branch January 24, 2024 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants