RSpec: Ensure that finders' `execute` always returns AR relation (!90991) · Merge requests · GitLab.org / GitLab

Peter Leitzen requested to merge pl-rspec-support-finder-exection-relation into master Jun 25, 2022

What does this MR do and why?

This MR ensures that all finders' execute method return ActiveRecord::Relation.

All current offenses are tracked in an allow list YAML which can be used to either exclude finders temporarily or permanently. These entries are not checked.

Closes #298771 (closed)

Failure example

When a finder method execute does not return a ActiveRecord::Relation users see the following example failure:

Failures:

  1) TagsFinder#execute filter only filters tags by name
     Failure/Error:
                 raise <<~MESSAGE
                   #{self.class}#execute returned `#{result.class}` instead of `ActiveRecord::Relation`.
                   All finder classes are expected to return `ActiveRecord::Relation`.

                   Read more at https://docs.gitlab.com/ee/development/reusing_abstractions.html#finders
                 MESSAGE

     RuntimeError:
       TagsFinder#execute returned `Array` instead of `ActiveRecord::Relation`.
       All finder classes are expected to return `ActiveRecord::Relation`.

       Read more at https://docs.gitlab.com/ee/development/reusing_abstractions.html#finders
     # ./spec/support/finder_collection.rb:30:in `execute'
     # ./spec/finders/tags_finder_spec.rb:11:in `load_tags'
     # ./spec/finders/tags_finder_spec.rb:47:in `block (4 levels) in <top (required)>'
     # ./spec/spec_helper.rb:405:in `block (3 levels) in <top (required)>'
     # ./spec/support/sidekiq_middleware.rb:9:in `with_sidekiq_server_middleware'
     # ./spec/spec_helper.rb:396:in `block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:392:in `block (3 levels) in <top (required)>'
     # ./lib/gitlab/application_context.rb:52:in `with_raw_context'
     # ./spec/spec_helper.rb:392:in `block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:261:in `block (2 levels) in <top (required)>'
     # ./spec/support/system_exit_detected.rb:7:in `block (2 levels) in <main>'
     # ./spec/support/database/prevent_cross_joins.rb:106:in `block (3 levels) in <main>'
     # ./spec/support/database/prevent_cross_joins.rb:60:in `with_cross_joins_prevented'
     # ./spec/support/database/prevent_cross_joins.rb:106:in `block (2 levels) in <main>'

Offenses extracted from pipeline log

Via

ruby ~/devel/gitlab/gitlab-tools/bin/ci_logs gitlab-org/gitlab 573031599 rspec | grep -E "^\S+#execute returned" | sort | uniq > .local/finder_collection.txt

AccessRequestsFinder#execute returned `Array` instead of `ActiveRecord::Relation`
Admin::PlansFinder#execute returned `NilClass` instead of `ActiveRecord::Relation`
Admin::PlansFinder#execute returned `Plan` instead of `ActiveRecord::Relation`
Analytics::CycleAnalytics::StageFinder#execute returned `Analytics::CycleAnalytics::GroupStage` instead of `ActiveRecord::Relation`
Analytics::CycleAnalytics::StageFinder#execute returned `Analytics::CycleAnalytics::ProjectStage` instead of `ActiveRecord::Relation`
ApplicationsFinder#execute returned `Doorkeeper::Application` instead of `ActiveRecord::Relation`
ApplicationsFinder#execute returned `NilClass` instead of `ActiveRecord::Relation`
Autocomplete::GroupFinder#execute returned `Group` instead of `ActiveRecord::Relation`
Autocomplete::GroupFinder#execute returned `NilClass` instead of `ActiveRecord::Relation`
Autocomplete::ProjectFinder#execute returned `NilClass` instead of `ActiveRecord::Relation`
Autocomplete::ProjectFinder#execute returned `Project` instead of `ActiveRecord::Relation`
Autocomplete::UsersFinder#execute returned `Array` instead of `ActiveRecord::Relation`
BilledUsersFinder#execute returned `Hash` instead of `ActiveRecord::Relation`
Boards::BoardsFinder#execute returned `Array` instead of `ActiveRecord::Relation`
Boards::VisitsFinder#execute returned `NilClass` instead of `ActiveRecord::Relation`
BranchesFinder#execute returned `Array` instead of `ActiveRecord::Relation`
Ci::AuthJobFinder#execute returned `Ci::Build` instead of `ActiveRecord::Relation`
Ci::AuthJobFinder#execute returned `NilClass` instead of `ActiveRecord::Relation`
Ci::CommitStatusesFinder#execute returned `Hash` instead of `ActiveRecord::Relation`
Ci::DailyBuildGroupReportResultsFinder#execute returned `Array` instead of `ActiveRecord::Relation`
ClusterAncestorsFinder#execute returned `Array` instead of `ActiveRecord::Relation`
Clusters::AgentAuthorizationsFinder#execute returned `Array` instead of `ActiveRecord::Relation`
Clusters::KubernetesNamespaceFinder#execute returned `Clusters::KubernetesNamespace` instead of `ActiveRecord::Relation`
Clusters::KubernetesNamespaceFinder#execute returned `NilClass` instead of `ActiveRecord::Relation`
ComplianceManagement::MergeRequests::ComplianceViolationsFinder#execute returned `NilClass` instead of `ActiveRecord::Relation`
ContainerRepositoriesFinder#execute returned `NilClass` instead of `ActiveRecord::Relation`
ContextCommitsFinder#execute returned `Array` instead of `ActiveRecord::Relation`
ContextCommitsFinder#execute returned `CommitCollection` instead of `ActiveRecord::Relation`
Environments::EnvironmentNamesFinder#execute returned `Array` instead of `ActiveRecord::Relation`
Environments::EnvironmentsByDeploymentsFinder#execute returned `Array` instead of `ActiveRecord::Relation`
EventsFinder#execute returned `Kaminari::PaginatableArray` instead of `ActiveRecord::Relation`
GroupDescendantsFinder#execute returned `Kaminari::PaginatableArray` instead of `ActiveRecord::Relation`
Groups::ProjectsRequiringAuthorizationsRefresh::OnDirectMembershipFinder#execute returned `Array` instead of `ActiveRecord::Relation`
Groups::ProjectsRequiringAuthorizationsRefresh::OnTransferFinder#execute returned `Array` instead of `ActiveRecord::Relation`
KeysFinder#execute returned `DeployKey` instead of `ActiveRecord::Relation`
KeysFinder#execute returned `Key` instead of `ActiveRecord::Relation`
KeysFinder#execute returned `NilClass` instead of `ActiveRecord::Relation`
LfsPointersFinder#execute returned `Array` instead of `ActiveRecord::Relation`
LicenseTemplateFinder#execute returned `Array` instead of `ActiveRecord::Relation`
LicenseTemplateFinder#execute returned `LicenseTemplate` instead of `ActiveRecord::Relation`
MergeRequests::OldestPerCommitFinder#execute returned `Hash` instead of `ActiveRecord::Relation`
NotesFinder#execute returned `NilClass` instead of `ActiveRecord::Relation`
Packages::BuildInfosFinder#execute returned `Array` instead of `ActiveRecord::Relation`
Packages::Conan::PackageFileFinder#execute returned `NilClass` instead of `ActiveRecord::Relation`
Packages::Conan::PackageFileFinder#execute returned `Packages::PackageFile` instead of `ActiveRecord::Relation`
Packages::Go::ModuleFinder#execute returned `NilClass` instead of `ActiveRecord::Relation`
Packages::Go::ModuleFinder#execute returned `Packages::Go::Module` instead of `ActiveRecord::Relation`
Packages::Go::PackageFinder#execute returned `NilClass` instead of `ActiveRecord::Relation`
Packages::Go::PackageFinder#execute returned `Packages::Package` instead of `ActiveRecord::Relation`
Packages::Go::VersionFinder#execute returned `Array` instead of `ActiveRecord::Relation`
Packages::PackageFileFinder#execute returned `NilClass` instead of `ActiveRecord::Relation`
Packages::PackageFileFinder#execute returned `Packages::PackageFile` instead of `ActiveRecord::Relation`
Packages::PackageFinder#execute returned `Packages::Package` instead of `ActiveRecord::Relation`
Packages::Pypi::PackageFinder#execute returned `Packages::Package` instead of `ActiveRecord::Relation`
Projects::Integrations::Jira::ByIdsFinder#execute returned `Hash` instead of `ActiveRecord::Relation`
Projects::Integrations::Jira::ByIdsFinder#execute returned `NilClass` instead of `ActiveRecord::Relation`
Projects::Integrations::Jira::IssuesFinder#execute returned `Array` instead of `ActiveRecord::Relation`
Releases::EvidencePipelineFinder#execute returned `Ci::Pipeline` instead of `ActiveRecord::Relation`
Releases::EvidencePipelineFinder#execute returned `NilClass` instead of `ActiveRecord::Relation`
Repositories::BranchNamesFinder#execute returned `Enumerator::Lazy` instead of `ActiveRecord::Relation`
Repositories::ChangelogTagFinder#execute returned `NilClass` instead of `ActiveRecord::Relation`
Repositories::ChangelogTagFinder#execute returned `RSpec::Mocks::Double` instead of `ActiveRecord::Relation`
Repositories::TreeFinder#execute returned `Array` instead of `ActiveRecord::Relation`
Security::FindingsFinder#execute returned `NilClass` instead of `ActiveRecord::Relation`
Security::FindingsFinder#execute returned `Security::FindingsFinder::ResultSet` instead of `ActiveRecord::Relation`
Security::PipelineVulnerabilitiesFinder#execute returned `Gitlab::Ci::Reports::Security::AggregatedReport` instead of `ActiveRecord::Relation`
Security::ScanExecutionPoliciesFinder#execute returned `Array` instead of `ActiveRecord::Relation`
Security::TrainingProviders::BaseUrlFinder#execute returned `Hash` instead of `ActiveRecord::Relation`
Security::TrainingProviders::BaseUrlFinder#execute returned `NilClass` instead of `ActiveRecord::Relation`
Security::TrainingUrlsFinder#execute returned `Array` instead of `ActiveRecord::Relation`
SentryIssueFinder#execute returned `NilClass` instead of `ActiveRecord::Relation`
SentryIssueFinder#execute returned `SentryIssue` instead of `ActiveRecord::Relation`
ServerlessDomainFinder#execute returned `NilClass` instead of `ActiveRecord::Relation`
ServerlessDomainFinder#execute returned `Serverless::Domain` instead of `ActiveRecord::Relation`
TagsFinder#execute returned `Array` instead of `ActiveRecord::Relation`
TemplateFinder#execute returned `Array` instead of `ActiveRecord::Relation`
TemplateFinder#execute returned `Gitlab::Template::DockerfileTemplate` instead of `ActiveRecord::Relation`
TemplateFinder#execute returned `Gitlab::Template::GitignoreTemplate` instead of `ActiveRecord::Relation`
TemplateFinder#execute returned `Gitlab::Template::GitlabCiYmlTemplate` instead of `ActiveRecord::Relation`
TemplateFinder#execute returned `Gitlab::Template::IssueTemplate` instead of `ActiveRecord::Relation`
TemplateFinder#execute returned `Gitlab::Template::MergeRequestTemplate` instead of `ActiveRecord::Relation`
TemplateFinder#execute returned `Gitlab::Template::MetricsDashboardTemplate` instead of `ActiveRecord::Relation`
TemplateFinder#execute returned `NilClass` instead of `ActiveRecord::Relation`
TemplateFinder#execute returned `OpenStruct` instead of `ActiveRecord::Relation`
UploaderFinder#execute returned `FileUploader` instead of `ActiveRecord::Relation`
UploaderFinder#execute returned `NilClass` instead of `ActiveRecord::Relation`
UserGroupNotificationSettingsFinder#execute returned `Array` instead of `ActiveRecord::Relation`
UserGroupsCounter#execute returned `Hash` instead of `ActiveRecord::Relation`

How to set up and validate locally

bin/rspec $(find spec ee/spec -name "*_finder_spec.rb") ✅  

echo "[]" > spec/support/finder_collection_allowlist.yml

bin/rspec $(find spec ee/spec -name "*_finder_spec.rb") 💥

MR acceptance checklist

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

I have evaluated the MR acceptance checklist for this MR.

Edited Jun 27, 2022 by Peter Leitzen

RSpec: Ensure that finders' `execute` always returns AR relation

What does this MR do and why?

Failure example

How to set up and validate locally

MR acceptance checklist

Merge request reports