Skip to content

Fix logic in Api::Internal test

Luke Duncalfe requested to merge fix-logic-in-43263-git-push-option-to-create-mr-test into master

What does this MR do?

This MR corrects a test of Api::Internal.

This test was passing, however was not testing what it was supposed to!

The intention of this test is to ensure that the service class MergeRequests::PushOptionsHandlerService does not run when the :mr_push_options feature flag is disabled.

What was wrong?

For one, setting Feature.disable(:feature) in the test does not disable the feature, as rspec config in spec_helper stubs Feature to make all features enabled https://gitlab.com/gitlab-org/gitlab-ce/commit/3ee48e422defaedd69946c607bd8d3672e510375

So the feature was still enabled in the test.

This test wasn't failing because unfortunately I had used expect(...).to instead of expect(...).not_to:

# This:
expect(MergeRequests::PushOptionsHandlerService).to receive(:new)
# Should have been:
expect(MergeRequests::PushOptionsHandlerService).not_to receive(:new)

This meant that the .new method was being stubbed, so the service class did not create a MergeRequest, which satisfied the second expectation.

Does this MR meet the acceptance criteria?

Conformity

Edited by Luke Duncalfe

Merge request reports