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:

3ee48e422d

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)
```
This commit is contained in:
Luke Duncalfe 2019-04-10 12:52:24 +12:00
parent 8c10e99de2
commit 37492c5d44
1 changed files with 2 additions and 4 deletions

View File

@ -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