[go: up one dir, main page]

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 endpoint shared example to default to use_second_scope: true
  • Add explicit use_second_scope: false to all existing tests that were not specifying this parameter
  • Remove the now-redundant use_second_scope: true from tests that were already specifying it
  • Add a user-friendly error message when tests don't provide a request_with_second_scope method

Why this approach: This is an intermediate step that:

  1. Prevents regressions: New tests will be required to implement second scope testing by default
  2. Maintains compatibility: Existing tests continue to work with explicit opt-out
  3. Enables gradual migration: Teams can incrementally add proper second scope testing to their existing specs
  4. 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

  1. Verify that tests with use_second_scope: false still work:

    bundle exec rspec spec/requests/api/projects_spec.rb -e "rate limited endpoint"
  2. Test that new tests without explicit use_second_scope parameter 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
  3. 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
Edited by Pedro Pombeiro

Merge request reports

Loading