Skip to content

Speed up spec/features/users/login spec.rb

Bogdan Denkovych requested to merge speed-up-spec/features/users/login_spec.rb into master

What does this MR do and why?

The problem

We started seeing many test failures in spec/features/users/login_spec.rb on our CI, see #402322 (closed). Since those specs are related to OTP codes, as @jessieay mentioned in !116022 (merged), submitting OTP code in specs takes ~45 seconds:

# in spec

TIME: 2023-03-28 21:13:23 UTC
USER ID: 15
USER OTP SECRET: IH2QUCNO5XCCRL4YO72VBCY2ICEDBUTS
USER OTP: 641912

# in lib/gitlab/auth/otp/strategies/devise.rb

TIME: 2023-03-28 21:14:08 UTC
CURRENT USER ID: 15
CURRENT USER OTP SECRET: IH2QUCNO5XCCRL4YO72VBCY2ICEDBUTS
ENTERED OTP CODE: 641912
CURRENT OTP: 227675

GitLab uses the default value for otp_allowed_drift, which is 30 seconds. If you note the timestamps in both failures above, they are more than 30 seconds apart. As a result, the entered OTP was no longer valid by the time it was being validated.

That made those specs fail.

In !116022 (merged) we prevented those specs from failing by freeze_time time. But we haven't identified the root cause of the slowness.

After the analysis in !116022 (comment 1333215044) I become more confident that the slowness is not caused by changes in the GitLab codebase, but in the feature specs.

I found that in !112212 (merged) we made enter_code method that we use for feature specs slower by 45 seconds on CI and by 10 seconds on the local environment.

We added the condition if page.has_content?("Sign in via 2FA code") to that method. Capybara periodically re-check the predicate/assertion up to the default_max_wait_time defined. On the CI default_max_wait_time is set to 45 seconds. On the local environment it is set to 10 seconds. See https://gitlab.com/gitlab-org/gitlab/-/blob/698239cbc031b11473ba573fd40ff52da0bcc659/spec/support/capybara.rb#L10.

That means if there is not supposed to be "Sign in via 2FA code" content on the page, if page.has_content?("Sign in via 2FA code") condition will take 45 seconds to return false. That was the root cause of the slowness.

The lesson

Make sure to not create conditions based on Capybara predicates in feature specs to DRY code or something since its execution will take default_max_wait_time seconds in cases it is expected to return false.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Bogdan Denkovych

Merge request reports