Merge branch '218725-summary-endpoint' into 'master'

Use the test report summary

See merge request gitlab-org/gitlab!36349

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

@@ -14,7 +14,7 @@ export default {
     TestSummaryTable,
   },
   computed: {
-    ...mapState(['isLoading', 'selectedSuiteIndex', 'testReports']),
+    ...mapState(['hasFullReport', 'isLoading', 'selectedSuiteIndex', 'testReports']),
     ...mapGetters(['getSelectedSuite']),
     showSuite() {
       return this.selectedSuiteIndex !== null;
@@ -28,12 +28,22 @@ export default {
     this.fetchSummary();
   },
   methods: {
-    ...mapActions(['fetchSummary', 'setSelectedSuiteIndex', 'removeSelectedSuiteIndex']),
+    ...mapActions([
+      'fetchFullReport',
+      'fetchSummary',
+      'setSelectedSuiteIndex',
+      'removeSelectedSuiteIndex',
+    ]),
     summaryBackClick() {
       this.removeSelectedSuiteIndex();
     },
     summaryTableRowClick(index) {
       this.setSelectedSuiteIndex(index);
+
+      // Fetch the full report when the user clicks to see more details
+      if (!this.hasFullReport) {
+        this.fetchFullReport();
+      }
     },
     beforeEnterTransition() {
       document.documentElement.style.overflowX = 'hidden';

app/assets/javascripts/pipelines/pipeline_details_bundle.js

@@ -122,13 +122,17 @@ const createTestDetails = () => {
   }
 
   const el = document.querySelector('#js-pipeline-tests-detail');
-  const { fullReportEndpoint, countEndpoint } = el?.dataset || {};
+  const { fullReportEndpoint, summaryEndpoint, countEndpoint } = el?.dataset || {};
 
   const testReportsStore = createTestReportsStore({
     fullReportEndpoint,
-    summaryEndpoint: countEndpoint,
+    summaryEndpoint: summaryEndpoint || countEndpoint,
+    useBuildSummaryReport: window.gon?.features?.buildReportSummary,
   });
-  createPipelinesTabs(testReportsStore);
+
+  if (!window.gon?.features?.buildReportSummary) {
+    createPipelinesTabs(testReportsStore);
+  }
 
   // eslint-disable-next-line no-new
   new Vue({

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

@@ -3,21 +3,33 @@ import * as types from './mutation_types';
 import createFlash from '~/flash';
 import { s__ } from '~/locale';
 
-export const fetchSummary = ({ state, commit }) => {
+export const fetchSummary = ({ state, commit, dispatch }) => {
+  // If we do this without the build_report_summary feature flag enabled
+  // it causes a race condition for toggleLoading and ruins the loading
+  // state in the application
+  if (state.useBuildSummaryReport) {
+    dispatch('toggleLoading');
+  }
+
   return axios
     .get(state.summaryEndpoint)
     .then(({ data }) => {
       commit(types.SET_SUMMARY, data);
 
-      // 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;
+      if (!state.useBuildSummaryReport) {
+        // 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)
+        document.querySelector('.js-test-report-badge-counter').innerHTML = data.total_count || 0;
       }
     })
     .catch(() => {
       createFlash(s__('TestReports|There was an error fetching the summary.'));
+    })
+    .finally(() => {
+      if (state.useBuildSummaryReport) {
+        dispatch('toggleLoading');
+      }
     });
 };
 

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

@@ -2,7 +2,7 @@ import * as types from './mutation_types';
 
 export default {
   [types.SET_REPORTS](state, testReports) {
-    Object.assign(state, { testReports });
+    Object.assign(state, { testReports, hasFullReport: true });
   },
 
   [types.SET_SELECTED_SUITE_INDEX](state, selectedSuiteIndex) {
@@ -10,7 +10,7 @@ export default {
   },
 
   [types.SET_SUMMARY](state, summary) {
-    Object.assign(state, { summary });
+    Object.assign(state, { testReports: { ...state.testReports, ...summary } });
   },
 
   [types.TOGGLE_LOADING](state) {

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

-export default ({ fullReportEndpoint = '', summaryEndpoint = '' }) => ({
+export default ({
+  fullReportEndpoint = '',
+  summaryEndpoint = '',
+  useBuildSummaryReport = false,
+}) => ({
   summaryEndpoint,
   fullReportEndpoint,
   testReports: {},
   selectedSuiteIndex: null,
-  summary: {},
+  hasFullReport: false,
   isLoading: false,
+  useBuildSummaryReport,
 });

app/views/projects/pipelines/_with_tabs.html.haml

@@ -24,7 +24,7 @@
       %li.js-tests-tab-link
         = link_to test_report_project_pipeline_path(@project, @pipeline), data: { target: '#js-tab-tests', action: 'test_report', toggle: 'tab' }, class: 'test-tab' do
           = s_('TestReports|Tests')
-          %span.badge.badge-pill.js-test-report-badge-counter
+          %span.badge.badge-pill.js-test-report-badge-counter= Feature.enabled?(:build_report_summary, @project) ? @pipeline.test_report_summary.total_count : ''
     = render_if_exists "projects/pipelines/tabs_holder", pipeline: @pipeline, project: @project
 
 .tab-content
@@ -86,5 +86,7 @@
       #js-pipeline-dag-vue{ data: { pipeline_data_path: dag_project_pipeline_path(@project, @pipeline), empty_svg_path: image_path('illustrations/empty-state/empty-dag-md.svg'), dag_doc_path: help_page_path('ci/yaml/README.md', anchor: 'needs')} }
 
   #js-tab-tests.tab-pane
-    #js-pipeline-tests-detail{ data: { full_report_endpoint: test_report_project_pipeline_path(@project, @pipeline, format: :json), count_endpoint: test_reports_count_project_pipeline_path(@project, @pipeline, format: :json) } }
+    #js-pipeline-tests-detail{ data: { full_report_endpoint: test_report_project_pipeline_path(@project, @pipeline, format: :json),
+      summary_endpoint: Feature.enabled?(:build_report_summary, @project) ? summary_project_pipeline_tests_path(@project, @pipeline, format: :json) : '',
+      count_endpoint: test_reports_count_project_pipeline_path(@project, @pipeline, format: :json) } }
   = render_if_exists "projects/pipelines/tabs_content", pipeline: @pipeline, project: @project

spec/features/projects/pipelines/pipeline_spec.rb

@@ -361,37 +361,68 @@ RSpec.describe 'Pipeline', :js do
     end
 
     describe 'test tabs' do
-      let(:pipeline) { create(:ci_pipeline, :with_test_reports, project: project) }
+      let(:pipeline) { create(:ci_pipeline, :with_test_reports, :with_report_results, project: project) }
 
-      before do
-        visit_pipeline
-        wait_for_requests
-      end
+      context 'with build_report_summary feature flag disabled' do
+        before do
+          stub_feature_flags(build_report_summary: false)
+          visit_pipeline
+          wait_for_requests
+        end
 
-      context 'with test reports' do
-        it 'shows badge counter in Tests tab' do
-          expect(pipeline.test_reports.total_count).to eq(4)
-          expect(page.find('.js-test-report-badge-counter').text).to eq(pipeline.test_reports.total_count.to_s)
+        context 'with test reports' do
+          it 'shows badge counter in Tests tab' do
+            expect(pipeline.test_reports.total_count).to eq(4)
+            expect(page.find('.js-test-report-badge-counter').text).to eq(pipeline.test_reports.total_count.to_s)
+          end
+
+          it 'does not call test_report.json endpoint by default', :js do
+            expect(page).to have_selector('.js-no-tests-to-show', visible: :all)
+          end
+
+          it 'does call test_report.json endpoint when tab is selected', :js do
+            find('.js-tests-tab-link').click
+            wait_for_requests
+
+            expect(page).to have_content('Test suites')
+            expect(page).to have_selector('.js-tests-detail', visible: :all)
+          end
         end
 
-        it 'does not call test_report.json endpoint by default', :js do
-          expect(page).to have_selector('.js-no-tests-to-show', visible: :all)
+        context 'without test reports' do
+          let(:pipeline) { create(:ci_pipeline, project: project) }
+
+          it 'shows zero' do
+            expect(page.find('.js-test-report-badge-counter', visible: :all).text).to eq("0")
+          end
         end
+      end
 
-        it 'does call test_report.json endpoint when tab is selected', :js do
-          find('.js-tests-tab-link').click
+      context 'with build_report_summary feature flag enabled' do
+        before do
+          visit_pipeline
           wait_for_requests
+        end
 
-          expect(page).to have_content('Test suites')
-          expect(page).to have_selector('.js-tests-detail', visible: :all)
+        context 'with test reports' do
+          it 'shows badge counter in Tests tab' do
+            expect(page.find('.js-test-report-badge-counter').text).to eq(pipeline.test_report_summary.total_count.to_s)
+          end
+
+          it 'calls summary.json endpoint', :js do
+            find('.js-tests-tab-link').click
+
+            expect(page).to have_content('Test suites')
+            expect(page).to have_selector('.js-tests-detail', visible: :all)
+          end
         end
-      end
 
-      context 'without test reports' do
-        let(:pipeline) { create(:ci_pipeline, project: project) }
+        context 'without test reports' do
+          let(:pipeline) { create(:ci_pipeline, project: project) }
 
-        it 'shows zero' do
-          expect(page.find('.js-test-report-badge-counter', visible: :all).text).to eq("0")
+          it 'shows zero' do
+            expect(page.find('.js-test-report-badge-counter', visible: :all).text).to eq("0")
+          end
         end
       end
     end

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

@@ -23,12 +23,12 @@ describe('Actions TestReports Store', () => {
     summaryEndpoint,
     testReports: {},
     selectedSuite: null,
-    summary: {},
+    useBuildSummaryReport: false,
   };
 
   beforeEach(() => {
     mock = new MockAdapter(axios);
-    state = defaultState;
+    state = { ...defaultState };
   });
 
   afterEach(() => {
@@ -40,31 +40,63 @@ describe('Actions TestReports Store', () => {
       mock.onGet(summaryEndpoint).replyOnce(200, summary, {});
     });
 
-    it('sets testReports and shows tests', done => {
-      testAction(
-        actions.fetchSummary,
-        null,
-        state,
-        [{ type: types.SET_SUMMARY, payload: summary }],
-        [],
-        done,
-      );
+    describe('when useBuildSummaryReport in state is true', () => {
+      it('sets testReports and shows tests', done => {
+        testAction(
+          actions.fetchSummary,
+          null,
+          { ...state, useBuildSummaryReport: true },
+          [{ type: types.SET_SUMMARY, payload: summary }],
+          [{ type: 'toggleLoading' }, { type: 'toggleLoading' }],
+          done,
+        );
+      });
+
+      it('should create flash on API error', done => {
+        testAction(
+          actions.fetchSummary,
+          null,
+          {
+            summaryEndpoint: null,
+            useBuildSummaryReport: true,
+          },
+          [],
+          [{ type: 'toggleLoading' }, { type: 'toggleLoading' }],
+          () => {
+            expect(createFlash).toHaveBeenCalled();
+            done();
+          },
+        );
+      });
     });
 
-    it('should create flash on API error', done => {
-      testAction(
-        actions.fetchSummary,
-        null,
-        {
-          summaryEndpoint: null,
-        },
-        [],
-        [],
-        () => {
-          expect(createFlash).toHaveBeenCalled();
-          done();
-        },
-      );
+    describe('when useBuildSummaryReport in state is false', () => {
+      it('sets testReports and shows tests', done => {
+        testAction(
+          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();
+          },
+        );
+      });
     });
   });
 
@@ -102,13 +134,13 @@ describe('Actions TestReports Store', () => {
   });
 
   describe('set selected suite index', () => {
-    const selectedSuiteIndex = 0;
-
     it('sets selectedSuiteIndex', done => {
+      const selectedSuiteIndex = 0;
+
       testAction(
         actions.setSelectedSuiteIndex,
         selectedSuiteIndex,
-        state,
+        { ...state, hasFullReport: true },
         [{ type: types.SET_SELECTED_SUITE_INDEX, payload: selectedSuiteIndex }],
         [],
         done,

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

@@ -12,10 +12,11 @@ describe('Mutations TestReports Store', () => {
     testReports: {},
     selectedSuite: null,
     isLoading: false,
+    hasFullReport: false,
   };
 
   beforeEach(() => {
-    mockState = defaultState;
+    mockState = { ...defaultState };
   });
 
   describe('set reports', () => {
@@ -24,6 +25,7 @@ describe('Mutations TestReports Store', () => {
       mutations[types.SET_REPORTS](mockState, testReports);
 
       expect(mockState.testReports).toEqual(expectedState.testReports);
+      expect(mockState.hasFullReport).toBe(true);
     });
   });
 
@@ -41,7 +43,7 @@ describe('Mutations TestReports Store', () => {
       const summary = { total_count: 1 };
       mutations[types.SET_SUMMARY](mockState, summary);
 
-      expect(mockState.summary).toEqual(summary);
+      expect(mockState.testReports).toEqual(summary);
     });
   });
 

spec/frontend/pipelines/test_reports/test_reports_spec.js

@@ -2,7 +2,8 @@ import Vuex from 'vuex';
 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';
+import TestSummary from '~/pipelines/components/test_reports/test_summary.vue';
+import TestSummaryTable from '~/pipelines/components/test_reports/test_summary_table.vue';
 import * as getters from '~/pipelines/stores/test_reports/getters';
 
 const localVue = createLocalVue();
@@ -17,19 +18,25 @@ describe('Test reports app', () => {
   const loadingSpinner = () => wrapper.find('.js-loading-spinner');
   const testsDetail = () => wrapper.find('.js-tests-detail');
   const noTestsToShow = () => wrapper.find('.js-no-tests-to-show');
+  const testSummary = () => wrapper.find(TestSummary);
+  const testSummaryTable = () => wrapper.find(TestSummaryTable);
+
+  const actionSpies = {
+    fetchFullReport: jest.fn(),
+    fetchSummary: jest.fn(),
+    setSelectedSuiteIndex: jest.fn(),
+    removeSelectedSuiteIndex: jest.fn(),
+  };
 
   const createComponent = (state = {}) => {
     store = new Vuex.Store({
       state: {
         isLoading: false,
-        selectedSuite: {},
+        selectedSuiteIndex: null,
         testReports,
         ...state,
       },
-      actions: {
-        ...actions,
-        fetchSummary: () => {},
-      },
+      actions: actionSpies,
       getters,
     });
 
@@ -43,6 +50,16 @@ describe('Test reports app', () => {
     wrapper.destroy();
   });
 
+  describe('when component is created', () => {
+    beforeEach(() => {
+      createComponent();
+    });
+
+    it('should call fetchSummary', () => {
+      expect(actionSpies.fetchSummary).toHaveBeenCalled();
+    });
+  });
+
   describe('when loading', () => {
     beforeEach(() => createComponent({ isLoading: true }));
 
@@ -72,4 +89,41 @@ describe('Test reports app', () => {
       expect(wrapper.vm.showTests).toBeTruthy();
     });
   });
+
+  describe('when a suite is clicked', () => {
+    describe('when the full test report has already been received', () => {
+      beforeEach(() => {
+        createComponent({ hasFullReport: true });
+        testSummaryTable().vm.$emit('row-click', 0);
+      });
+
+      it('should only call setSelectedSuiteIndex', () => {
+        expect(actionSpies.setSelectedSuiteIndex).toHaveBeenCalled();
+        expect(actionSpies.fetchFullReport).not.toHaveBeenCalled();
+      });
+    });
+
+    describe('when the full test report has not been received', () => {
+      beforeEach(() => {
+        createComponent({ hasFullReport: false });
+        testSummaryTable().vm.$emit('row-click', 0);
+      });
+
+      it('should call setSelectedSuiteIndex and fetchFullReport', () => {
+        expect(actionSpies.setSelectedSuiteIndex).toHaveBeenCalled();
+        expect(actionSpies.fetchFullReport).toHaveBeenCalled();
+      });
+    });
+  });
+
+  describe('when clicking back to summary', () => {
+    beforeEach(() => {
+      createComponent({ selectedSuiteIndex: 0 });
+      testSummary().vm.$emit('on-back-click');
+    });
+
+    it('should call removeSelectedSuiteIndex', () => {
+      expect(actionSpies.removeSelectedSuiteIndex).toHaveBeenCalled();
+    });
+  });
 });