From 37492c5d4439e03f374da487b43ac3e016a93b15 Mon Sep 17 00:00:00 2001 From: Luke Duncalfe Date: Wed, 10 Apr 2019 12:52:24 +1200 Subject: Fix logic in Api::Internal test 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. This test was passing, however was not testing what it was supposed to be! 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. But this test wasn't failing because unfortunately I had put: ``` expect(MergeRequests::PushOptionsHandlerService).to receive(:new) ``` instead of not_to! This meant that the `.new` method was being stubbed, so the service class did not create a MergeRequest, which satisfied the second expectation. ``` expect(MergeRequests::PushOptionsHandlerService).to receive(:new) ``` --- spec/requests/api/internal_spec.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal_spec.rb index 1ce8f520962..88c19448373 100644 --- a/spec/requests/api/internal_spec.rb +++ b/spec/requests/api/internal_spec.rb @@ -998,15 +998,13 @@ describe API::Internal do context 'when the feature is disabled' do it 'does not invoke MergeRequests::PushOptionsHandlerService' do - Feature.disable(:mr_push_options) + stub_feature_flags(mr_push_options: false) - expect(MergeRequests::PushOptionsHandlerService).to receive(:new) + expect(MergeRequests::PushOptionsHandlerService).not_to receive(:new) expect do post api('/internal/post_receive'), params: valid_params end.not_to change { MergeRequest.count } - - Feature.enable(:mr_push_options) end end end -- cgit v1.2.1