Default to require second scope on rate limiter specs
What does this MR do and why?
This MR improves the rate limited endpoint shared RSpec examples to better prevent rate limiting bugs by defaulting to require a second scope in tests. This is an intermediate step toward implementing comprehensive multi-scope testing for all rate-limited endpoints.
Background:
A previous MR (!204751 (merged)) caused an S1 incident because it didn't properly test rate limiting with multiple scopes. The existing rate limited endpoint shared examples only exercised a single scope, making it difficult to catch these types of problems.
Changes made:
- Change the
rate limited endpointshared example to default touse_second_scope: true - Add explicit
use_second_scope: falseto all existing tests that were not specifying this parameter - Remove the now-redundant
use_second_scope: truefrom tests that were already specifying it - Add a user-friendly error message when tests don't provide a
request_with_second_scopemethod
Why this approach: This is an intermediate step that:
- Prevents regressions: New tests will be required to implement second scope testing by default
- Maintains compatibility: Existing tests continue to work with explicit opt-out
- Enables gradual migration: Teams can incrementally add proper second scope testing to their existing specs
- Provides clear guidance: Tests introduced subsequently that need second scope implementation will get helpful error messages
References
Closes #572497 (closed)
Screenshots or screen recordings
This is a testing infrastructure change with no user-facing impact. The improvement is in test coverage and preventing future rate limiting bugs.
How to set up and validate locally
-
Verify that tests with
use_second_scope: falsestill work:bundle exec rspec spec/requests/api/projects_spec.rb -e "rate limited endpoint" -
Test that new tests without explicit
use_second_scopeparameter will require second scope implementation:# This should raise a helpful error message it_behaves_like 'rate limited endpoint', rate_limit_key: :test_key do def request get '/some/path' end # Missing request_with_second_scope method will cause error end -
Run a broader test to ensure no existing functionality is broken:
bundle exec rspec spec/requests/api/ -e "rate limited"
MR acceptance checklist
Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.
-
Functionality: Improves test coverage to prevent rate limiting bugs -
Performance: No performance impact - only affects test execution -
Reliability: Enhances reliability by requiring better test coverage for new code -
Security: Indirectly improves security by preventing rate limiting bypass bugs -
Maintainability: Makes the codebase more maintainable by enforcing better testing practices -
Testing: This change specifically improves our testing infrastructure -
Documentation: The error message provides clear guidance for developers