Skip to content

Remove redundant N+1 checks in spec

Radamanthus Batnag requested to merge 448409-remove-unnecessary-nplusone-checks into master

🌱 Context

We're running the N+1 check for all "accept" cases in this RSpec table. If I counted correctly, that's 101 runs. And the shared example is called in three places:

These three are already among the top 20 slowest specs. Removing redundant checks will help speed them up a bit.

We don't really need to do the N+1 check for all the permutations in the RSpec table.

If we look at the lib/api/npm_project_packages.rb implementation, we need to check for N+1 in two places:

And if we look at the NpmNamespaceEndpoints concern which is reused by both lib/api/npm_group_packages.rb and lib/api/npm_instance_packages.rb, we also need to check for N+1 in only a few places:

  • ::Packages::Npm::PackagesFinder call here
  • ::Packages::Npm::PackagesForUserFinder call here
  • generate_metadata_service call here

A better place to do the N+1 check is in the specs of the finders above, and in the specs for ::Packages::Npm::GenerateMetadataService.

We already have N+1 specs for ::Packages::Npm::GenerateMetadataService here.

What does this MR do and why?

This MR does two things:

  • removes the flaky N+1 spec in the API request spec (incident)
  • adds the N+1 spec to the specs of the finders used by the request that had the N+1 specs

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Screenshots or screen recordings

This is a spec-only change. No UI changes 🌈

How to set up and validate locally

This is a spec-only change. No behavior changes to test 🌈

Related to #448409

Edited by Radamanthus Batnag

Merge request reports