Clean up the maven API specs code
What does this MR do?
This is a follow up MR after some discussions in !27612 (merged).
This MR tries to clean up the specs code for the Maven API class by:
- mainly having
let_it_bevars and re-use them as much as possible - not using AR requests but use the existing scopes on the models
A similar cleanup has been done for npm, here: !26810 (merged)
Screenshots
TestProf was used to analyze the current state of the spec and identify which factories could be converted to let_it_be.
Before this MR
$ FPROF=1 bundle exec rspec ee/spec/requests/api/maven_packages_spec.rb
[TEST PROF INFO] FactoryProf enabled (simple mode)
Run options: include {:focus=>true}
All examples were filtered out; ignoring {:focus=>true}
==> Setting up Gitaly...
Checking gitaly-ruby Gemfile...
Checking gitaly-ruby bundle...
The Gemfile's dependencies are satisfied
Trying to connect to gitaly: ....... OK
Trying to connect to praefect: ... OK
Gitaly set up in 1.514383 seconds...
==> Setting up GitLab Workhorse...
GitLab Workhorse set up in 0.001272 seconds...
==> Setting up GitLab Elasticsearch Indexer...
GitLab Elasticsearch Indexer set up in 0.000481 seconds...
...............[fog][WARNING] Your region 'gdk' does not match the default region 'us-east-1'
...............[fog][WARNING] Your region 'gdk' does not match the default region 'us-east-1'
.[fog][WARNING] Your region 'gdk' does not match the default region 'us-east-1'
...........[fog][WARNING] Your region 'gdk' does not match the default region 'us-east-1'
.[fog][WARNING] Your region 'gdk' does not match the default region 'us-east-1'
..................
Finished in 36.99 seconds (files took 30.9 seconds to load)
61 examples, 0 failures
[TEST PROF INFO] Factories usage
Total: 591
Total top-level: 242
Total time: 16.8900s
Total uniq factories: 12
total top-level total time top-level time name
117 0 1.5553s 0.0000s package_file
114 61 3.0445s 1.5488s user
101 55 11.4200s 5.1626s project
53 53 1.0345s 1.0345s group
48 0 1.4880s 0.0000s namespace
39 39 7.6868s 7.6868s maven_package
39 0 5.7932s 0.0000s maven_metadatum
39 0 5.6216s 0.0000s package
26 26 0.1379s 0.1379s personal_access_token
7 0 0.9858s 0.0000s ci_pipeline
7 7 1.3090s 1.3090s ci_build
1 1 0.0103s 0.0103s license- 591 calls to factories accounting for 16.89secs out of the 36.99secs of the execution time (~45.66%)
After this MR
$ FPROF=1 bundle exec rspec ee/spec/requests/api/maven_packages_spec.rb
[TEST PROF INFO] FactoryProf enabled (simple mode)
Run options: include {:focus=>true}
All examples were filtered out; ignoring {:focus=>true}
==> Setting up Gitaly...
Checking gitaly-ruby Gemfile...
Checking gitaly-ruby bundle...
The Gemfile's dependencies are satisfied
Trying to connect to gitaly: ........ OK
Trying to connect to praefect: ... OK
Gitaly set up in 1.622047 seconds...
==> Setting up GitLab Workhorse...
GitLab Workhorse set up in 0.000111 seconds...
==> Setting up GitLab Elasticsearch Indexer...
GitLab Elasticsearch Indexer set up in 0.000193 seconds...
...............[fog][WARNING] Your region 'gdk' does not match the default region 'us-east-1'
...............[fog][WARNING] Your region 'gdk' does not match the default region 'us-east-1'
.[fog][WARNING] Your region 'gdk' does not match the default region 'us-east-1'
...........[fog][WARNING] Your region 'gdk' does not match the default region 'us-east-1'
.[fog][WARNING] Your region 'gdk' does not match the default region 'us-east-1'
..................
Finished in 22.12 seconds (files took 35.51 seconds to load)
61 examples, 0 failures
[TEST PROF INFO] Factories usage
Total: 18
Total top-level: 7
Total time: 1.3265s
Total uniq factories: 12
total top-level total time top-level time name
3 0 0.0529s 0.0000s package_file
3 1 0.8411s 0.5336s project
2 1 0.1475s 0.0866s user
2 0 0.0633s 0.0000s namespace
1 0 0.1891s 0.0000s package
1 1 0.0162s 0.0162s personal_access_token
1 1 0.3071s 0.3071s ci_build
1 1 0.0259s 0.0259s license
1 0 0.1674s 0.0000s ci_pipeline
1 1 0.0818s 0.0818s group
1 1 0.2754s 0.2754s maven_package
1 0 0.2042s 0.0000s maven_metadatum- 18 calls accounting for 1.32secs out of 22.12secs (~5.9%)
- Execution time reduced by ~40%
Does this MR meet the acceptance criteria?
Conformity
- [-] Changelog entry
- [-] Documentation (if required)
-
Code review guidelines -
Merge request performance guidelines -
Style guides -
Database guides -
Separation of EE specific content
Availability and Testing
- [-] Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process.
- [-] Tested in all supported browsers
- [-] Informed Infrastructure department of a default or new setting change, if applicable per definition of done
Security
If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:
- [-] Label as security and @ mention
@gitlab-com/gl-security/appsec - [-] The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
- [-] Security reports checked/validated by a reviewer from the AppSec team
Edited by David Fernandez