Sum JUnit report times across multiple files

What does this MR do and why?

A job that publishes several JUnit XML files reports only the last-parsed file's execution time instead of the sum. This MR fixes the aggregation.

Ci::Build#collect_test_reports! parses every report blob into the same TestSuite, so set_total_time_from_xml assigning that file's time discarded what the previous files contributed. The parser now adds to the running total instead.

Two things to know for review:

  • Parallel jobs hit this too, and are the more common trigger. test_suite_name strips the parallel suffix, so rspec 1/5 and rspec 2/5 share one suite and overwrite each other even when each publishes a single file.
  • The REST API is affected, not just the UI. The wrong value is persisted to build_report_results.tests_duration, so durations already recorded stay wrong; this fixes new pipelines only.

@s.sebastian.geiger diagnosed this bug and proposed a patch in the issue. This MR builds on that patch. I reworked it against current master and added test coverage, and he is credited as co-author on the commit.

The issue also tracks an unrelated comma-separator parsing bug (time="1,043.988" truncated to 1.0), which is untouched here.

🛠️ with ❤️ at Siemens

References

How to set up and validate locally

  1. Seed a job carrying a 2-file JUnit report. The files declare time="10.5" and time="3.25", so the correct total is 13.75.

    Setup script — paste into rails c
    user = User.find_by(username: 'root')
    
    # Isolated demo project, owned by root. Safe to re-run.
    project = Project.find_by_full_path("#{user.namespace.full_path}/junit-multifile-demo") ||
      Projects::CreateService.new(user, name: 'junit-multifile-demo',
                                  namespace_id: user.namespace_id,
                                  visibility_level: Gitlab::VisibilityLevel::PRIVATE).execute
    
    files = ['<testsuite name="a" time="10.5"><testcase name="a1" classname="A" time="4.0"/></testsuite>',
             '<testsuite name="b" time="3.25"><testcase name="b1" classname="B" time="1.25"/></testsuite>']
    
    pipeline = Ci::Pipeline.create!(project: project, ref: 'main', sha: SecureRandom.hex(20),
                                    source: :push, user: user, status: :success)
    stage = Ci::Stage.create!(pipeline: pipeline, project: project, name: 'test', position: 0, status: :success)
    build = Ci::Build.create!(pipeline: pipeline, project: project, ci_stage: stage, ref: pipeline.ref,
                              name: 'junit-demo', stage_idx: 0, scheduling_type: :stage, status: :success,
                              user: user, started_at: 5.minutes.ago, finished_at: 4.minutes.ago)
    
    # Both files go into ONE artifact as concatenated gzip members.
    path = File.join(Dir.mktmpdir, 'junit.xml.gz')
    File.open(path, 'wb') do |out|
      files.each do |xml|
        io = StringIO.new
        gz = Zlib::GzipWriter.new(io)
        gz.write(xml)
        gz.close
        out.write(io.string)
      end
    end
    
    artifact = Ci::JobArtifact.new(job: build, project: project, file_type: :junit, file_format: :gzip)
    File.open(path) { |f| artifact.file = f }
    artifact.save!
    Ci::BuildReportResultService.new.execute(build)
    
    puts "#{Gitlab.config.gitlab.url}/#{project.full_path}/-/pipelines/#{pipeline.id}/test_report"
    puts "#{Gitlab.config.gitlab.url}/#{project.full_path}/-/jobs/#{build.id}/test_report"
  2. Open both printed URLs. The duration reads 13.75s on this branch, 3.25s before the fix.

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Gerardo Navarro

Merge request reports

Loading
Loading