Skip to content

Fix flaky spec where commits where displayed in inverted order

Ahmed Hemdan requested to merge fix-flaky-spec-to-move-secrets-check-out-of-quarantine into master

What does this MR do and why?

This merge request fixes a flaky spec for Secret Push Protection, in which the commits are displayed in an inverted order in the error message displayed when multiple commits include the same secret (like for example when pushing two commits in which the earlier one has a secret).

More context copied from @serenafang (over Slack):

The root of the problem is that in ee/spec/support/shared_examples/lib/gitlab/secrets_check_shared_examples.rb we are defining changes as:

    let(:changes) do
      [
        { oldrev: initial_commit, newrev: new_commit, ref: 'refs/heads/master' },
        { oldrev: initial_commit, newrev: second_commit_with_secret, ref: 'refs/heads/master' }
      ]
    end

with 2 pairs of oldrev and newrev.

But the actual behavior is different -- oldrev is the latest pushed change, and newrev is the latest unpushed change, so there is only 1 pair of oldrev newrev, even with 3 unpushed commits.

In order to fix this, we define only one pair of changes instead of two, but then we have to stub repository.new_commits method to return the correct set of commits that should have been scanned for that specific branch we're testing on. With the correct commits returned, the test passes every time and is no longer flaky.

Please also see the original merge request introducing the flaky test for extra context.

Manual tests

I have also followed the flaky tests guide and tested this locally as follows:

  1. Ran the flaky test against all seeds causing failures: 1, 2, 3, 4, 5.
  2. Ran scripts/rspec_check_order_dependence ./ee/spec/lib/gitlab/checks/secrets_check_spec.rb.
  3. Ran while :; do bin/rspec ee/spec/lib/gitlab/checks/secrets_check_spec.rb[1:1:2:2:2:2:9:1] || break; done.

Resolves #469071 (closed).

MR acceptance checklist

I have evaluated this MR against the MR acceptance checklist.

Edited by Ahmed Hemdan

Merge request reports