diff options
| -rw-r--r-- | changelogs/unreleased/fix-junit-parser.yml | 5 | ||||
| -rw-r--r-- | doc/ci/junit_test_reports.md | 8 | ||||
| -rw-r--r-- | lib/gitlab/ci/parsers/junit.rb | 45 | ||||
| -rw-r--r-- | spec/lib/gitlab/ci/parsers/junit_spec.rb | 64 |
4 files changed, 93 insertions, 29 deletions
diff --git a/changelogs/unreleased/fix-junit-parser.yml b/changelogs/unreleased/fix-junit-parser.yml new file mode 100644 index 00000000000..e0a9ad8f210 --- /dev/null +++ b/changelogs/unreleased/fix-junit-parser.yml @@ -0,0 +1,5 @@ +--- +title: Fix edge cases of JUnitParser +merge_request: 21469 +author: +type: fixed diff --git a/doc/ci/junit_test_reports.md b/doc/ci/junit_test_reports.md index 6717dd2dad1..cf22450914c 100644 --- a/doc/ci/junit_test_reports.md +++ b/doc/ci/junit_test_reports.md @@ -139,3 +139,11 @@ java: - target/surefire-reports/TEST-*.xml - target/failsafe-reports/TEST-*.xml ``` + +## Limitations + +Currently, the following tools might not work because their XML formats are unsupported in GitLab. + +|Case|Tool|Issue| +|---|---|---| +|`<testcase>` does not have `classname` attribute|ESlint, sass-lint|https://gitlab.com/gitlab-org/gitlab-ce/issues/50964| diff --git a/lib/gitlab/ci/parsers/junit.rb b/lib/gitlab/ci/parsers/junit.rb index 3c4668ec13b..d1c136f2009 100644 --- a/lib/gitlab/ci/parsers/junit.rb +++ b/lib/gitlab/ci/parsers/junit.rb @@ -2,18 +2,14 @@ module Gitlab module Ci module Parsers class Junit - attr_reader :data - JunitParserError = Class.new(StandardError) def parse!(xml_data, test_suite) - @data = Hash.from_xml(xml_data) + root = Hash.from_xml(xml_data) - each_suite do |testcases| - testcases.each do |testcase| - test_case = create_test_case(testcase) - test_suite.add_test_case(test_case) - end + all_cases(root) do |test_case| + test_case = create_test_case(test_case) + test_suite.add_test_case(test_case) end rescue REXML::ParseException => e raise JunitParserError, "XML parsing failed: #{e.message}" @@ -23,26 +19,27 @@ module Gitlab private - def each_suite - testsuites.each do |testsuite| - yield testcases(testsuite) - end - end + def all_cases(root, parent = nil, &blk) + return unless root.present? - def testsuites - if data['testsuites'] - data['testsuites']['testsuite'] - else - [data['testsuite']] + [root].flatten.compact.map do |node| + next unless node.is_a?(Hash) + + # we allow only one top-level 'testsuites' + all_cases(node['testsuites'], root, &blk) unless parent + + # we require at least one level of testsuites or testsuite + each_case(node['testcase'], &blk) if parent + + # we allow multiple nested 'testsuite' (eg. PHPUnit) + all_cases(node['testsuite'], root, &blk) end end - def testcases(testsuite) - if testsuite['testcase'].is_a?(Array) - testsuite['testcase'] - else - [testsuite['testcase']] - end + def each_case(testcase, &blk) + return unless testcase.present? + + [testcase].flatten.compact.map(&blk) end def create_test_case(data) diff --git a/spec/lib/gitlab/ci/parsers/junit_spec.rb b/spec/lib/gitlab/ci/parsers/junit_spec.rb index f7ec86f5385..47497f06229 100644 --- a/spec/lib/gitlab/ci/parsers/junit_spec.rb +++ b/spec/lib/gitlab/ci/parsers/junit_spec.rb @@ -1,4 +1,4 @@ -require 'spec_helper' +require 'fast_spec_helper' describe Gitlab::Ci::Parsers::Junit do describe '#parse!' do @@ -8,21 +8,35 @@ describe Gitlab::Ci::Parsers::Junit do let(:test_cases) { flattened_test_cases(test_suite) } context 'when data is JUnit style XML' do - context 'when there are no test cases' do + context 'when there are no <testcases> in <testsuite>' do let(:junit) do <<-EOF.strip_heredoc <testsuite></testsuite> EOF end - it 'raises an error and does not add any test cases' do - expect { subject }.to raise_error(described_class::JunitParserError) + it 'ignores the case' do + expect { subject }.not_to raise_error + + expect(test_cases.count).to eq(0) + end + end + + context 'when there are no <testcases> in <testsuites>' do + let(:junit) do + <<-EOF.strip_heredoc + <testsuites><testsuite /></testsuites> + EOF + end + + it 'ignores the case' do + expect { subject }.not_to raise_error expect(test_cases.count).to eq(0) end end - context 'when there is a test case' do + context 'when there is only one <testcase> in <testsuite>' do let(:junit) do <<-EOF.strip_heredoc <testsuite> @@ -40,6 +54,46 @@ describe Gitlab::Ci::Parsers::Junit do end end + context 'when there is only one <testsuite> in <testsuites>' do + let(:junit) do + <<-EOF.strip_heredoc + <testsuites> + <testsuite> + <testcase classname='Calculator' name='sumTest1' time='0.01'></testcase> + </testsuite> + </testsuites> + EOF + end + + it 'parses XML and adds a test case to a suite' do + expect { subject }.not_to raise_error + + 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) + end + end + + context 'PHPUnit' do + let(:junit) do + <<-EOF.strip_heredoc + <testsuites> + <testsuite name="Project Test Suite" tests="1" assertions="1" failures="0" errors="0" time="1.376748"> + <testsuite name="XXX\\FrontEnd\\WebBundle\\Tests\\Controller\\LogControllerTest" file="/Users/mcfedr/projects/xxx/server/tests/XXX/FrontEnd/WebBundle/Tests/Controller/LogControllerTest.php" tests="1" assertions="1" failures="0" errors="0" time="1.376748"> + <testcase name="testIndexAction" class="XXX\\FrontEnd\\WebBundle\\Tests\\Controller\\LogControllerTest" file="/Users/mcfedr/projects/xxx/server/tests/XXX/FrontEnd/WebBundle/Tests/Controller/LogControllerTest.php" line="9" assertions="1" time="1.376748"/> + </testsuite> + </testsuite> + </testsuites> + EOF + end + + it 'parses XML and adds a test case to a suite' do + expect { subject }.not_to raise_error + + expect(test_cases.count).to eq(1) + end + end + context 'when there are two test cases' do let(:junit) do <<-EOF.strip_heredoc |