Merge branch 'artifact-format-v2' into 'master'

Extend gitlab-ci.yml to request junit.xml test reports

See merge request gitlab-org/gitlab-ce!20390
This commit is contained in:
Grzegorz Bizon 2018-07-30 12:57:10 +00:00
commit 31a4f48cde
26 changed files with 506 additions and 38 deletions

View File

@ -22,9 +22,10 @@ module Ci
has_many :trace_chunks, class_name: 'Ci::BuildTraceChunk', foreign_key: :build_id
has_many :job_artifacts, class_name: 'Ci::JobArtifact', foreign_key: :job_id, dependent: :destroy, inverse_of: :job # rubocop:disable Cop/ActiveRecordDependent
has_one :job_artifacts_archive, -> { where(file_type: Ci::JobArtifact.file_types[:archive]) }, class_name: 'Ci::JobArtifact', inverse_of: :job, foreign_key: :job_id
has_one :job_artifacts_metadata, -> { where(file_type: Ci::JobArtifact.file_types[:metadata]) }, class_name: 'Ci::JobArtifact', inverse_of: :job, foreign_key: :job_id
has_one :job_artifacts_trace, -> { where(file_type: Ci::JobArtifact.file_types[:trace]) }, class_name: 'Ci::JobArtifact', inverse_of: :job, foreign_key: :job_id
Ci::JobArtifact.file_types.each do |key, value|
has_one :"job_artifacts_#{key}", -> { where(file_type: value) }, class_name: 'Ci::JobArtifact', inverse_of: :job, foreign_key: :job_id
end
has_one :metadata, class_name: 'Ci::BuildMetadata'
has_one :runner_session, class_name: 'Ci::BuildRunnerSession', validate: true, inverse_of: :build
@ -386,6 +387,10 @@ module Ci
trace.exist?
end
def has_test_reports?
job_artifacts.test_reports.any?
end
def has_old_trace?
old_trace.present?
end
@ -453,16 +458,22 @@ module Ci
save
end
def erase_test_reports!
# TODO: Use fast_destroy_all in the context of https://gitlab.com/gitlab-org/gitlab-ce/issues/35240
job_artifacts_junit&.destroy
end
def erase(opts = {})
return false unless erasable?
erase_artifacts!
erase_test_reports!
erase_trace!
update_erased!(opts[:erased_by])
end
def erasable?
complete? && (artifacts? || has_trace?)
complete? && (artifacts? || has_test_reports? || has_trace?)
end
def erased?
@ -539,10 +550,6 @@ module Ci
Gitlab::Ci::Build::Image.from_services(self)
end
def artifacts
[options[:artifacts]]
end
def cache
cache = options[:cache]

View File

@ -4,11 +4,17 @@ module Ci
include ObjectStorage::BackgroundMove
extend Gitlab::Ci::Model
TEST_REPORT_FILE_TYPES = %w[junit].freeze
DEFAULT_FILE_NAMES = { junit: 'junit.xml' }.freeze
TYPE_AND_FORMAT_PAIRS = { archive: :zip, metadata: :gzip, trace: :raw, junit: :gzip }.freeze
belongs_to :project
belongs_to :job, class_name: "Ci::Build", foreign_key: :job_id
mount_uploader :file, JobArtifactUploader
validates :file_format, presence: true, unless: :trace?, on: :create
validate :valid_file_format?, unless: :trace?, on: :create
before_save :set_size, if: :file_changed?
after_save :update_project_statistics_after_save, if: :size_changed?
after_destroy :update_project_statistics_after_destroy, unless: :project_destroyed?
@ -17,14 +23,33 @@ module Ci
scope :with_files_stored_locally, -> { where(file_store: [nil, ::JobArtifactUploader::Store::LOCAL]) }
scope :test_reports, -> do
types = self.file_types.select { |file_type| TEST_REPORT_FILE_TYPES.include?(file_type) }.values
where(file_type: types)
end
delegate :exists?, :open, to: :file
enum file_type: {
archive: 1,
metadata: 2,
trace: 3
trace: 3,
junit: 4
}
enum file_format: {
raw: 1,
zip: 2,
gzip: 3
}
def valid_file_format?
unless TYPE_AND_FORMAT_PAIRS[self.file_type&.to_sym] == self.file_format&.to_sym
errors.add(:file_format, 'Invalid file format with specified file type')
end
end
def update_file_store
# The file.object_store is set during `uploader.store!`
# which happens after object is inserted/updated

View File

@ -0,0 +1,43 @@
module Ci
class BuildRunnerPresenter < SimpleDelegator
def artifacts
return unless options[:artifacts]
list = []
list << create_archive(options[:artifacts])
list << create_reports(options[:artifacts][:reports], expire_in: options[:artifacts][:expire_in])
list.flatten.compact
end
private
def create_archive(artifacts)
return unless artifacts[:untracked] || artifacts[:paths]
{
artifact_type: :archive,
artifact_format: :zip,
name: artifacts[:name],
untracked: artifacts[:untracked],
paths: artifacts[:paths],
when: artifacts[:when],
expire_in: artifacts[:expire_in]
}
end
def create_reports(reports, expire_in:)
return unless reports&.any?
reports.map do |k, v|
{
artifact_type: k.to_sym,
artifact_format: :gzip,
name: ::Ci::JobArtifact::DEFAULT_FILE_NAMES[k.to_sym],
paths: v,
when: 'always',
expire_in: expire_in
}
end
end
end
end

View File

@ -0,0 +1,5 @@
---
title: Extend gitlab-ci.yml to request junit.xml test reports
merge_request: 20390
author:
type: added

View File

@ -0,0 +1,7 @@
class AddFileFormatToCiJobArtifacts < ActiveRecord::Migration
DOWNTIME = false
def change
add_column :ci_job_artifacts, :file_format, :integer, limit: 2
end
end

View File

@ -393,6 +393,7 @@ ActiveRecord::Schema.define(version: 20180722103201) do
t.datetime_with_timezone "expire_at"
t.string "file"
t.binary "file_sha256"
t.integer "file_format", limit: 2
end
add_index "ci_job_artifacts", ["expire_at", "job_id"], name: "index_ci_job_artifacts_on_expire_at_and_job_id", using: :btree

View File

@ -1236,7 +1236,13 @@ module API
end
class Artifacts < Grape::Entity
expose :name, :untracked, :paths, :when, :expire_in
expose :name
expose :untracked
expose :paths
expose :when
expose :expire_in
expose :artifact_type
expose :artifact_format
end
class Cache < Grape::Entity

View File

@ -109,7 +109,7 @@ module API
if result.valid?
if result.build
Gitlab::Metrics.add_event(:build_found)
present result.build, with: Entities::JobRequest::Response
present Ci::BuildRunnerPresenter.new(result.build), with: Entities::JobRequest::Response
else
Gitlab::Metrics.add_event(:build_not_found)
header 'X-GitLab-Last-Update', new_update
@ -231,6 +231,10 @@ module API
requires :id, type: Integer, desc: %q(Job's ID)
optional :token, type: String, desc: %q(Job's authentication token)
optional :expire_in, type: String, desc: %q(Specify when artifacts should expire)
optional :artifact_type, type: String, desc: %q(The type of artifact),
default: 'archive', values: Ci::JobArtifact.file_types.keys
optional :artifact_format, type: String, desc: %q(The format of artifact),
default: 'zip', values: Ci::JobArtifact.file_formats.keys
optional 'file.path', type: String, desc: %q(path to locally stored body (generated by Workhorse))
optional 'file.name', type: String, desc: %q(real filename as send in Content-Disposition (generated by Workhorse))
optional 'file.type', type: String, desc: %q(real content type as send in Content-Type (generated by Workhorse))
@ -254,29 +258,29 @@ module API
bad_request!('Missing artifacts file!') unless artifacts
file_to_large! unless artifacts.size < max_artifacts_size
bad_request!("Already uploaded") if job.job_artifacts_archive
expire_in = params['expire_in'] ||
Gitlab::CurrentSettings.current_application_settings.default_artifacts_expire_in
job.build_job_artifacts_archive(
job.job_artifacts.build(
project: job.project,
file: artifacts,
file_type: :archive,
file_type: params['artifact_type'],
file_format: params['artifact_format'],
file_sha256: artifacts.sha256,
expire_in: expire_in)
if metadata
job.build_job_artifacts_metadata(
job.job_artifacts.build(
project: job.project,
file: metadata,
file_type: :metadata,
file_format: :gzip,
file_sha256: metadata.sha256,
expire_in: expire_in)
end
if job.update(artifacts_expire_in: expire_in)
present job, with: Entities::JobRequest::Response
present Ci::BuildRunnerPresenter.new(job), with: Entities::JobRequest::Response
else
render_validation_error!(job)
end

View File

@ -6,13 +6,16 @@ module Gitlab
# Entry that represents a configuration of job artifacts.
#
class Artifacts < Node
include Configurable
include Validatable
include Attributable
ALLOWED_KEYS = %i[name untracked paths when expire_in].freeze
ALLOWED_KEYS = %i[name untracked paths reports when expire_in].freeze
attributes ALLOWED_KEYS
entry :reports, Entry::Reports, description: 'Report-type artifacts.'
validations do
validates :config, type: Hash
validates :config, allowed_keys: ALLOWED_KEYS
@ -21,6 +24,7 @@ module Gitlab
validates :name, type: String
validates :untracked, boolean: true
validates :paths, array_of_strings: true
validates :reports, type: Hash
validates :when,
inclusion: { in: %w[on_success on_failure always],
message: 'should be on_success, on_failure ' \
@ -28,6 +32,13 @@ module Gitlab
validates :expire_in, duration: true
end
end
helpers :reports
def value
@config[:reports] = reports_value if @config.key?(:reports)
@config
end
end
end
end

View File

@ -9,18 +9,7 @@ module Gitlab
include Validatable
validations do
include LegacyValidationHelpers
validate do
unless string_or_array_of_strings?(config)
errors.add(:config,
'should be a string or an array of strings')
end
end
def string_or_array_of_strings?(field)
validate_string(field) || validate_array_of_strings(field)
end
validates :config, array_of_strings_or_string: true
end
def value

View File

@ -0,0 +1,32 @@
module Gitlab
module Ci
class Config
module Entry
##
# Entry that represents a configuration of job artifacts.
#
class Reports < Node
include Validatable
include Attributable
ALLOWED_KEYS = %i[junit].freeze
attributes ALLOWED_KEYS
validations do
validates :config, type: Hash
validates :config, allowed_keys: ALLOWED_KEYS
with_options allow_nil: true do
validates :junit, array_of_strings_or_string: true
end
end
def value
@config.transform_values { |v| Array(v) }
end
end
end
end
end
end

View File

@ -130,6 +130,20 @@ module Gitlab
end
end
class ArrayOfStringsOrStringValidator < RegexpValidator
def validate_each(record, attribute, value)
unless validate_array_of_strings_or_string(value)
record.errors.add(attribute, 'should be an array of strings or a string')
end
end
private
def validate_array_of_strings_or_string(values)
validate_array_of_strings(values) || validate_string(values)
end
end
class TypeValidator < ActiveModel::EachValidator
def validate_each(record, attribute, value)
type = options[:with]

View File

@ -164,6 +164,8 @@ module Gitlab
def create_build_trace!(job, path)
File.open(path) do |stream|
# TODO: Set `file_format: :raw` after we've cleaned up legacy traces migration
# https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/20307
job.create_job_artifacts_trace!(
project: job.project,
file_type: :trace,

View File

@ -187,6 +187,13 @@ FactoryBot.define do
end
end
trait :test_reports do
after(:create) do |build|
create(:ci_job_artifact, :junit, job: build)
build.reload
end
end
trait :expired do
artifacts_expire_at 1.minute.ago
end

View File

@ -4,6 +4,7 @@ FactoryBot.define do
factory :ci_job_artifact, class: Ci::JobArtifact do
job factory: :ci_build
file_type :archive
file_format :zip
trait :remote_store do
file_store JobArtifactUploader::Store::REMOTE
@ -15,6 +16,7 @@ FactoryBot.define do
trait :archive do
file_type :archive
file_format :zip
after(:build) do |artifact, _|
artifact.file = fixture_file_upload(
@ -24,6 +26,7 @@ FactoryBot.define do
trait :metadata do
file_type :metadata
file_format :gzip
after(:build) do |artifact, _|
artifact.file = fixture_file_upload(
@ -33,6 +36,7 @@ FactoryBot.define do
trait :trace do
file_type :trace
file_format :raw
after(:build) do |artifact, evaluator|
artifact.file = fixture_file_upload(
@ -40,6 +44,16 @@ FactoryBot.define do
end
end
trait :junit do
file_type :junit
file_format :gzip
after(:build) do |artifact, evaluator|
artifact.file = fixture_file_upload(
Rails.root.join('spec/fixtures/junit.xml.gz'), 'application/x-gzip')
end
end
trait :correct_checksum do
after(:build) do |artifact, evaluator|
artifact.file_sha256 = Digest::SHA256.file(artifact.file.path).hexdigest

BIN
spec/fixtures/junit.xml.gz vendored Normal file

Binary file not shown.

View File

@ -18,6 +18,14 @@ describe Gitlab::Ci::Config::Entry::Artifacts do
expect(entry).to be_valid
end
end
context "when value includes 'reports' keyword" do
let(:config) { { paths: %w[public/], reports: { junit: 'junit.xml' } } }
it 'returns general artifact and report-type artifacts configuration' do
expect(entry.value).to eq config
end
end
end
context 'when entry value is not correct' do
@ -39,6 +47,15 @@ describe Gitlab::Ci::Config::Entry::Artifacts do
.to include 'artifacts config contains unknown keys: test'
end
end
context "when 'reports' keyword is not hash" do
let(:config) { { paths: %w[public/], reports: 'junit.xml' } }
it 'reports error' do
expect(entry.errors)
.to include 'artifacts reports should be a hash'
end
end
end
end
end

View File

@ -41,8 +41,7 @@ describe Gitlab::Ci::Config::Entry::Commands do
describe '#errors' do
it 'saves errors' do
expect(entry.errors)
.to include 'commands config should be a ' \
'string or an array of strings'
.to include 'commands config should be an array of strings or a string'
end
end
end

View File

@ -0,0 +1,53 @@
require 'spec_helper'
describe Gitlab::Ci::Config::Entry::Reports do
let(:entry) { described_class.new(config) }
describe 'validation' do
context 'when entry config value is correct' do
let(:config) { { junit: %w[junit.xml] } }
describe '#value' do
it 'returns artifacs configuration' do
expect(entry.value).to eq config
end
end
describe '#valid?' do
it 'is valid' do
expect(entry).to be_valid
end
end
context 'when value is not array' do
let(:config) { { junit: 'junit.xml' } }
it 'converts to array' do
expect(entry.value).to eq({ junit: ['junit.xml'] } )
end
end
end
context 'when entry value is not correct' do
describe '#errors' do
context 'when value of attribute is invalid' do
let(:config) { { junit: 10 } }
it 'reports error' do
expect(entry.errors)
.to include 'reports junit should be an array of strings or a string'
end
end
context 'when there is an unknown key present' do
let(:config) { { codeclimate: 'codeclimate.json' } }
it 'reports error' do
expect(entry.errors)
.to include 'reports config contains unknown keys: codeclimate'
end
end
end
end
end
end

View File

@ -514,6 +514,44 @@ describe Ci::Build do
end
end
describe '#has_test_reports?' do
subject { build.has_test_reports? }
context 'when build has a test report' do
let(:build) { create(:ci_build, :test_reports) }
it { is_expected.to be_truthy }
end
context 'when build does not have test reports' do
let(:build) { create(:ci_build, :artifacts) }
it { is_expected.to be_falsy }
end
end
describe '#erase_test_reports!' do
subject { build.erase_test_reports! }
context 'when build has a test report' do
let!(:build) { create(:ci_build, :test_reports) }
it 'removes a test report' do
subject
expect(build.has_test_reports?).to be_falsy
end
end
context 'when build does not have test reports' do
let!(:build) { create(:ci_build, :artifacts) }
it 'does not erase anything' do
expect { subject }.not_to change { Ci::JobArtifact.count }
end
end
end
describe '#has_old_trace?' do
subject { build.has_old_trace? }
@ -776,6 +814,10 @@ describe Ci::Build do
expect(build.artifacts_metadata.exists?).to be_falsy
end
it 'removes test reports' do
expect(build.job_artifacts.test_reports.count).to eq(0)
end
it 'erases build trace in trace file' do
expect(build).not_to have_trace
end
@ -807,7 +849,7 @@ describe Ci::Build do
context 'build is erasable' do
context 'new artifacts' do
let!(:build) { create(:ci_build, :trace_artifact, :success, :artifacts) }
let!(:build) { create(:ci_build, :test_reports, :trace_artifact, :success, :artifacts) }
describe '#erase' do
before do

View File

@ -15,6 +15,22 @@ describe Ci::JobArtifact do
it { is_expected.to delegate_method(:open).to(:file) }
it { is_expected.to delegate_method(:exists?).to(:file) }
describe '.test_reports' do
subject { described_class.test_reports }
context 'when there is a test report' do
let!(:artifact) { create(:ci_job_artifact, :junit) }
it { is_expected.to eq([artifact]) }
end
context 'when there are no test reports' do
let!(:artifact) { create(:ci_job_artifact, :archive) }
it { is_expected.to be_empty }
end
end
describe 'callbacks' do
subject { create(:ci_job_artifact, :archive) }
@ -87,6 +103,40 @@ describe Ci::JobArtifact do
end
end
describe 'validates file format' do
subject { artifact }
context 'when archive type with zip format' do
let(:artifact) { build(:ci_job_artifact, :archive, file_format: :zip) }
it { is_expected.to be_valid }
end
context 'when archive type with gzip format' do
let(:artifact) { build(:ci_job_artifact, :archive, file_format: :gzip) }
it { is_expected.not_to be_valid }
end
context 'when archive type without format specification' do
let(:artifact) { build(:ci_job_artifact, :archive, file_format: nil) }
it { is_expected.not_to be_valid }
end
context 'when junit type with zip format' do
let(:artifact) { build(:ci_job_artifact, :junit, file_format: :zip) }
it { is_expected.not_to be_valid }
end
context 'when junit type with gzip format' do
let(:artifact) { build(:ci_job_artifact, :junit, file_format: :gzip) }
it { is_expected.to be_valid }
end
end
describe '#file' do
subject { artifact.file }

View File

@ -0,0 +1,86 @@
require 'spec_helper'
describe Ci::BuildRunnerPresenter do
let(:presenter) { described_class.new(build) }
let(:archive) { { paths: ['sample.txt'] } }
let(:junit) { { junit: ['junit.xml'] } }
let(:archive_expectation) do
{
artifact_type: :archive,
artifact_format: :zip,
paths: archive[:paths],
untracked: archive[:untracked]
}
end
let(:junit_expectation) do
{
name: 'junit.xml',
artifact_type: :junit,
artifact_format: :gzip,
paths: ['junit.xml'],
when: 'always'
}
end
describe '#artifacts' do
context "when option contains archive-type artifacts" do
let(:build) { create(:ci_build, options: { artifacts: archive } ) }
it 'presents correct hash' do
expect(presenter.artifacts.first).to include(archive_expectation)
end
context "when untracked is specified" do
let(:archive) { { untracked: true } }
it 'presents correct hash' do
expect(presenter.artifacts.first).to include(archive_expectation)
end
end
context "when untracked and paths are missing" do
let(:archive) { { when: 'always' } }
it 'does not present hash' do
expect(presenter.artifacts).to be_empty
end
end
end
context "when option has 'junit' keyword" do
let(:build) { create(:ci_build, options: { artifacts: { reports: junit } } ) }
it 'presents correct hash' do
expect(presenter.artifacts.first).to include(junit_expectation)
end
end
context "when option has both archive and reports specification" do
let(:build) { create(:ci_build, options: { script: 'echo', artifacts: { **archive, reports: junit } } ) }
it 'presents correct hash' do
expect(presenter.artifacts.first).to include(archive_expectation)
expect(presenter.artifacts.second).to include(junit_expectation)
end
context "when archive specifies 'expire_in' keyword" do
let(:archive) { { paths: ['sample.txt'], expire_in: '3 mins 4 sec' } }
it 'inherits expire_in from archive' do
expect(presenter.artifacts.first).to include({ **archive_expectation, expire_in: '3 mins 4 sec' })
expect(presenter.artifacts.second).to include({ **junit_expectation, expire_in: '3 mins 4 sec' })
end
end
end
context "when option has no artifact keywords" do
let(:build) { create(:ci_build, :no_options) }
it 'does not present hash' do
expect(presenter.artifacts).to be_nil
end
end
end
end

View File

@ -655,13 +655,15 @@ describe API::Jobs do
end
context 'job is erasable' do
let(:job) { create(:ci_build, :trace_artifact, :artifacts, :success, project: project, pipeline: pipeline) }
let(:job) { create(:ci_build, :trace_artifact, :artifacts, :test_reports, :success, project: project, pipeline: pipeline) }
it 'erases job content' do
expect(response).to have_gitlab_http_status(201)
expect(job.job_artifacts.count).to eq(0)
expect(job.trace.exist?).to be_falsy
expect(job.artifacts_file.exists?).to be_falsy
expect(job.artifacts_metadata.exists?).to be_falsy
expect(job.has_test_reports?).to be_falsy
end
it 'updates job' do

View File

@ -424,7 +424,9 @@ describe API::Runner, :clean_gitlab_redis_shared_state do
'untracked' => false,
'paths' => %w(out/),
'when' => 'always',
'expire_in' => '7d' }]
'expire_in' => '7d',
"artifact_type" => "archive",
"artifact_format" => "zip" }]
end
let(:expected_cache) do
@ -1420,6 +1422,56 @@ describe API::Runner, :clean_gitlab_redis_shared_state do
end
end
end
context 'when artifact_type is archive' do
context 'when artifact_format is zip' do
let(:params) { { artifact_type: :archive, artifact_format: :zip } }
it 'stores junit test report' do
upload_artifacts(file_upload, headers_with_token, params)
expect(response).to have_gitlab_http_status(201)
expect(job.reload.job_artifacts_archive).not_to be_nil
end
end
context 'when artifact_format is gzip' do
let(:params) { { artifact_type: :archive, artifact_format: :gzip } }
it 'returns an error' do
upload_artifacts(file_upload, headers_with_token, params)
expect(response).to have_gitlab_http_status(400)
expect(job.reload.job_artifacts_archive).to be_nil
end
end
end
context 'when artifact_type is junit' do
context 'when artifact_format is gzip' do
let(:file_upload) { fixture_file_upload('spec/fixtures/junit.xml.gz') }
let(:params) { { artifact_type: :junit, artifact_format: :gzip } }
it 'stores junit test report' do
upload_artifacts(file_upload, headers_with_token, params)
expect(response).to have_gitlab_http_status(201)
expect(job.reload.job_artifacts_junit).not_to be_nil
end
end
context 'when artifact_format is raw' do
let(:file_upload) { fixture_file_upload('spec/fixtures/junit.xml.gz') }
let(:params) { { artifact_type: :junit, artifact_format: :raw } }
it 'returns an error' do
upload_artifacts(file_upload, headers_with_token, params)
expect(response).to have_gitlab_http_status(400)
expect(job.reload.job_artifacts_junit).to be_nil
end
end
end
end
context 'when artifacts are being stored outside of tmp path' do

View File

@ -24,7 +24,7 @@ describe Ci::RetryBuildService do
artifacts_file artifacts_metadata artifacts_size created_at
updated_at started_at finished_at queued_at erased_by
erased_at auto_canceled_by job_artifacts job_artifacts_archive
job_artifacts_metadata job_artifacts_trace].freeze
job_artifacts_metadata job_artifacts_trace job_artifacts_junit].freeze
IGNORE_ACCESSORS =
%i[type lock_version target_url base_tags trace_sections
@ -38,7 +38,7 @@ describe Ci::RetryBuildService do
let(:another_pipeline) { create(:ci_empty_pipeline, project: project) }
let(:build) do
create(:ci_build, :failed, :artifacts, :expired, :erased,
create(:ci_build, :failed, :artifacts, :test_reports, :expired, :erased,
:queued, :coverage, :tags, :allowed_to_fail, :on_tag,
:triggered, :trace_artifact, :teardown_environment,
description: 'my-job', stage: 'test', stage_id: stage.id,

View File

@ -79,7 +79,7 @@ describe Projects::UpdatePagesService do
context "for a valid job" do
before do
create(:ci_job_artifact, file: file, job: build)
create(:ci_job_artifact, file_type: :metadata, file: metadata, job: build)
create(:ci_job_artifact, file_type: :metadata, file_format: :gzip, file: metadata, job: build)
build.reload
end