Merge branch '218725-refactor-test-report-store' into 'master'

Refactor pipeline test report store

See merge request gitlab-org/gitlab!35789

app/assets/javascripts/pipelines/components/test_reports/test_reports.vue

@@ -4,7 +4,6 @@ import { GlLoadingIcon } from '@gitlab/ui';
 import TestSuiteTable from './test_suite_table.vue';
 import TestSummary from './test_summary.vue';
 import TestSummaryTable from './test_summary_table.vue';
-import store from '~/pipelines/stores/test_reports';
 
 export default {
   name: 'TestReports',
@@ -14,7 +13,6 @@ export default {
     TestSummary,
     TestSummaryTable,
   },
-  store,
   computed: {
     ...mapState(['isLoading', 'selectedSuite', 'testReports']),
     showSuite() {
@@ -25,8 +23,11 @@ export default {
       return testSuites.length > 0;
     },
   },
+  created() {
+    this.fetchSummary();
+  },
   methods: {
-    ...mapActions(['setSelectedSuite', 'removeSelectedSuite']),
+    ...mapActions(['fetchSummary', 'setSelectedSuite', 'removeSelectedSuite']),
     summaryBackClick() {
       this.removeSelectedSuite();
     },

app/assets/javascripts/pipelines/components/test_reports/test_suite_table.vue

 <script>
 import { mapGetters } from 'vuex';
 import Icon from '~/vue_shared/components/icon.vue';
-import store from '~/pipelines/stores/test_reports';
 import { __ } from '~/locale';
 import { GlTooltipDirective } from '@gitlab/ui';
 import SmartVirtualList from '~/vue_shared/components/smart_virtual_list.vue';
@@ -15,7 +14,6 @@ export default {
   directives: {
     GlTooltip: GlTooltipDirective,
   },
-  store,
   props: {
     heading: {
       type: String,

app/assets/javascripts/pipelines/components/test_reports/test_summary_table.vue

@@ -2,7 +2,6 @@
 import { mapGetters } from 'vuex';
 import { s__ } from '~/locale';
 import { GlIcon, GlTooltipDirective } from '@gitlab/ui';
-import store from '~/pipelines/stores/test_reports';
 import SmartVirtualList from '~/vue_shared/components/smart_virtual_list.vue';
 
 export default {
@@ -14,7 +13,6 @@ export default {
   directives: {
     GlTooltip: GlTooltipDirective,
   },
-  store,
   props: {
     heading: {
       type: String,

app/assets/javascripts/pipelines/pipeline_details_bundle.js

@@ -10,8 +10,7 @@ import PipelinesMediator from './pipeline_details_mediator';
 import pipelineHeader from './components/header_component.vue';
 import eventHub from './event_hub';
 import TestReports from './components/test_reports/test_reports.vue';
-import testReportsStore from './stores/test_reports';
-import axios from '~/lib/utils/axios_utils';
+import createTestReportsStore from './stores/test_reports';
 
 Vue.use(Translate);
 
@@ -93,15 +92,11 @@ const createPipelineHeaderApp = mediator => {
   });
 };
 
-const createPipelinesTabs = dataset => {
+const createPipelinesTabs = testReportsStore => {
   const tabsElement = document.querySelector('.pipelines-tabs');
-  const testReportsEnabled =
-    window.gon && window.gon.features && window.gon.features.junitPipelineView;
-
-  if (tabsElement && testReportsEnabled) {
-    const fetchReportsAction = 'fetchReports';
-    testReportsStore.dispatch('setEndpoint', dataset.testReportEndpoint);
 
+  if (tabsElement) {
+    const fetchReportsAction = 'fetchFullReport';
     const isTestTabActive = Boolean(
       document.querySelector('.pipelines-tabs > li > a.test-tab.active'),
     );
@@ -121,28 +116,25 @@ const createPipelinesTabs = dataset => {
   }
 };
 
-const createTestDetails = detailsEndpoint => {
+const createTestDetails = (fullReportEndpoint, summaryEndpoint) => {
+  if (!window.gon?.features?.junitPipelineView) {
+    return;
+  }
+
+  const testReportsStore = createTestReportsStore({ fullReportEndpoint, summaryEndpoint });
+  createPipelinesTabs(testReportsStore);
+
   // eslint-disable-next-line no-new
   new Vue({
     el: '#js-pipeline-tests-detail',
     components: {
       TestReports,
     },
+    store: testReportsStore,
     render(createElement) {
       return createElement('test-reports');
     },
   });
-
-  axios
-    .get(detailsEndpoint)
-    .then(({ data }) => {
-      if (!data.total_count) {
-        return;
-      }
-
-      document.querySelector('.js-test-report-badge-counter').innerHTML = data.total_count;
-    })
-    .catch(() => {});
 };
 
 const createDagApp = () => {
@@ -178,7 +170,6 @@ export default () => {
 
   createPipelinesDetailApp(mediator);
   createPipelineHeaderApp(mediator);
-  createPipelinesTabs(dataset);
-  createTestDetails(dataset.testReportsCountEndpoint);
+  createTestDetails(dataset.testReportEndpoint, dataset.testReportsCountEndpoint);
   createDagApp();
 };

app/assets/javascripts/pipelines/stores/test_reports/actions.js

@@ -3,17 +3,30 @@ import * as types from './mutation_types';
 import createFlash from '~/flash';
 import { s__ } from '~/locale';
 
-export const setEndpoint = ({ commit }, data) => commit(types.SET_ENDPOINT, data);
+export const fetchSummary = ({ state, commit }) => {
+  return axios
+    .get(state.summaryEndpoint)
+    .then(({ data }) => {
+      commit(types.SET_SUMMARY, data);
 
-export const fetchReports = ({ state, commit, dispatch }) => {
+      // Set the tab counter badge to total_count
+      // This is temporary until we can server-side render that count number
+      // (see https://gitlab.com/gitlab-org/gitlab/-/issues/223134)
+      if (data.total_count !== undefined) {
+        document.querySelector('.js-test-report-badge-counter').innerHTML = data.total_count;
+      }
+    })
+    .catch(() => {
+      createFlash(s__('TestReports|There was an error fetching the summary.'));
+    });
+};
+
+export const fetchFullReport = ({ state, commit, dispatch }) => {
   dispatch('toggleLoading');
 
   return axios
-    .get(state.endpoint)
-    .then(response => {
-      const { data } = response;
-      commit(types.SET_REPORTS, data);
-    })
+    .get(state.fullReportEndpoint)
+    .then(({ data }) => commit(types.SET_REPORTS, data))
     .catch(() => {
       createFlash(s__('TestReports|There was an error fetching the test reports.'));
     })

app/assets/javascripts/pipelines/stores/test_reports/index.js

@@ -7,9 +7,10 @@ import mutations from './mutations';
 
 Vue.use(Vuex);
 
-export default new Vuex.Store({
-  actions,
-  getters,
-  mutations,
-  state,
-});
+export default initialState =>
+  new Vuex.Store({
+    actions,
+    getters,
+    mutations,
+    state: state(initialState),
+  });

app/assets/javascripts/pipelines/stores/test_reports/mutation_types.js

-export const SET_ENDPOINT = 'SET_ENDPOINT';
 export const SET_REPORTS = 'SET_REPORTS';
 export const SET_SELECTED_SUITE = 'SET_SELECTED_SUITE';
+export const SET_SUMMARY = 'SET_SUMMARY';
 export const TOGGLE_LOADING = 'TOGGLE_LOADING';

app/assets/javascripts/pipelines/stores/test_reports/mutations.js

 import * as types from './mutation_types';
 
 export default {
-  [types.SET_ENDPOINT](state, endpoint) {
-    Object.assign(state, { endpoint });
-  },
-
   [types.SET_REPORTS](state, testReports) {
     Object.assign(state, { testReports });
   },
@@ -13,6 +9,10 @@ export default {
     Object.assign(state, { selectedSuite });
   },
 
+  [types.SET_SUMMARY](state, summary) {
+    Object.assign(state, { summary });
+  },
+
   [types.TOGGLE_LOADING](state) {
     Object.assign(state, { isLoading: !state.isLoading });
   },

app/assets/javascripts/pipelines/stores/test_reports/state.js

-export default () => ({
-  endpoint: '',
+export default ({ fullReportEndpoint = '', summaryEndpoint = '' }) => ({
+  summaryEndpoint,
+  fullReportEndpoint,
   testReports: {},
   selectedSuite: {},
+  summary: {},
   isLoading: false,
 });

locale/gitlab.pot

@@ -22626,6 +22626,9 @@ msgstr ""
 msgid "TestReports|There are no tests to show."
 msgstr ""
 
+msgid "TestReports|There was an error fetching the summary."
+msgstr ""
+
 msgid "TestReports|There was an error fetching the test reports."
 msgstr ""
 

spec/features/projects/pipelines/pipeline_spec.rb

@@ -383,8 +383,8 @@ RSpec.describe 'Pipeline', :js do
       context 'without test reports' do
         let(:pipeline) { create(:ci_pipeline, project: project) }
 
-        it 'shows nothing' do
-          expect(page.find('.js-test-report-badge-counter', visible: :all).text).to eq("")
+        it 'shows zero' do
+          expect(page.find('.js-test-report-badge-counter', visible: :all).text).to eq("0")
         end
       end
     end

spec/frontend/pipelines/test_reports/stores/actions_spec.js

@@ -14,12 +14,16 @@ describe('Actions TestReports Store', () => {
   let state;
 
   const testReports = getJSONFixture('pipelines/test_report.json');
+  const summary = { total_count: 1 };
 
-  const endpoint = `${TEST_HOST}/test_reports.json`;
+  const fullReportEndpoint = `${TEST_HOST}/test_reports.json`;
+  const summaryEndpoint = `${TEST_HOST}/test_reports/summary.json`;
   const defaultState = {
-    endpoint,
+    fullReportEndpoint,
+    summaryEndpoint,
     testReports: {},
     selectedSuite: {},
+    summary: {},
   };
 
   beforeEach(() => {
@@ -31,14 +35,47 @@ describe('Actions TestReports Store', () => {
     mock.restore();
   });
 
-  describe('fetch reports', () => {
+  describe('fetch report summary', () => {
     beforeEach(() => {
-      mock.onGet(`${TEST_HOST}/test_reports.json`).replyOnce(200, testReports, {});
+      mock.onGet(summaryEndpoint).replyOnce(200, summary, {});
     });
 
     it('sets testReports and shows tests', done => {
       testAction(
-        actions.fetchReports,
+        actions.fetchSummary,
+        null,
+        state,
+        [{ type: types.SET_SUMMARY, payload: summary }],
+        [],
+        done,
+      );
+    });
+
+    it('should create flash on API error', done => {
+      testAction(
+        actions.fetchSummary,
+        null,
+        {
+          summaryEndpoint: null,
+        },
+        [],
+        [],
+        () => {
+          expect(createFlash).toHaveBeenCalled();
+          done();
+        },
+      );
+    });
+  });
+
+  describe('fetch full report', () => {
+    beforeEach(() => {
+      mock.onGet(fullReportEndpoint).replyOnce(200, testReports, {});
+    });
+
+    it('sets testReports and shows tests', done => {
+      testAction(
+        actions.fetchFullReport,
         null,
         state,
         [{ type: types.SET_REPORTS, payload: testReports }],
@@ -49,10 +86,10 @@ describe('Actions TestReports Store', () => {
 
     it('should create flash on API error', done => {
       testAction(
-        actions.fetchReports,
+        actions.fetchFullReport,
         null,
         {
-          endpoint: null,
+          fullReportEndpoint: null,
         },
         [],
         [{ type: 'toggleLoading' }, { type: 'toggleLoading' }],

spec/frontend/pipelines/test_reports/stores/mutations_spec.js

@@ -18,15 +18,6 @@ describe('Mutations TestReports Store', () => {
     mockState = defaultState;
   });
 
-  describe('set endpoint', () => {
-    it('should set endpoint', () => {
-      const expectedState = { ...mockState, endpoint: 'foo' };
-      mutations[types.SET_ENDPOINT](mockState, 'foo');
-
-      expect(mockState.endpoint).toEqual(expectedState.endpoint);
-    });
-  });
-
   describe('set reports', () => {
     it('should set testReports', () => {
       const expectedState = { ...mockState, testReports };
@@ -45,6 +36,15 @@ describe('Mutations TestReports Store', () => {
     });
   });
 
+  describe('set summary', () => {
+    it('should set summary', () => {
+      const summary = { total_count: 1 };
+      mutations[types.SET_SUMMARY](mockState, summary);
+
+      expect(mockState.summary).toEqual(summary);
+    });
+  });
+
   describe('toggle loading', () => {
     it('should set to true', () => {
       const expectedState = { ...mockState, isLoading: true };

spec/frontend/pipelines/test_reports/test_reports_spec.js

 import Vuex from 'vuex';
-import { shallowMount } from '@vue/test-utils';
+import { shallowMount, createLocalVue } from '@vue/test-utils';
 import { getJSONFixture } from 'helpers/fixtures';
 import TestReports from '~/pipelines/components/test_reports/test_reports.vue';
 import * as actions from '~/pipelines/stores/test_reports/actions';
 
+const localVue = createLocalVue();
+localVue.use(Vuex);
+
 describe('Test reports app', () => {
   let wrapper;
   let store;
@@ -22,11 +25,15 @@ describe('Test reports app', () => {
         testReports,
         ...state,
       },
-      actions,
+      actions: {
+        ...actions,
+        fetchSummary: () => {},
+      },
     });
 
     wrapper = shallowMount(TestReports, {
       store,
+      localVue,
     });
   };
 

spec/frontend/pipelines/test_reports/test_suite_table_spec.js

 import Vuex from 'vuex';
-import { shallowMount } from '@vue/test-utils';
+import { shallowMount, createLocalVue } from '@vue/test-utils';
 import { getJSONFixture } from 'helpers/fixtures';
 import SuiteTable from '~/pipelines/components/test_reports/test_suite_table.vue';
 import * as getters from '~/pipelines/stores/test_reports/getters';
 import { TestStatus } from '~/pipelines/constants';
 import skippedTestCases from './mock_data';
 
+const localVue = createLocalVue();
+localVue.use(Vuex);
+
 describe('Test reports suite table', () => {
   let wrapper;
   let store;
@@ -32,6 +35,7 @@ describe('Test reports suite table', () => {
 
     wrapper = shallowMount(SuiteTable, {
       store,
+      localVue,
     });
   };