From bd1122ee2fd5c8083c945390a1593b2b5774d8a1 Mon Sep 17 00:00:00 2001 From: Paul Slaughter Date: Thu, 4 Apr 2019 02:44:16 -0500 Subject: Fix vue render error for IDE status bar **What?** A Vue warning that `ide_status_bar` sent a `Boolean` to a `String` property (`img-src). **What was the fix?** Previously, `latestPipeline` could be one of the following values: | | | |----------|--------| | `null` | The pipeline hasn't loaded yet | | `false` | The pipeline has loaded, but nothing was returned. | | `Object` | The piepline has loaded. | Giving a semantic meaning to different falsey values hurts maintainability. This commit fixes the above problem by removing the `false` value and introducing a `hasLoadedPipeline` state property. --- .../ide/components/pipelines/list_spec.js | 31 ++++-- .../ide/stores/modules/pipelines/mutations_spec.js | 106 +++++++++++---------- 2 files changed, 81 insertions(+), 56 deletions(-) (limited to 'spec') diff --git a/spec/javascripts/ide/components/pipelines/list_spec.js b/spec/javascripts/ide/components/pipelines/list_spec.js index 68487733cb9..80829f2358a 100644 --- a/spec/javascripts/ide/components/pipelines/list_spec.js +++ b/spec/javascripts/ide/components/pipelines/list_spec.js @@ -11,6 +11,8 @@ describe('IDE pipelines list', () => { let vm; let mock; + const findLoadingState = () => vm.$el.querySelector('.loading-container'); + beforeEach(done => { const store = createStore(); @@ -95,7 +97,7 @@ describe('IDE pipelines list', () => { describe('empty state', () => { it('renders pipelines empty state', done => { - vm.$store.state.pipelines.latestPipeline = false; + vm.$store.state.pipelines.latestPipeline = null; vm.$nextTick(() => { expect(vm.$el.querySelector('.empty-state')).not.toBe(null); @@ -106,15 +108,30 @@ describe('IDE pipelines list', () => { }); describe('loading state', () => { - it('renders loading state when there is no latest pipeline', done => { - vm.$store.state.pipelines.latestPipeline = null; + beforeEach(() => { vm.$store.state.pipelines.isLoadingPipeline = true; + }); - vm.$nextTick(() => { - expect(vm.$el.querySelector('.loading-container')).not.toBe(null); + it('does not render when pipeline has loaded before', done => { + vm.$store.state.pipelines.hasLoadedPipeline = true; - done(); - }); + vm.$nextTick() + .then(() => { + expect(findLoadingState()).toBe(null); + }) + .then(done) + .catch(done.fail); + }); + + it('renders loading state when there is no latest pipeline', done => { + vm.$store.state.pipelines.hasLoadedPipeline = false; + + vm.$nextTick() + .then(() => { + expect(findLoadingState()).not.toBe(null); + }) + .then(done) + .catch(done.fail); }); }); }); diff --git a/spec/javascripts/ide/stores/modules/pipelines/mutations_spec.js b/spec/javascripts/ide/stores/modules/pipelines/mutations_spec.js index eb7346bd5fc..b558c45f574 100644 --- a/spec/javascripts/ide/stores/modules/pipelines/mutations_spec.js +++ b/spec/javascripts/ide/stores/modules/pipelines/mutations_spec.js @@ -27,63 +27,71 @@ describe('IDE pipelines mutations', () => { }); describe(types.RECEIVE_LASTEST_PIPELINE_SUCCESS, () => { - it('sets loading to false on success', () => { - mutations[types.RECEIVE_LASTEST_PIPELINE_SUCCESS]( - mockedState, - fullPipelinesResponse.data.pipelines[0], - ); + const itSetsPipelineLoadingStates = () => { + it('sets has loaded to true', () => { + expect(mockedState.hasLoadedPipeline).toBe(true); + }); - expect(mockedState.isLoadingPipeline).toBe(false); - }); + it('sets loading to false on success', () => { + expect(mockedState.isLoadingPipeline).toBe(false); + }); + }; + + describe('with pipeline', () => { + beforeEach(() => { + mutations[types.RECEIVE_LASTEST_PIPELINE_SUCCESS]( + mockedState, + fullPipelinesResponse.data.pipelines[0], + ); + }); - it('sets latestPipeline', () => { - mutations[types.RECEIVE_LASTEST_PIPELINE_SUCCESS]( - mockedState, - fullPipelinesResponse.data.pipelines[0], - ); + itSetsPipelineLoadingStates(); - expect(mockedState.latestPipeline).toEqual({ - id: '51', - path: 'test', - commit: { id: '123' }, - details: { status: jasmine.any(Object) }, - yamlError: undefined, + it('sets latestPipeline', () => { + expect(mockedState.latestPipeline).toEqual({ + id: '51', + path: 'test', + commit: { id: '123' }, + details: { status: jasmine.any(Object) }, + yamlError: undefined, + }); }); - }); - it('does not set latest pipeline if pipeline is null', () => { - mutations[types.RECEIVE_LASTEST_PIPELINE_SUCCESS](mockedState, null); - - expect(mockedState.latestPipeline).toEqual(false); + it('sets stages', () => { + expect(mockedState.stages.length).toBe(2); + expect(mockedState.stages).toEqual([ + { + id: 0, + dropdownPath: stages[0].dropdown_path, + name: stages[0].name, + status: stages[0].status, + isCollapsed: false, + isLoading: false, + jobs: [], + }, + { + id: 1, + dropdownPath: stages[1].dropdown_path, + name: stages[1].name, + status: stages[1].status, + isCollapsed: false, + isLoading: false, + jobs: [], + }, + ]); + }); }); - it('sets stages', () => { - mutations[types.RECEIVE_LASTEST_PIPELINE_SUCCESS]( - mockedState, - fullPipelinesResponse.data.pipelines[0], - ); + describe('with null', () => { + beforeEach(() => { + mutations[types.RECEIVE_LASTEST_PIPELINE_SUCCESS](mockedState, null); + }); - expect(mockedState.stages.length).toBe(2); - expect(mockedState.stages).toEqual([ - { - id: 0, - dropdownPath: stages[0].dropdown_path, - name: stages[0].name, - status: stages[0].status, - isCollapsed: false, - isLoading: false, - jobs: [], - }, - { - id: 1, - dropdownPath: stages[1].dropdown_path, - name: stages[1].name, - status: stages[1].status, - isCollapsed: false, - isLoading: false, - jobs: [], - }, - ]); + itSetsPipelineLoadingStates(); + + it('does not set latest pipeline if pipeline is null', () => { + expect(mockedState.latestPipeline).toEqual(null); + }); }); }); -- cgit v1.2.1