diff options
author | Eric Eastwood <contact@ericeastwood.com> | 2017-08-31 19:46:18 -0500 |
---|---|---|
committer | Eric Eastwood <contact@ericeastwood.com> | 2017-09-18 18:06:39 -0500 |
commit | 5ecfd3cd0f6be51d4e01292b0c5583427d73aa8d (patch) | |
tree | f51e560435c96326f07dd7c6a623172b9679390c /spec | |
parent | f0b089cf7881a0eed7f3aefe22d5b3006a39023d (diff) | |
download | gitlab-ce-5ecfd3cd0f6be51d4e01292b0c5583427d73aa8d.tar.gz |
Fix MR widget with external CI services/integrations33287-fix-mr-widget-errors-with-external-services
Fix https://gitlab.com/gitlab-org/gitlab-ce/issues/33287
The MR widget was trying to render the pipelines section when
there are no GitLab CI pipelines which was throwing some NPE
errors.
Diffstat (limited to 'spec')
5 files changed, 128 insertions, 16 deletions
diff --git a/spec/features/merge_requests/widget_spec.rb b/spec/features/merge_requests/widget_spec.rb index fd991293ee9..443b596b3c6 100644 --- a/spec/features/merge_requests/widget_spec.rb +++ b/spec/features/merge_requests/widget_spec.rb @@ -142,6 +142,24 @@ describe 'Merge request', :js do end end + context 'view merge request where project has CI setup but no CI status' do + before do + pipeline = create(:ci_pipeline, project: project, + sha: merge_request.diff_head_sha, + ref: merge_request.source_branch) + create(:ci_build, pipeline: pipeline) + + visit project_merge_request_path(project, merge_request) + end + + it 'has pipeline error text' do + # Wait for the `ci_status` and `merge_check` requests + wait_for_requests + + expect(page).to have_text('Could not connect to the CI server. Please check your settings and try again') + end + end + context 'view merge request with MWPS enabled but automatically merge fails' do before do merge_request.update( diff --git a/spec/javascripts/vue_mr_widget/components/mr_widget_pipeline_spec.js b/spec/javascripts/vue_mr_widget/components/mr_widget_pipeline_spec.js index c763487d12f..690665ae12c 100644 --- a/spec/javascripts/vue_mr_widget/components/mr_widget_pipeline_spec.js +++ b/spec/javascripts/vue_mr_widget/components/mr_widget_pipeline_spec.js @@ -37,6 +37,26 @@ describe('MRWidgetPipeline', () => { }); }); + describe('hasPipeline', () => { + it('should return true when there is a pipeline', () => { + expect(Object.keys(mockData.pipeline).length).toBeGreaterThan(0); + + const vm = createComponent({ + pipeline: mockData.pipeline, + }); + + expect(vm.hasPipeline).toBeTruthy(); + }); + + it('should return false when there is no pipeline', () => { + const vm = createComponent({ + pipeline: null, + }); + + expect(vm.hasPipeline).toBeFalsy(); + }); + }); + describe('hasCIError', () => { it('should return false when there is no CI error', () => { const vm = createComponent({ diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js index c607c9746a4..8f69458ffb0 100644 --- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js +++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js @@ -11,6 +11,7 @@ const createComponent = (customConfig = {}) => { isPipelineActive: false, pipeline: null, isPipelineFailed: false, + isPipelinePassing: false, onlyAllowMergeIfPipelineSucceeds: false, hasCI: false, ciStatus: null, @@ -68,6 +69,18 @@ describe('MRWidgetReadyToMerge', () => { }); describe('computed', () => { + describe('shouldShowMergeWhenPipelineSucceedsText', () => { + it('should return true with active pipeline', () => { + vm.mr.isPipelineActive = true; + expect(vm.shouldShowMergeWhenPipelineSucceedsText).toBeTruthy(); + }); + + it('should return false with inactive pipeline', () => { + vm.mr.isPipelineActive = false; + expect(vm.shouldShowMergeWhenPipelineSucceedsText).toBeFalsy(); + }); + }); + describe('commitMessageLinkTitle', () => { const withDesc = 'Include description in commit message'; const withoutDesc = "Don't include description in commit message"; @@ -203,20 +216,55 @@ describe('MRWidgetReadyToMerge', () => { describe('methods', () => { describe('isMergeAllowed', () => { - it('should return false with initial data', () => { + it('should return true when no pipeline and not required to succeed', () => { + vm.mr.onlyAllowMergeIfPipelineSucceeds = false; + vm.mr.isPipelinePassing = false; expect(vm.isMergeAllowed()).toBeTruthy(); }); - it('should return false when MR is set only merge when pipeline succeeds', () => { - vm.mr.onlyAllowMergeIfPipelineSucceeds = true; + it('should return true when pipeline failed and not required to succeed', () => { + vm.mr.onlyAllowMergeIfPipelineSucceeds = false; + vm.mr.isPipelinePassing = false; expect(vm.isMergeAllowed()).toBeTruthy(); }); - it('should return true true', () => { + it('should return false when pipeline failed and required to succeed', () => { vm.mr.onlyAllowMergeIfPipelineSucceeds = true; - vm.mr.isPipelineFailed = true; + vm.mr.isPipelinePassing = false; expect(vm.isMergeAllowed()).toBeFalsy(); }); + + it('should return true when pipeline succeeded and required to succeed', () => { + vm.mr.onlyAllowMergeIfPipelineSucceeds = true; + vm.mr.isPipelinePassing = true; + expect(vm.isMergeAllowed()).toBeTruthy(); + }); + }); + + describe('shouldShowMergeControls', () => { + it('should return false when an external pipeline is running and required to succeed', () => { + spyOn(vm, 'isMergeAllowed').and.returnValue(false); + vm.mr.isPipelineActive = false; + expect(vm.shouldShowMergeControls()).toBeFalsy(); + }); + + it('should return true when the build succeeded or build not required to succeed', () => { + spyOn(vm, 'isMergeAllowed').and.returnValue(true); + vm.mr.isPipelineActive = false; + expect(vm.shouldShowMergeControls()).toBeTruthy(); + }); + + it('should return true when showing the MWPS button and a pipeline is running that needs to be successful', () => { + spyOn(vm, 'isMergeAllowed').and.returnValue(false); + vm.mr.isPipelineActive = true; + expect(vm.shouldShowMergeControls()).toBeTruthy(); + }); + + it('should return true when showing the MWPS button but not required for the pipeline to succeed', () => { + spyOn(vm, 'isMergeAllowed').and.returnValue(true); + vm.mr.isPipelineActive = true; + expect(vm.shouldShowMergeControls()).toBeTruthy(); + }); }); describe('updateCommitMessage', () => { diff --git a/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js b/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js index 669ee248bf1..da66c7504cb 100644 --- a/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js +++ b/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js @@ -59,23 +59,15 @@ describe('mrWidgetOptions', () => { }); describe('shouldRenderPipelines', () => { - it('should return true for the initial data', () => { - expect(vm.shouldRenderPipelines).toBeTruthy(); - }); + it('should return true when hasCI is true', () => { + vm.mr.hasCI = true; - it('should return true when pipeline is empty but MR.hasCI is set to true', () => { - vm.mr.pipeline = {}; expect(vm.shouldRenderPipelines).toBeTruthy(); }); - it('should return true when pipeline available', () => { + it('should return false when hasCI is false', () => { vm.mr.hasCI = false; - expect(vm.shouldRenderPipelines).toBeTruthy(); - }); - it('should return false when there is no pipeline', () => { - vm.mr.pipeline = {}; - vm.mr.hasCI = false; expect(vm.shouldRenderPipelines).toBeFalsy(); }); }); diff --git a/spec/javascripts/vue_mr_widget/stores/mr_widget_store_spec.js b/spec/javascripts/vue_mr_widget/stores/mr_widget_store_spec.js index 56dd0198ae2..8e5614b20f0 100644 --- a/spec/javascripts/vue_mr_widget/stores/mr_widget_store_spec.js +++ b/spec/javascripts/vue_mr_widget/stores/mr_widget_store_spec.js @@ -18,5 +18,39 @@ describe('MergeRequestStore', () => { store.setData({ ...mockData, work_in_progress: !mockData.work_in_progress }); expect(store.hasSHAChanged).toBe(false); }); + + describe('isPipelinePassing', () => { + it('is true when the CI status is `success`', () => { + store.setData({ ...mockData, ci_status: 'success' }); + expect(store.isPipelinePassing).toBe(true); + }); + + it('is true when the CI status is `success_with_warnings`', () => { + store.setData({ ...mockData, ci_status: 'success_with_warnings' }); + expect(store.isPipelinePassing).toBe(true); + }); + + it('is false when the CI status is `failed`', () => { + store.setData({ ...mockData, ci_status: 'failed' }); + expect(store.isPipelinePassing).toBe(false); + }); + + it('is false when the CI status is anything except `success`', () => { + store.setData({ ...mockData, ci_status: 'foobarbaz' }); + expect(store.isPipelinePassing).toBe(false); + }); + }); + + describe('isPipelineSkipped', () => { + it('should set isPipelineSkipped=true when the CI status is `skipped`', () => { + store.setData({ ...mockData, ci_status: 'skipped' }); + expect(store.isPipelineSkipped).toBe(true); + }); + + it('should set isPipelineSkipped=false when the CI status is anything except `skipped`', () => { + store.setData({ ...mockData, ci_status: 'foobarbaz' }); + expect(store.isPipelineSkipped).toBe(false); + }); + }); }); }); |