## Summary The `ee/spec/requests/api/internal/shellhorse_spec.rb` test file has several structural issues that make it hard to understand, maintain, and extend. ## Problems ### 1. Unrealistic test combinations The spec generates tests for all 4 combinations of caller × protocol: - Shell + SSH ✅ (realistic) - Shell + HTTP ❌ (GitLab Shell only handles SSH) - Workhorse + SSH ❌ (Workhorse only handles HTTP) - Workhorse + HTTP ✅ (realistic) Half the generated tests represent scenarios that never happen in production. The `shell` vs `horse` distinction only changes the authentication header, and `ssh` vs `http` is just a string param. The actual endpoint behavior is identical across all 4. ### 2. Unnecessary shared example nesting - `'logs single streaming audit event'` is only used once (inside `'logs streaming audit events'`). It doesn't need to be a shared example. - `'logs streaming audit events'` does very little - includes a context, adds a developer, and delegates. Could be a plain `context` block. - `'break response in several invalid cases'` is reasonable as a shared example since it's called multiple times, but the nesting with the shared context makes it hard to trace what params are actually being used. ### 3. Only one actor type is tested All tests use the same actor: a human user with a regular SSH key (`create(:key, user: user)`). There is no coverage for: - Deploy keys (with human user or ghost user) - Deploy tokens This gap contributed to the production issue in #591572 where ~6.6M daily 404 errors went undetected. ### 4. Hidden params across abstractions To understand what a single test does, you need to jump between: - The shared context (`'with git audit event env'`) for `header` and `valid_params` - The shared example that includes it for `action` and `packfile_stats` - The parameterized table for the actual values ## Proposal 1. **Remove unrealistic combinations**: Only test Shell+SSH and Workhorse+HTTP 2. **Inline shared examples that are used once**: Replace `'logs single streaming audit event'` and `'logs streaming audit events'` with plain context blocks 3. **Add actor type coverage**: Add tests for deploy keys and deploy tokens 4. **Make params visible**: Keep test setup close to the test assertions so each test is self-contained ## Related - #591572 - The production issue that exposed the test coverage gap