Skip to content

Fix flaky examples in group policy spec

Peter Leitzen requested to merge pl-fix-flaky-group-policy-spec into master

What does this MR do and why?

Prefer before over before_all for updating group attributes. This is needed because in group uses refind: true (in shared context) which doesn't play well with before_all.

Clear memoization for Group#has_project_with_service_desk_enabled? by using let_it_be_with_refind.

Fixes #387704 (closed).

Performance impact

The runtime is roughly the same but we are seeing more Total events (7828 to 8014) because of update! in before block (was before_all). Not too bad though.

No changes for factories created.

Interestingly, running bin/rspec spec/policies/group_policy_spec.rb fails on master with:

Failures:

  1) GroupPolicy support bot when group hierarchy has a project with service desk enabled is expected to be allowed :read_label
     Failure/Error: permissions.each { |p| is_expected.to be_allowed(p) }
       expected `#<GroupPolicy (@support-bot : Group/671)>.allowed?(:read_label)` to be truthy, got false
     # ./spec/support/helpers/policy_helpers.rb:5:in `block in expect_allowed'
     # ./spec/support/helpers/policy_helpers.rb:5:in `each'
     # ./spec/support/helpers/policy_helpers.rb:5:in `expect_allowed'
     # ./spec/policies/group_policy_spec.rb:1089:in `block (4 levels) in <top (required)>'
     # ./spec/spec_helper.rb:415:in `block (3 levels) in <top (required)>'
     # ./spec/support/sidekiq_middleware.rb:18:in `with_sidekiq_server_middleware'
     # ./spec/spec_helper.rb:407:in `block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:403:in `block (3 levels) in <top (required)>'
     # ./lib/gitlab/application_context.rb:59:in `with_raw_context'
     # ./spec/spec_helper.rb:403:in `block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:239: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>'

Finished in 25.31 seconds (files took 8.46 seconds to load)
221 examples, 1 failure

Failed examples:

rspec ./spec/policies/group_policy_spec.rb:1089 # GroupPolicy support bot when group hierarchy has a project with service desk enabled is expected to be allowed :read_label

Before

221 examples, 1 failure

Failed examples:

rspec ./spec/policies/group_policy_spec.rb:1089 # GroupPolicy support bot when group hierarchy has a project with se
rvice desk enabled is expected to be allowed :read_label

Randomized with seed 20475

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

 Total: 338
 Total top-level: 146
 Total time: 00:12.642 (out of 00:37.993)
 Total uniq factories: 12

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

      61          61        4.5831s            0.0751s             4.5831s              group
      61           0        0.6730s            0.0110s             0.0000s namespace_settings
      61           0        0.2562s            0.0042s             0.0000s namespace_ci_cd_settings
      54           0        0.2396s            0.0044s             0.0000s       crm_settings
      44          39        3.8564s            0.0876s             3.5243s               user
      16          16        0.1195s            0.0075s             0.1195s       deploy_token
      12          10        3.8557s            0.3213s             3.3931s            project
      10           1        0.9358s            0.0936s             0.1054s          namespace
       9           9        0.0607s            0.0067s             0.0607s group_deploy_token
       8           8        0.7907s            0.0988s             0.7907s project_group_link
       1           1        0.0585s            0.0585s             0.0585s              admin
       1           1        0.0071s            0.0071s             0.0071s            license

After

Finished in 26.57 seconds (files took 8.66 seconds to load)
221 examples, 0 failures

Randomized with seed 38078

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

 Total: 338
 Total top-level: 146
 Total time: 00:10.222 (out of 00:33.806)
 Total uniq factories: 12

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

      61          61        3.7033s            0.0607s             3.7033s              group
      61           0        0.2620s            0.0043s             0.0000s namespace_settings
      61           0        0.2117s            0.0035s             0.0000s namespace_ci_cd_settings
      54           0        0.2014s            0.0037s             0.0000s       crm_settings
      44          39        3.4974s            0.0795s             2.8076s               user
      16          16        0.1274s            0.0080s             0.1274s       deploy_token
      12          10        3.1679s            0.2640s             2.7559s            project
      10           1        0.7003s            0.0700s             0.0662s          namespace
       9           9        0.0526s            0.0058s             0.0526s group_deploy_token
       8           8        0.6638s            0.0830s             0.6638s project_group_link
       1           1        0.0385s            0.0385s             0.0385s              admin
       1           1        0.0073s            0.0073s             0.0073s            license

How to set up and validate locally

bin/rspec ./spec/policies/group_policy_spec.rb[1:27:1,1:27:2:1] --seed 16138

bin/rspec ./spec/policies/group_policy_spec.rb[1:9:1:1,1:17:2:3:1] --order random --seed 2131

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 Peter Leitzen

Merge request reports