Fix gitlab:geo:check_replication_verification_status spec

Problem

While writing !41618 (merged), the tests which I copied from check_replication_verification_status did not seem to act as expected. The output tests succeeded regardless of the output. I don't remember exactly if the exit code tests were affected.

But I discovered @brodock's abort_execution matcher which was perfect for this case anyway. So I suggest we go that route for this spec as well.

The cause is an around hook in the describe block that runs example.run rescue SystemExit. The rake task always ends by calling exit 0, exit 1, or abort. That raises SystemExit, which escapes the expect { run_task } block before the output matcher gets a chance to compare anything. The around hook then catches the exception outside the example, so RSpec reports the example as passing. The result is that the success and error message assertions never actually run. The "exits with a 1" example still works, because it asserts on the raised exception directly rather than on output.

We can't use abort_execution everywhere, so it's added as an optional step in the implementation plan.

Proposal

Remove the around hook, and have each example assert on the exit explicitly by chaining .and raise_error(SystemExit) onto the output matcher. This pattern is already used elsewhere in the same file, for gitlab:geo:update_primary_node_url.

expect { run_task }
  .to output(/SUCCESS - Replication is up-to-date/).to_stdout
  .and raise_error(SystemExit)

The abort_execution matcher in spec/support/matchers/abort_matcher.rb was suggested originally as an alternative. It only partially fits here: it checks stderr and requires an exit status of 1, but this task prints to stdout and exits 0 on success. It can still be used for the two paths that call abort (the license check and the "Geo is not configured for this site" case), but that is optional.

Implementation plan

  1. Delete the around block (lines 24-27) in the check_replication_verification_status describe.
  2. Update the success-message examples (the plain one and the org-migration-target one) to use expect { run_task }.to output(/SUCCESS - Replication is up-to-date/).to_stdout.and raise_error(SystemExit).
  3. Update the error-message example the same way. Optionally fold the separate "exits with a 1" example into it, by checking the exit status inside a block passed to raise_error(SystemExit).
  4. Update the "on a primary node" example to drop its inner rescue SystemExit and use the same chained form as the other examples.
  5. Optionally convert the "when geo is not properly configured" example to use abort_execution.with_message(...).
  6. Verify the fix by temporarily replacing an expected regex with a bogus string and confirming the example now fails, then run the full spec file to confirm everything else still passes.
Edited by 🤖 GitLab Bot 🤖