Merge branch '322626-use-grouped-list-for-test-report-widget' into 'master'

Add headers to test report widget

See merge request gitlab-org/gitlab!56252

app/assets/javascripts/reports/components/grouped_issues_list.vue

@@ -14,6 +14,12 @@ export default {
       required: false,
       default: '',
     },
+    nestedLevel: {
+      type: Number,
+      required: false,
+      default: 0,
+      validator: (value) => [0, 1, 2].includes(value),
+    },
     resolvedIssues: {
       type: Array,
       required: false,
@@ -58,6 +64,12 @@ export default {
 
       return groupsCount + issuesCount;
     },
+    listClasses() {
+      return {
+        'gl-pl-7': this.nestedLevel === 1,
+        'gl-pl-9': this.nestedLevel === 2,
+      };
+    },
   },
 };
 </script>
@@ -67,6 +79,7 @@ export default {
     :length="listLength"
     :remain="$options.maxShownReportItems"
     :size="$options.typicalReportItemHeight"
+    :class="listClasses"
     class="report-block-container"
     wtag="ul"
     wclass="report-block-list"

app/assets/javascripts/reports/components/grouped_test_reports_app.vue

@@ -11,8 +11,8 @@ import {
   statusIcon,
   recentFailuresTextBuilder,
 } from '../store/utils';
+import GroupedIssuesList from './grouped_issues_list.vue';
 import { componentNames } from './issue_body';
-import IssuesList from './issues_list.vue';
 import Modal from './modal.vue';
 import ReportSection from './report_section.vue';
 import SummaryRow from './summary_row.vue';
@@ -23,7 +23,7 @@ export default {
   components: {
     ReportSection,
     SummaryRow,
-    IssuesList,
+    GroupedIssuesList,
     Modal,
     GlButton,
     GlIcon,
@@ -112,10 +112,12 @@ export default {
       );
     },
     unresolvedIssues(report) {
-      return report.existing_failures.concat(report.existing_errors);
-    },
-    newIssues(report) {
-      return report.new_failures.concat(report.new_errors);
+      return [
+        ...report.new_failures,
+        ...report.new_errors,
+        ...report.existing_failures,
+        ...report.existing_errors,
+      ];
     },
     resolvedIssues(report) {
       return report.resolved_failures.concat(report.resolved_errors);
@@ -178,11 +180,10 @@ export default {
               </div>
             </template>
           </summary-row>
-          <issues-list
+          <grouped-issues-list
             v-if="shouldRenderIssuesList(report)"
             :key="`issues-list-${i}`"
             :unresolved-issues="unresolvedIssues(report)"
-            :new-issues="newIssues(report)"
             :resolved-issues="resolvedIssues(report)"
             :component="$options.componentNames.TestIssueBody"
             :nested-level="2"

app/assets/javascripts/reports/components/test_issue_body.vue

 <script>
-import { GlBadge, GlSprintf } from '@gitlab/ui';
+import { GlBadge, GlButton } from '@gitlab/ui';
 import { mapActions } from 'vuex';
+import { sprintf, n__ } from '~/locale';
+import IssueStatusIcon from '~/reports/components/issue_status_icon.vue';
+import { STATUS_NEUTRAL } from '../constants';
 
 export default {
   name: 'TestIssueBody',
   components: {
     GlBadge,
-    GlSprintf,
+    GlButton,
+    IssueStatusIcon,
   },
   props: {
     issue: {
       type: Object,
       required: true,
     },
-    // failed || success
-    status: {
-      type: String,
-      required: true,
-    },
-    isNew: {
-      type: Boolean,
-      required: false,
-      default: false,
-    },
   },
   computed: {
+    recentFailureMessage() {
+      return sprintf(
+        n__(
+          'Reports|Failed %{count} time in %{base_branch} in the last 14 days',
+          'Reports|Failed %{count} times in %{base_branch} in the last 14 days',
+          this.issue.recent_failures.count,
+        ),
+        this.issue.recent_failures,
+      );
+    },
     showRecentFailures() {
       return this.issue.recent_failures?.count && this.issue.recent_failures?.base_branch;
     },
+    status() {
+      return this.issue.status || STATUS_NEUTRAL;
+    },
   },
   methods: {
     ...mapActions(['openModal']),
@@ -35,30 +42,23 @@ export default {
 };
 </script>
 <template>
-  <div class="report-block-list-issue-description gl-mt-2 gl-mb-2">
-    <div class="report-block-list-issue-description-text" data-testid="test-issue-body-description">
-      <button
-        type="button"
-        class="btn-link btn-blank text-left break-link vulnerability-name-button"
-        @click="openModal({ issue })"
-      >
-        <gl-badge v-if="isNew" variant="danger" class="gl-mr-2">{{ s__('New') }}</gl-badge>
-        <gl-badge v-if="showRecentFailures" variant="warning" class="gl-mr-2">
-          <gl-sprintf
-            :message="
-              n__(
-                'Reports|Failed %{count} time in %{base_branch} in the last 14 days',
-                'Reports|Failed %{count} times in %{base_branch} in the last 14 days',
-                issue.recent_failures.count,
-              )
-            "
-          >
-            <template #count>{{ issue.recent_failures.count }}</template>
-            <template #base_branch>{{ issue.recent_failures.base_branch }}</template>
-          </gl-sprintf>
-        </gl-badge>
-        {{ issue.name }}
-      </button>
-    </div>
+  <div class="gl-display-flex gl-mt-2 gl-mb-2">
+    <issue-status-icon :status="status" :status-icon-size="24" class="gl-mr-3" />
+    <gl-badge
+      v-if="showRecentFailures"
+      variant="warning"
+      class="gl-mr-2"
+      data-testid="test-issue-body-recent-failures"
+    >
+      {{ recentFailureMessage }}
+    </gl-badge>
+    <gl-button
+      button-text-classes="gl-white-space-normal! gl-word-break-all gl-text-left"
+      variant="link"
+      data-testid="test-issue-body-description"
+      @click="openModal({ issue })"
+    >
+      {{ issue.name }}
+    </gl-button>
   </div>
 </template>

changelogs/unreleased/322626-use-grouped-list-for-test-report-widget.yml

+---
+title: Add section headers for the test report widget on the merge request page
+merge_request: 56252
+author:
+type: added

spec/features/merge_request/user_sees_merge_widget_spec.rb

@@ -651,7 +651,6 @@ RSpec.describe 'Merge request > User sees merge widget', :js do
             within(".js-report-section-container") do
               expect(page).to have_content('rspec found 1 failed out of 1 total test')
               expect(page).to have_content('junit found no changed test results out of 1 total test')
-              expect(page).not_to have_content('New')
               expect(page).to have_content('Test#sum when a is 1 and b is 3 returns summary')
             end
           end
@@ -792,7 +791,6 @@ RSpec.describe 'Merge request > User sees merge widget', :js do
             within(".js-report-section-container") do
               expect(page).to have_content('rspec found 1 error out of 1 total test')
               expect(page).to have_content('junit found no changed test results out of 1 total test')
-              expect(page).not_to have_content('New')
               expect(page).to have_content('Test#sum when a is 4 and b is 4 returns summary')
             end
           end

spec/frontend/reports/components/grouped_test_reports_app_spec.js

@@ -43,7 +43,11 @@ describe('Grouped test reports app', () => {
   const findExpandButton = () => wrapper.find('[data-testid="report-section-expand-button"]');
   const findFullTestReportLink = () => wrapper.find('[data-testid="group-test-reports-full-link"]');
   const findSummaryDescription = () => wrapper.find('[data-testid="summary-row-description"]');
+  const findIssueListUnresolvedHeading = () => wrapper.find('[data-testid="unresolvedHeading"]');
+  const findIssueListResolvedHeading = () => wrapper.find('[data-testid="resolvedHeading"]');
   const findIssueDescription = () => wrapper.find('[data-testid="test-issue-body-description"]');
+  const findIssueRecentFailures = () =>
+    wrapper.find('[data-testid="test-issue-body-recent-failures"]');
   const findAllIssueDescriptions = () =>
     wrapper.findAll('[data-testid="test-issue-body-description"]');
 
@@ -133,6 +137,10 @@ describe('Grouped test reports app', () => {
       mountComponent();
     });
 
+    it('renders New heading', () => {
+      expect(findIssueListUnresolvedHeading().text()).toBe('New');
+    });
+
     it('renders failed summary text', () => {
       expect(findHeader().text()).toBe('Test summary contained 2 failed out of 11 total tests');
     });
@@ -144,7 +152,6 @@ describe('Grouped test reports app', () => {
     });
 
     it('renders failed issue in list', () => {
-      expect(findIssueDescription().text()).toContain('New');
       expect(findIssueDescription().text()).toContain(
         'Test#sum when a is 1 and b is 2 returns summary',
       );
@@ -157,6 +164,10 @@ describe('Grouped test reports app', () => {
       mountComponent();
     });
 
+    it('renders New heading', () => {
+      expect(findIssueListUnresolvedHeading().text()).toBe('New');
+    });
+
     it('renders error summary text', () => {
       expect(findHeader().text()).toBe('Test summary contained 2 errors out of 11 total tests');
     });
@@ -168,7 +179,6 @@ describe('Grouped test reports app', () => {
     });
 
     it('renders error issue in list', () => {
-      expect(findIssueDescription().text()).toContain('New');
       expect(findIssueDescription().text()).toContain(
         'Test#sum when a is 1 and b is 2 returns summary',
       );
@@ -181,6 +191,11 @@ describe('Grouped test reports app', () => {
       mountComponent();
     });
 
+    it('renders New and Fixed headings', () => {
+      expect(findIssueListUnresolvedHeading().text()).toBe('New');
+      expect(findIssueListResolvedHeading().text()).toBe('Fixed');
+    });
+
     it('renders summary text', () => {
       expect(findHeader().text()).toBe(
         'Test summary contained 2 failed and 2 fixed test results out of 11 total tests',
@@ -194,7 +209,6 @@ describe('Grouped test reports app', () => {
     });
 
     it('renders failed issue in list', () => {
-      expect(findIssueDescription().text()).toContain('New');
       expect(findIssueDescription().text()).toContain(
         'Test#subtract when a is 2 and b is 1 returns correct result',
       );
@@ -207,6 +221,10 @@ describe('Grouped test reports app', () => {
       mountComponent();
     });
 
+    it('renders Fixed heading', () => {
+      expect(findIssueListResolvedHeading().text()).toBe('Fixed');
+    });
+
     it('renders summary text', () => {
       expect(findHeader().text()).toBe(
         'Test summary contained 4 fixed test results out of 11 total tests',
@@ -252,7 +270,7 @@ describe('Grouped test reports app', () => {
       });
 
       it('renders the recent failures count on the test case', () => {
-        expect(findIssueDescription().text()).toContain(
+        expect(findIssueRecentFailures().text()).toBe(
           'Failed 8 times in master in the last 14 days',
         );
       });

spec/frontend/reports/components/test_issue_body_spec.js

-import Vue from 'vue';
-import { trimText } from 'helpers/text_helper';
-import { mountComponentWithStore } from 'helpers/vue_mount_component_helper';
-import component from '~/reports/components/test_issue_body.vue';
-import createStore from '~/reports/store';
-import { issue } from '../mock_data/mock_data';
-
-describe('Test Issue body', () => {
-  let vm;
-  const Component = Vue.extend(component);
-  const store = createStore();
-
-  const commonProps = {
-    issue,
-    status: 'failed',
-  };
+import { GlBadge, GlButton } from '@gitlab/ui';
+import { shallowMount, createLocalVue } from '@vue/test-utils';
+import Vuex from 'vuex';
+import { extendedWrapper } from 'helpers/vue_test_utils_helper';
+import IssueStatusIcon from '~/reports/components/issue_status_icon.vue';
+import TestIssueBody from '~/reports/components/test_issue_body.vue';
+import { failedIssue, successIssue } from '../mock_data/mock_data';
 
-  afterEach(() => {
-    vm.$destroy();
-  });
+const localVue = createLocalVue();
+localVue.use(Vuex);
 
-  describe('on click', () => {
-    it('calls openModal action', () => {
-      vm = mountComponentWithStore(Component, {
-        store,
-        props: commonProps,
-      });
+describe('Test issue body', () => {
+  let wrapper;
+  let store;
 
-      jest.spyOn(vm, 'openModal').mockImplementation(() => {});
+  const findDescription = () => wrapper.findByTestId('test-issue-body-description');
+  const findStatusIcon = () => wrapper.findComponent(IssueStatusIcon);
+  const findBadge = () => wrapper.findComponent(GlBadge);
 
-      vm.$el.querySelector('button').click();
+  const actionSpies = {
+    openModal: jest.fn(),
+  };
 
-      expect(vm.openModal).toHaveBeenCalledWith({
-        issue: commonProps.issue,
-      });
+  const createComponent = ({ issue = failedIssue } = {}) => {
+    store = new Vuex.Store({
+      actions: actionSpies,
     });
+
+    wrapper = extendedWrapper(
+      shallowMount(TestIssueBody, {
+        store,
+        localVue,
+        propsData: {
+          issue,
+        },
+        stubs: {
+          GlBadge,
+          GlButton,
+          IssueStatusIcon,
+        },
+      }),
+    );
+  };
+
+  afterEach(() => {
+    wrapper.destroy();
   });
 
-  describe('is new', () => {
+  describe('when issue has failed status', () => {
     beforeEach(() => {
-      vm = mountComponentWithStore(Component, {
-        store,
-        props: { ...commonProps, isNew: true },
-      });
+      createComponent();
     });
 
     it('renders issue name', () => {
-      expect(vm.$el.textContent).toContain(commonProps.issue.name);
+      expect(findDescription().text()).toBe(failedIssue.name);
+    });
+
+    it('renders failed status icon', () => {
+      expect(findStatusIcon().props('status')).toBe('failed');
     });
 
-    it('renders new badge', () => {
-      expect(trimText(vm.$el.querySelector('.badge').textContent)).toEqual('New');
+    describe('when issue has recent failures', () => {
+      it('renders recent failures badge', () => {
+        expect(findBadge().exists()).toBe(true);
+      });
     });
   });
 
-  describe('not new', () => {
+  describe('when issue has success status', () => {
     beforeEach(() => {
-      vm = mountComponentWithStore(Component, {
-        store,
-        props: commonProps,
-      });
+      createComponent({ issue: successIssue });
+    });
+
+    it('does not render recent failures', () => {
+      expect(findBadge().exists()).toBe(false);
     });
 
     it('renders issue name', () => {
-      expect(vm.$el.textContent).toContain(commonProps.issue.name);
+      expect(findDescription().text()).toBe(successIssue.name);
+    });
+
+    it('renders success status icon', () => {
+      expect(findStatusIcon().props('status')).toBe('success');
     });
+  });
 
-    it('does not renders new badge', () => {
-      expect(vm.$el.querySelector('.badge')).toEqual(null);
+  describe('when clicking on an issue', () => {
+    it('calls openModal action', () => {
+      createComponent();
+      wrapper.findComponent(GlButton).trigger('click');
+
+      expect(actionSpies.openModal).toHaveBeenCalledWith(expect.any(Object), {
+        issue: failedIssue,
+      });
     });
   });
 });

spec/frontend/reports/mock_data/mock_data.js

-export const issue = {
+export const failedIssue = {
   result: 'failure',
   name: 'Test#sum when a is 1 and b is 2 returns summary',
   execution_time: 0.009411,
+  status: 'failed',
   system_output:
     "Failure/Error: is_expected.to eq(3)\n\n  expected: 3\n       got: -1\n\n  (compared using ==)\n./spec/test_spec.rb:12:in `block (4 levels) in \u003ctop (required)\u003e'",
+  recent_failures: {
+    count: 3,
+    base_branch: 'master',
+  },
+};
+
+export const successIssue = {
+  result: 'success',
+  name: 'Test#sum when a is 1 and b is 2 returns summary',
+  execution_time: 0.009411,
+  status: 'success',
+  system_output: null,
+  recent_failures: null,
 };
 
 export const failedReport = {

spec/frontend/reports/store/mutations_spec.js

 import * as types from '~/reports/store/mutation_types';
 import mutations from '~/reports/store/mutations';
 import state from '~/reports/store/state';
-import { issue } from '../mock_data/mock_data';
+import { failedIssue } from '../mock_data/mock_data';
 
 describe('Reports Store Mutations', () => {
   let stateCopy;
@@ -115,17 +115,17 @@ describe('Reports Store Mutations', () => {
   describe('SET_ISSUE_MODAL_DATA', () => {
     beforeEach(() => {
       mutations[types.SET_ISSUE_MODAL_DATA](stateCopy, {
-        issue,
+        issue: failedIssue,
       });
     });
 
     it('should set modal title', () => {
-      expect(stateCopy.modal.title).toEqual(issue.name);
+      expect(stateCopy.modal.title).toEqual(failedIssue.name);
     });
 
     it('should set modal data', () => {
-      expect(stateCopy.modal.data.execution_time.value).toEqual(issue.execution_time);
-      expect(stateCopy.modal.data.system_output.value).toEqual(issue.system_output);
+      expect(stateCopy.modal.data.execution_time.value).toEqual(failedIssue.execution_time);
+      expect(stateCopy.modal.data.system_output.value).toEqual(failedIssue.system_output);
     });
 
     it('should open modal', () => {
@@ -136,7 +136,7 @@ describe('Reports Store Mutations', () => {
   describe('RESET_ISSUE_MODAL_DATA', () => {
     beforeEach(() => {
       mutations[types.SET_ISSUE_MODAL_DATA](stateCopy, {
-        issue,
+        issue: failedIssue,
       });
 
       mutations[types.RESET_ISSUE_MODAL_DATA](stateCopy);