Fix PruneDeletionsWorker spec after safety-net refresh change

Eugie Limpinrequested to merge
el-fix-prune-deletions-worker-spec into master

What does this MR do and why?

Merge request Batch authorized projects refresh in PruneDelet... (!254125 - merged) added specs that expect AuthorizedProjectUpdate::UserRefreshFromReplicaWorker to receive bulk_perform_in. While that MR was open, Create a queue to run safety-net refreshes of p... (!249045 - merged) landed on master and added the feature flag use_db_to_queue_safety_net_auth_refresh. When that flag is on, UserProjectAccessChangedService#execute(priority: LOW_PRIORITY) writes to the project_authorization_reverifications table via Authz::ProjectAuthorizationReverification.queue_users instead of enqueuing AuthorizedProjectUpdate::UserRefreshFromReplicaWorker. Our spec suite turns feature flags on by default, so the worker is never called anymore and the expectation fails. That MR's own pipeline passed because it ran against a master that did not yet have the flag, so nothing showed a conflict until both changes were on master together.

This is a test-only change. It removes the two expectations on AuthorizedProjectUpdate::UserRefreshFromReplicaWorker.bulk_perform_in, one that it is called once and one that it is not called, and the stub_feature_flags(do_not_run_safety_net_auth_refresh_jobs: false) line that only existed for them.

Members::PruneDeletionsWorker never enqueues that worker itself. It only asks UserProjectAccessChangedService for a refresh, and both the batch refresh and the per-membership safety net go through that same call with a different priority. The spec still checks how many times the worker asks and with which priority: once at medium priority when the flag is on, once at low priority when it is off.

Why not assert on the reverification queue instead? That would swap one service internal for another, so the spec would break again the next time the service changes, for example when the use_db_to_queue_safety_net_auth_refresh flag is removed. It would also need the do_not_run_safety_net_auth_refresh_jobs stub back, because that flag is on by default in specs and stops the low priority path before anything is queued.

References

MR acceptance checklist

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

Edited by Eugie Limpin

Merge request reports