<!--IssueSummary start--> <details> <summary> Everyone can contribute. [Help move this issue forward](https://handbook.gitlab.com/handbook/marketing/developer-relations/contributor-success/community-contributors-workflows/#contributor-links) while earning points, leveling up and collecting rewards. </summary> - [Close this issue](https://contributors.gitlab.com/manage-issue?action=close&projectId=278964&issueIid=425510) </details> <!--IssueSummary end--> This came up in an import-related MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/131958#note_1564294464 I updated the `validates :"#{type}_access_levels` validation in `app/models/concerns/protected_ref.rb` in the wrong way but no specs failed as far as we could tell. I tried to backfill specs myself but it was getting complex so I decided to open an issue instead. ## Add test coverage for this validation * `validates :"#{type}_access_levels", length: { is: 1, message: "are restricted to a single instance per #{self.model_name.human}." }, unless: -> { allow_multiple?(type) }` is part of the `ProtectedRef` concern, which is included in `ProtectedBranch` and `ProtectedTag`. * Ths validation is overridden in `ee/app/models/concerns/ee/protected_ref.rb` to `validates :"#{type}_access_levels", length: { is: 1 }, if: -> { false }` so that the validation is actually not run in an EE context. * There are no EE-specific models for `ProtectedBranch` and `ProtectedTag` so I am not sure where would make most sense to add unit tests for this differing behavior in EE vs non-EE contexts. If I added a unit test to `spec/models/protected_branch_spec.rb`, it will pass when run with `FOSS_ONLY=1` prepended to the spec run but not otherwise (or vice versa depending on the spec expectation) * Idea: We sometimes have return if/unless Gitlab.ee? in our tests, so a test will only execute in the FOSS-only pipeline, or only in the regular pipeline that “runs as EE”. That might give the control to isolate the test to either EE or FOSS * I opened a Draft MR that changes this validation to never run here: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/132077+ -- this should help determine if we have any indirect test coverage for it ## Update EE validation In the regular `ProtectedRef` concern, we validate that there is exactly 1 `"#{type}_access_levels"`. In the `EE::ProtectedRef` concern, we do not validate the length at all. Shouldn't we update the validation to ensure that there is *at least* 1? It seems like the intention is to allow multiple but it seems odd to me that `ProtectedRef` requires exactly 1 and `EE::ProtectedRef` has no size limitation for this relation.