Skip to content
Snippets Groups Projects

Remove a few offenses for ExcessiveCreateList cop

Merged David Dieulivol requested to merge 416899-resolve_some_rubocop_offenses into master
All threads resolved!

Context

Related to #416899 (closed)

What does this MR do and why?

Fixes a few Rubocop offenses for the RSpec/FactoryBot/ExcessiveCreateList cop.

Performance improvements

  • 32.56 seconds versus 28.24 seconds locally
  • Went from 353 to 182 Factory objects creation

Command we used to measure

FPROF=1 be rspec \
  ee/spec/controllers/projects/vulnerability_feedback_controller_spec.rb \
  ee/spec/frontend/fixtures/on_demand_dast_scans.rb \
  ee/spec/graphql/types/dast_scanner_profile_type_spec.rb \
  ee/spec/graphql/types/dast_site_profile_type_spec.rb

Before

[TEST PROF INFO] FactoryProf enabled (simple mode)
Run options: include {:focus=>true}

All examples were filtered out; ignoring {:focus=>true}
[TEST PROF INFO] No factories detected
[TEST PROF INFO] No factories detected
[TEST PROF INFO] No factories detected
[TEST PROF INFO] No factories detected
[TEST PROF INFO] No factories detected
[TEST PROF INFO] No factories detected

Test environment set up in 6.308292 seconds
..........................................Missing metadata feature_category: ./ee/spec/frontend/fixtures/on_demand_dast_scans.rb:32 See https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#feature-category-metadata
.Missing metadata feature_category: ./ee/spec/frontend/fixtures/on_demand_dast_scans.rb:65 See https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#feature-category-metadata
.Missing metadata feature_category: ./ee/spec/frontend/fixtures/on_demand_dast_scans.rb:79 See https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#feature-category-metadata
......Missing metadata feature_category: ./ee/spec/graphql/types/dast_site_profile_type_spec.rb:18 See https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#feature-category-metadata
.Missing metadata feature_category: ./ee/spec/graphql/types/dast_site_profile_type_spec.rb:19 See https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#feature-category-metadata
.Missing metadata feature_category: ./ee/spec/graphql/types/dast_site_profile_type_spec.rb:20 See https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#feature-category-metadata
.Missing metadata feature_category: ./ee/spec/graphql/types/dast_site_profile_type_spec.rb:22 See https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#feature-category-metadata
.Missing metadata feature_category: ./ee/spec/graphql/types/dast_site_profile_type_spec.rb:23 See https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#feature-category-metadata
.Missing metadata feature_category: ./ee/spec/graphql/types/dast_site_profile_type_spec.rb:26 See https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#feature-category-metadata
.Missing metadata feature_category: ./ee/spec/graphql/types/dast_site_profile_type_spec.rb:32 See https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#feature-category-metadata
.Missing metadata feature_category: ./ee/spec/graphql/types/dast_site_profile_type_spec.rb:38 See https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#feature-category-metadata
.Missing metadata feature_category: ./ee/spec/graphql/types/dast_site_profile_type_spec.rb:44 See https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#feature-category-metadata
.Missing metadata feature_category: ./ee/spec/graphql/types/dast_site_profile_type_spec.rb:50 See https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#feature-category-metadata
.Missing metadata feature_category: ./ee/spec/graphql/types/dast_site_profile_type_spec.rb:58 See https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#feature-category-metadata
.Missing metadata feature_category: ./ee/spec/graphql/types/dast_site_profile_type_spec.rb:64 See https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#feature-category-metadata
.Missing metadata feature_category: ./ee/spec/graphql/types/dast_site_profile_type_spec.rb:71 See https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#feature-category-metadata
.Missing metadata feature_category: ./ee/spec/graphql/types/dast_site_profile_type_spec.rb:77 See https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#feature-category-metadata
.Missing metadata feature_category: ./ee/spec/graphql/types/dast_site_profile_type_spec.rb:86 See https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#feature-category-metadata
.Missing metadata feature_category: ./ee/spec/graphql/types/dast_site_profile_type_spec.rb:92 See https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#feature-category-metadata
.Missing metadata feature_category: ./ee/spec/graphql/types/dast_site_profile_type_spec.rb:100 See https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#feature-category-metadata
.Missing metadata feature_category: ./ee/spec/graphql/types/dast_site_profile_type_spec.rb:109 See https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#feature-category-metadata
.Missing metadata feature_category: ./ee/spec/graphql/types/dast_site_profile_type_spec.rb:119 See https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#feature-category-metadata
.Missing metadata feature_category: ./ee/spec/graphql/types/dast_site_profile_type_spec.rb:178 See https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#feature-category-metadata
.Missing metadata feature_category: ./ee/spec/graphql/types/dast_site_profile_type_spec.rb:186 See https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#feature-category-metadata
.Missing metadata feature_category: ./ee/spec/graphql/types/dast_site_profile_type_spec.rb:200 See https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#feature-category-metadata
.

Finished in 32.56 seconds (files took 14.13 seconds to load)
72 examples, 0 failures

[TEST PROF INFO] Time spent in factories: 00:13.352 (38.19% of total time)
[TEST PROF INFO] Factories usage

 Total: 353
 Total top-level: 244
 Total time: 00:13.352 (out of 00:41.925)
 Total uniq factories: 26

   total   top-level     total time      time per call      top-level time               name

      85          85        1.4085s            0.0166s             1.4085s vulnerability_feedback
      64          55        1.0612s            0.0166s             0.9695s        ci_pipeline
      33           0        1.0432s            0.0316s             0.0000s          dast_site
      33          32        2.1796s            0.0660s             2.1161s  dast_site_profile
      32          31        0.2773s            0.0087s             0.2677s dast_scanner_profile
      26           9        6.1339s            0.2359s             4.0774s            project
      21           0        1.0601s            0.0505s             0.0000s          namespace
      14           9        0.6834s            0.0488s             0.3966s               user
       7           0        0.2113s            0.0302s             0.0000s             author
       4           4        0.0233s            0.0058s             0.0233s            license
       4           4        1.8095s            0.4524s             1.8095s      vulnerability
       3           0        0.3179s            0.1060s             0.0000s              issue
       3           2        1.0427s            0.3476s             0.8064s      merge_request
       3           3        0.9934s            0.3311s             0.9934s vulnerabilities_finding
       3           0        0.3330s            0.1110s             0.0000s vulnerabilities_identifier
       3           0        0.3279s            0.1093s             0.0000s vulnerabilities_scanner
       3           3        0.0237s            0.0079s             0.0237s personal_access_token
       3           0        0.0373s            0.0124s             0.0000s     work_item_type
       2           2        0.0128s            0.0064s             0.0128s security_orchestration_policy_configuration
       1           0        0.0141s            0.0141s             0.0000s namespace_settings
       1           1        0.0079s            0.0079s             0.0079s dast_site_validation
       1           1        0.3071s            0.3071s             0.3071s              group
       1           1        0.0428s            0.0428s             0.0428s    dast_site_token
       1           1        0.0080s            0.0080s             0.0080s dast_site_profile_secret_variable
       1           0        0.0035s            0.0035s             0.0000s namespace_ci_cd_settings
       1           1        0.0822s            0.0822s             0.0822s       dast_profile

After

$ FPROF=1 be rspec \
  ee/spec/controllers/projects/vulnerability_feedback_controller_spec.rb \
  ee/spec/frontend/fixtures/on_demand_dast_scans.rb \
  ee/spec/graphql/types/dast_scanner_profile_type_spec.rb \
  ee/spec/graphql/types/dast_site_profile_type_spec.rb
[TEST PROF INFO] FactoryProf enabled (simple mode)
Run options: include {:focus=>true}

All examples were filtered out; ignoring {:focus=>true}
[TEST PROF INFO] No factories detected
[TEST PROF INFO] No factories detected
[TEST PROF INFO] No factories detected
[TEST PROF INFO] No factories detected
[TEST PROF INFO] No factories detected
[TEST PROF INFO] No factories detected

Test environment set up in 6.1079 seconds
...........................................................................

Finished in 28.24 seconds (files took 12.63 seconds to load)
75 examples, 0 failures

[TEST PROF INFO] Time spent in factories: 00:10.095 (33.34% of total time)
[TEST PROF INFO] Factories usage

 Total: 182
 Total top-level: 101
 Total time: 00:10.095 (out of 00:36.728)
 Total uniq factories: 26

   total   top-level     total time      time per call      top-level time               name

      37          28        0.8534s            0.0231s             0.7513s        ci_pipeline
      26           9        6.0834s            0.2340s             4.0413s            project
      25          25        0.9279s            0.0371s             0.9279s vulnerability_feedback
      21           0        0.8002s            0.0381s             0.0000s          namespace
      14           9        0.6058s            0.0433s             0.3618s               user
       7           0        0.2164s            0.0309s             0.0000s             author
       5           4        0.2898s            0.0580s             0.2289s  dast_site_profile
       5           0        0.1524s            0.0305s             0.0000s          dast_site
       4           4        0.0216s            0.0054s             0.0216s            license
       4           4        1.3844s            0.3461s             1.3844s      vulnerability
       4           3        0.1524s            0.0381s             0.1437s dast_scanner_profile
       3           2        0.9491s            0.3164s             0.7121s      merge_request
       3           3        1.0764s            0.3588s             1.0764s vulnerabilities_finding
       3           0        0.3425s            0.1142s             0.0000s vulnerabilities_identifier
       3           0        0.3253s            0.1084s             0.0000s vulnerabilities_scanner
       3           3        0.0222s            0.0074s             0.0222s personal_access_token
       3           0        0.2009s            0.0670s             0.0000s              issue
       3           0        0.0170s            0.0057s             0.0000s     work_item_type
       2           2        0.0126s            0.0063s             0.0126s security_orchestration_policy_configuration
       1           1        0.2803s            0.2803s             0.2803s              group
       1           1        0.0383s            0.0383s             0.0383s    dast_site_token
       1           0        0.0030s            0.0030s             0.0000s namespace_ci_cd_settings
       1           1        0.0062s            0.0062s             0.0062s dast_site_validation
       1           1        0.0079s            0.0079s             0.0079s dast_site_profile_secret_variable
       1           0        0.0136s            0.0136s             0.0000s namespace_settings
       1           1        0.0782s            0.0782s             0.0782s       dast_profile

MR acceptance checklist

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

Edited by David Dieulivol

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • A deleted user added backend frontend labels

    added backend frontend labels

  • 1 Warning
    :warning: 671e6130: Commits that change 30 or more lines across at least 3 files should describe these changes in the commit body. For more information, take a look at our Commit message guidelines.
    1 Message
    :book: CHANGELOG missing:

    If you want to create a changelog entry for GitLab FOSS, add the Changelog trailer to the commit message you want to add to the changelog.

    If you want to create a changelog entry for GitLab EE, also add the EE: true trailer to your commit message.

    If this merge request doesn't need a CHANGELOG entry, feel free to ignore this message.

    Reviewer roulette

    Changes that require review have been detected!

    Please refer to the table below for assigning reviewers and maintainers suggested by Danger in the specified category:

    Category Reviewer Maintainer
    backend Marcos Rocha current availability (@mc_rocha) (UTC-4, 6 hours behind @ddieulivol) Heinrich Lee Yu current availability (@engwan) (UTC+8, 6 hours ahead of @ddieulivol)
    frontend Ankit Panchal current availability (@ankit.panchal) (UTC+5.5, 3.5 hours ahead of @ddieulivol) Miguel Rincon current availability (@mrincon) (UTC+2, same timezone as @ddieulivol)

    To spread load more evenly across eligible reviewers, Danger has picked a candidate for each review slot, based on their timezone. Feel free to override these selections if you think someone else would be better-suited or use the GitLab Review Workload Dashboard to find other available reviewers.

    To read more on how to use the reviewer roulette, please take a look at the Engineering workflow and code review guidelines. Please consider assigning a reviewer or maintainer who is a domain expert in the area of the merge request.

    Once you've decided who will review this merge request, assign them as a reviewer! Danger does not automatically notify them for you.

    If needed, you can retry the :repeat: danger-review job that generated this comment.

    Generated by :no_entry_sign: Danger

    Edited by Ghost User
  • David Dieulivol added 1 commit

    added 1 commit

    • 6ec0f244 - Remove a few offenses for ExcessiveCreateList cop

    Compare with previous version

  • David Dieulivol added 1 commit

    added 1 commit

    • 671e6130 - Remove a few offenses for ExcessiveCreateList cop

    Compare with previous version

  • David Dieulivol changed the description

    changed the description

  • David Dieulivol requested review from @splattael

    requested review from @splattael

  • Peter Leitzen
  • Peter Leitzen approved this merge request

    approved this merge request

  • Peter Leitzen requested review from @engwan and @mrincon and removed review request for @splattael

    requested review from @engwan and @mrincon and removed review request for @splattael

  • :wave: @splattael, thanks for approving this merge request.

    This is the first time the merge request is approved. To ensure full test coverage, a new pipeline will be started shortly.

    For more info, please refer to the following links:

  • Miguel Rincon requested review from @pgascouvaillancourt and removed review request for @mrincon

    requested review from @pgascouvaillancourt and removed review request for @mrincon

  • Heinrich Lee Yu removed review request for @engwan

    removed review request for @engwan

  • Heinrich Lee Yu approved this merge request

    approved this merge request

  • Paul Gascou-Vaillancourt approved this merge request

    approved this merge request

  • Paul Gascou-Vaillancourt resolved all threads

    resolved all threads

  • mentioned in commit 692d347e

  • added workflowstaging label and removed workflowcanary label

  • Mark Florian mentioned in merge request !125126 (merged)

    mentioned in merge request !125126 (merged)

  • Please register or sign in to reply
    Loading