Speed up projects merge requests controller specs
What does this MR do?
This MR improves the performance of projects merge requests controller specs by using let_it_be
. It also fixes some Rails/SaveBang
.
Related to &3752.
Contributes to https://gitlab.com/gitlab-org/plan/-/issues/145 and #220040 (closed).
Here's the output from test-prof
when run with FPROF=1
. Note:
-
Total
amount of factories created went down from 827 to 444👍 -
Total events
went down from 32846 to 21749 - Queries saved: 11097
Before
[TEST PROF INFO] EventProf results for sql.active_record
Total time: 00:43.357 of 02:06.514 (34.27%)
Total events: 32846
Top 5 slowest suites (by time):
Projects::Mer...estsController (./spec/controllers/projects/merge_requests_controller_spec.rb:5) – 00:43.357 (32846 / 172) of 02:06.514 (34.27%)
Finished in 2 minutes 13.2 seconds (files took 1.66 seconds to load)
173 examples, 0 failure
[TEST PROF INFO] Factories usage
Total: 827
Total top-level: 581
Total time: 80.0098s
Total uniq factories: 20
total top-level total time time per call top-level time name
193 193 36.3331s 0.1883s 36.3331s project
175 4 8.3180s 0.0475s 0.1247s namespace
105 105 16.4824s 0.1570s 16.4824s merge_request
88 88 17.0537s 0.1938s 17.0537s merge_request_with_diffs
86 49 2.9574s 0.0344s 1.8930s ci_pipeline
35 35 1.6236s 0.0464s 1.6236s user
22 6 0.8162s 0.0371s 0.1130s ci_job_artifact
18 18 0.3855s 0.0214s 0.3855s discussion_note_on_merge_request
18 18 1.0362s 0.0576s 1.0362s group
18 0 0.0963s 0.0054s 0.0000s namespace_settings
15 15 0.9250s 0.0617s 0.9250s ci_build
13 13 0.2572s 0.0198s 0.2572s environment
13 13 1.3190s 0.1015s 1.3190s deployment
8 8 0.4026s 0.0503s 0.4026s issue
6 6 0.1945s 0.0324s 0.1945s ci_empty_pipeline
6 2 0.4602s 0.0767s 0.1320s diff_note_on_merge_request
4 4 1.3790s 0.3448s 1.3790s merge_request_with_diff_notes
2 2 0.2505s 0.1253s 0.2505s invalid_merge_request
1 1 0.0078s 0.0078s 0.0078s license
1 1 0.0970s 0.0970s 0.0970s diff_note_on_commit
After
[TEST PROF INFO] EventProf results for sql.active_record
Total time: 00:27.435 of 01:25.549 (32.07%)
Total events: 21749
Top 5 slowest suites (by time):
Projects::Mer...estsController (./spec/controllers/projects/merge_requests_controller_spec.rb:5) – 00:27.435 (21749 / 172) of 01:25.549 (32.07%)
Finished in 1 minute 32.56 seconds (files took 1.68 seconds to load)
173 examples, 0 failure
[TEST PROF INFO] Factories usage
Total: 444
Total top-level: 385
Total time: 38.3504s
Total uniq factories: 20
total top-level total time time per call top-level time name
88 88 14.4287s 0.1640s 14.4287s merge_request_with_diffs
73 73 9.8153s 0.1345s 9.8153s merge_request
54 49 2.8688s 0.0531s 2.7493s ci_pipeline
35 35 5.0335s 0.1438s 5.0335s project
32 32 1.4473s 0.0452s 1.4473s user
22 6 1.6126s 0.0733s 0.0901s ci_job_artifact
18 18 0.3901s 0.0217s 0.3901s discussion_note_on_merge_request
18 18 0.4898s 0.0272s 0.4898s group
18 0 0.0766s 0.0043s 0.0000s namespace_settings
17 1 0.7966s 0.0469s 0.0347s namespace
15 15 0.6921s 0.0461s 0.6921s ci_build
13 13 0.1999s 0.0154s 0.1999s environment
13 13 1.0962s 0.0843s 1.0962s deployment
8 8 0.3445s 0.0431s 0.3445s issue
6 6 0.2295s 0.0382s 0.2295s ci_empty_pipeline
6 2 0.4106s 0.0684s 0.1327s diff_note_on_merge_request
4 4 0.8722s 0.2180s 0.8722s merge_request_with_diff_notes
2 2 0.2057s 0.1028s 0.2057s invalid_merge_request
1 1 0.0095s 0.0095s 0.0095s license
1 1 0.0893s 0.0893s 0.0893s diff_note_on_commit
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 Peter Leitzen