Set failed status for test cases with error

Made test summary message clearer by stating we are merging
failed and error test cases in failures.

app/assets/javascripts/reports/store/utils.js

@@ -11,7 +11,7 @@ const textBuilder = results => {
   const { failed, resolved, total } = results;
 
   const failedString = failed
-    ? n__('%d failed test result', '%d failed test results', failed)
+    ? n__('%d failed/error test result', '%d failed/error test results', failed)
     : null;
   const resolvedString = resolved
     ? n__('%d fixed test result', '%d fixed test results', resolved)

changelogs/unreleased/eb-support-test-report-error-key.yml

+---
+title: MR Test Summary now shows errors as failures.
+merge_request: 17039
+author:
+type: changed

lib/gitlab/ci/parsers/test/junit.rb

@@ -49,6 +49,12 @@ module Gitlab
             if data['failure']
               status = ::Gitlab::Ci::Reports::TestCase::STATUS_FAILED
               system_output = data['failure']
+            elsif data['error']
+              # For now, as an MVC, we are grouping error test cases together
+              # with failed ones. But we will improve this further on
+              # https://gitlab.com/gitlab-org/gitlab/issues/32046.
+              status = ::Gitlab::Ci::Reports::TestCase::STATUS_FAILED
+              system_output = data['error']
             else
               status = ::Gitlab::Ci::Reports::TestCase::STATUS_SUCCESS
               system_output = nil

locale/gitlab.pot

@@ -95,8 +95,8 @@ msgid_plural "%d exporters"
 msgstr[0] ""
 msgstr[1] ""
 
-msgid "%d failed test result"
-msgid_plural "%d failed test results"
+msgid "%d failed/error test result"
+msgid_plural "%d failed/error test results"
 msgstr[0] ""
 msgstr[1] ""
 

spec/features/merge_request/user_sees_merge_widget_spec.rb

@@ -536,10 +536,10 @@ describe 'Merge request > User sees merge widget', :js do
           within(".js-reports-container") do
             click_button 'Expand'
 
-            expect(page).to have_content('Test summary contained 1 failed test result out of 2 total tests')
+            expect(page).to have_content('Test summary contained 1 failed/error test result out of 2 total tests')
             within(".js-report-section-container") do
               expect(page).to have_content('rspec found no changed test results out of 1 total test')
-              expect(page).to have_content('junit found 1 failed test result out of 1 total test')
+              expect(page).to have_content('junit found 1 failed/error test result out of 1 total test')
               expect(page).to have_content('New')
               expect(page).to have_content('addTest')
             end
@@ -581,9 +581,9 @@ describe 'Merge request > User sees merge widget', :js do
           within(".js-reports-container") do
             click_button 'Expand'
 
-            expect(page).to have_content('Test summary contained 1 failed test result out of 2 total tests')
+            expect(page).to have_content('Test summary contained 1 failed/error test result out of 2 total tests')
             within(".js-report-section-container") do
-              expect(page).to have_content('rspec found 1 failed test result out of 1 total test')
+              expect(page).to have_content('rspec found 1 failed/error test result 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')
@@ -677,10 +677,10 @@ describe 'Merge request > User sees merge widget', :js do
           within(".js-reports-container") do
             click_button 'Expand'
 
-            expect(page).to have_content('Test summary contained 20 failed test results out of 20 total tests')
+            expect(page).to have_content('Test summary contained 20 failed/error test results out of 20 total tests')
             within(".js-report-section-container") do
-              expect(page).to have_content('rspec found 10 failed test results out of 10 total tests')
-              expect(page).to have_content('junit found 10 failed test results out of 10 total tests')
+              expect(page).to have_content('rspec found 10 failed/error test results out of 10 total tests')
+              expect(page).to have_content('junit found 10 failed/error test results out of 10 total tests')
 
               expect(page).to have_content('Test#sum when a is 1 and b is 3 returns summary', count: 2)
             end

spec/frontend/reports/store/utils_spec.js

@@ -30,7 +30,9 @@ describe('Reports store utils', () => {
       const data = { failed: 3, total: 10 };
       const result = utils.summaryTextBuilder(name, data);
 
-      expect(result).toBe('Test summary contained 3 failed test results out of 10 total tests');
+      expect(result).toBe(
+        'Test summary contained 3 failed/error test results out of 10 total tests',
+      );
     });
 
     it('should render text for multiple fixed results', () => {
@@ -47,7 +49,7 @@ describe('Reports store utils', () => {
       const result = utils.summaryTextBuilder(name, data);
 
       expect(result).toBe(
-        'Test summary contained 3 failed test results and 4 fixed test results out of 10 total tests',
+        'Test summary contained 3 failed/error test results and 4 fixed test results out of 10 total tests',
       );
     });
 
@@ -57,7 +59,7 @@ describe('Reports store utils', () => {
       const result = utils.summaryTextBuilder(name, data);
 
       expect(result).toBe(
-        'Test summary contained 1 failed test result and 1 fixed test result out of 10 total tests',
+        'Test summary contained 1 failed/error test result and 1 fixed test result out of 10 total tests',
       );
     });
   });
@@ -84,7 +86,7 @@ describe('Reports store utils', () => {
       const data = { failed: 3, total: 10 };
       const result = utils.reportTextBuilder(name, data);
 
-      expect(result).toBe('Rspec found 3 failed test results out of 10 total tests');
+      expect(result).toBe('Rspec found 3 failed/error test results out of 10 total tests');
     });
 
     it('should render text for multiple fixed results', () => {
@@ -101,7 +103,7 @@ describe('Reports store utils', () => {
       const result = utils.reportTextBuilder(name, data);
 
       expect(result).toBe(
-        'Rspec found 3 failed test results and 4 fixed test results out of 10 total tests',
+        'Rspec found 3 failed/error test results and 4 fixed test results out of 10 total tests',
       );
     });
 
@@ -111,7 +113,7 @@ describe('Reports store utils', () => {
       const result = utils.reportTextBuilder(name, data);
 
       expect(result).toBe(
-        'Rspec found 1 failed test result and 1 fixed test result out of 10 total tests',
+        'Rspec found 1 failed/error test result and 1 fixed test result out of 10 total tests',
       );
     });
   });

spec/javascripts/reports/components/grouped_test_reports_app_spec.js

@@ -83,11 +83,11 @@ describe('Grouped Test Reports App', () => {
       setTimeout(() => {
         expect(vm.$el.querySelector('.gl-spinner')).toBeNull();
         expect(vm.$el.querySelector('.js-code-text').textContent.trim()).toEqual(
-          'Test summary contained 2 failed test results out of 11 total tests',
+          'Test summary contained 2 failed/error test results out of 11 total tests',
         );
 
         expect(vm.$el.textContent).toContain(
-          'rspec:pg found 2 failed test results out of 8 total tests',
+          'rspec:pg found 2 failed/error test results out of 8 total tests',
         );
 
         expect(vm.$el.textContent).toContain('New');
@@ -111,16 +111,16 @@ describe('Grouped Test Reports App', () => {
       setTimeout(() => {
         expect(vm.$el.querySelector('.gl-spinner')).toBeNull();
         expect(vm.$el.querySelector('.js-code-text').textContent.trim()).toEqual(
-          'Test summary contained 2 failed test results and 2 fixed test results out of 11 total tests',
+          'Test summary contained 2 failed/error test results and 2 fixed test results out of 11 total tests',
         );
 
         expect(vm.$el.textContent).toContain(
-          'rspec:pg found 1 failed test result and 2 fixed test results out of 8 total tests',
+          'rspec:pg found 1 failed/error test result and 2 fixed test results out of 8 total tests',
         );
 
         expect(vm.$el.textContent).toContain('New');
         expect(vm.$el.textContent).toContain(
-          ' java ant found 1 failed test result out of 3 total tests',
+          ' java ant found 1 failed/error test result out of 3 total tests',
         );
         done();
       }, 0);

spec/lib/gitlab/ci/parsers/test/junit_spec.rb

@@ -38,12 +38,14 @@ describe Gitlab::Ci::Parsers::Test::Junit do
         end
       end
 
-      context 'when there is only one <testcase> in <testsuite>' do
+      context 'when there is only one <testsuite> in <testsuites>' do
         let(:junit) do
           <<-EOF.strip_heredoc
-            <testsuite>
-              <testcase classname='Calculator' name='sumTest1' time='0.01'></testcase>
-            </testsuite>
+            <testsuites>
+              <testsuite>
+                <testcase classname='Calculator' name='sumTest1' time='0.01'></testcase>
+              </testsuite>
+            </testsuites>
           EOF
         end
 
@@ -56,23 +58,65 @@ describe Gitlab::Ci::Parsers::Test::Junit do
         end
       end
 
-      context 'when there is only one <testsuite> in <testsuites>' do
+      context 'when there is <testcase>' do
         let(:junit) do
           <<-EOF.strip_heredoc
-            <testsuites>
               <testsuite>
-                <testcase classname='Calculator' name='sumTest1' time='0.01'></testcase>
+                <testcase classname='Calculator' name='sumTest1' time='0.01'>
+                  #{testcase_content}
+                </testcase>
               </testsuite>
-            </testsuites>
           EOF
         end
 
-        it 'parses XML and adds a test case to a suite' do
+        let(:test_case) { test_cases[0] }
+
+        before do
           expect { subject }.not_to raise_error
+        end
 
-          expect(test_cases[0].classname).to eq('Calculator')
-          expect(test_cases[0].name).to eq('sumTest1')
-          expect(test_cases[0].execution_time).to eq(0.01)
+        shared_examples_for '<testcase> XML parser' do |status, output|
+          it 'parses XML and adds a test case to the suite' do
+            aggregate_failures do
+              expect(test_case.classname).to eq('Calculator')
+              expect(test_case.name).to eq('sumTest1')
+              expect(test_case.execution_time).to eq(0.01)
+              expect(test_case.status).to eq(status)
+              expect(test_case.system_output).to eq(output)
+            end
+          end
+        end
+
+        context 'and has failure' do
+          let(:testcase_content) { '<failure>Some failure</failure>' }
+
+          it_behaves_like '<testcase> XML parser',
+            ::Gitlab::Ci::Reports::TestCase::STATUS_FAILED,
+            'Some failure'
+        end
+
+        context 'and has error' do
+          let(:testcase_content) { '<error>Some error</error>' }
+
+          it_behaves_like '<testcase> XML parser',
+            ::Gitlab::Ci::Reports::TestCase::STATUS_FAILED,
+            'Some error'
+        end
+
+        context 'and has an unknown type' do
+          let(:testcase_content) { '<foo>Some foo</foo>' }
+
+          it_behaves_like '<testcase> XML parser',
+            ::Gitlab::Ci::Reports::TestCase::STATUS_SUCCESS,
+            nil
+        end
+
+        context 'and has no content' do
+          let(:testcase_content) { '' }
+
+          it_behaves_like '<testcase> XML parser',
+            ::Gitlab::Ci::Reports::TestCase::STATUS_SUCCESS,
+            nil
         end
       end