Commit 6405b943 authored by charlieablett's avatar charlieablett

Parse and persist separate requirement states

- persist requirements report by iid and state from JSON
parent b5a642ab
...@@ -13,15 +13,48 @@ module RequirementsManagement ...@@ -13,15 +13,48 @@ module RequirementsManagement
validates :requirement, :state, presence: true validates :requirement, :state, presence: true
validate :validate_pipeline_reference validate :validate_pipeline_reference
enum state: { passed: 1 } enum state: { passed: 1, failed: 2 }
scope :for_user_build, ->(user_id, build_id) { where(author_id: user_id, build_id: build_id) } scope :for_user_build, ->(user_id, build_id) { where(author_id: user_id, build_id: build_id) }
def self.persist_all_requirement_reports_as_passed(build) class << self
reports = [] def persist_requirement_reports(build, ci_report)
timestamp = Time.current timestamp = Time.current
build.project.requirements.opened.select(:id).find_each do |requirement|
reports << new( if ci_report.all_passed?
bulk_insert!(persist_all_requirement_reports_as_passed(build, timestamp))
else
bulk_insert!(persist_individual_reports(build, ci_report, timestamp))
end
end
private
def persist_all_requirement_reports_as_passed(build, timestamp)
[].tap do |reports|
build.project.requirements.opened.select(:id).find_each do |requirement|
reports << build_report(state: :passed, requirement: requirement, build: build, timestamp: timestamp)
end
end
end
def persist_individual_reports(build, ci_report, timestamp)
[].tap do |reports|
iids = ci_report.requirements.keys
break [] if iids.empty?
build.project.requirements.opened.select(:id, :iid).where(iid: iids).each do |requirement|
# ignore anything with any unexpected state
new_state = ci_report.requirements[requirement.iid.to_s]
next unless states.key?(new_state)
reports << build_report(state: new_state, requirement: requirement, build: build, timestamp: timestamp)
end
end
end
def build_report(state:, requirement:, build:, timestamp:)
new(
requirement_id: requirement.id, requirement_id: requirement.id,
# pipeline_reference will be removed: # pipeline_reference will be removed:
# https://gitlab.com/gitlab-org/gitlab/-/issues/219999 # https://gitlab.com/gitlab-org/gitlab/-/issues/219999
...@@ -29,15 +62,11 @@ module RequirementsManagement ...@@ -29,15 +62,11 @@ module RequirementsManagement
build_id: build.id, build_id: build.id,
author_id: build.user_id, author_id: build.user_id,
created_at: timestamp, created_at: timestamp,
state: :passed state: state
) )
end end
bulk_insert!(reports)
end end
private
def validate_pipeline_reference def validate_pipeline_reference
if pipeline_id != build&.pipeline_id if pipeline_id != build&.pipeline_id
errors.add(:build, _('build pipeline reference mismatch')) errors.add(:build, _('build pipeline reference mismatch'))
......
...@@ -12,12 +12,13 @@ module RequirementsManagement ...@@ -12,12 +12,13 @@ module RequirementsManagement
end end
def execute def execute
return unless @build.project.feature_available?(:requirements)
return if @build.project.requirements.empty?
return if test_report_already_generated? return if test_report_already_generated?
return unless report.all_passed?
raise Gitlab::Access::AccessDeniedError unless can?(@build.user, :create_requirement_test_report, @build.project) raise Gitlab::Access::AccessDeniedError unless can?(@build.user, :create_requirement_test_report, @build.project)
RequirementsManagement::TestReport.persist_all_requirement_reports_as_passed(@build) RequirementsManagement::TestReport.persist_requirement_reports(@build, report)
end end
private private
......
---
title: Persist individual requirement report results to test report
merge_request: 34162
author:
type: added
...@@ -12,7 +12,7 @@ module Gitlab ...@@ -12,7 +12,7 @@ module Gitlab
end end
def add_requirement(key, value) def add_requirement(key, value)
@requirements[key] = value @requirements[key.remove('requirement_iid')] = value
end end
def all_passed? def all_passed?
......
...@@ -124,7 +124,7 @@ FactoryBot.define do ...@@ -124,7 +124,7 @@ FactoryBot.define do
trait :requirements_report do trait :requirements_report do
after(:build) do |build| after(:build) do |build|
build.job_artifacts << create(:ee_ci_job_artifact, :requirements, job: build) build.job_artifacts << create(:ee_ci_job_artifact, :all_passing_requirements, job: build)
end end
end end
end end
......
...@@ -333,13 +333,23 @@ FactoryBot.define do ...@@ -333,13 +333,23 @@ FactoryBot.define do
end end
end end
trait :requirements do trait :all_passing_requirements do
file_format { :raw } file_format { :raw }
file_type { :requirements } file_type { :requirements }
after(:build) do |artifact, _| after(:build) do |artifact, _|
artifact.file = fixture_file_upload( artifact.file = fixture_file_upload(
Rails.root.join('ee/spec/fixtures/requirements_management/report.json'), 'application/json') Rails.root.join('ee/spec/fixtures/requirements_management/all_passing_report.json'), 'application/json')
end
end
trait :individual_requirements do
file_format { :raw }
file_type { :requirements }
after(:build) do |artifact, _|
artifact.file = fixture_file_upload(
Rails.root.join('ee/spec/fixtures/requirements_management/report_by_requirement.json'), 'application/json')
end end
end end
end end
......
{
"requirement_iid1": "passed",
"requirement_iid2": "failed",
"requirement_iid3": "passed"
}
...@@ -432,7 +432,7 @@ RSpec.describe Ci::Build do ...@@ -432,7 +432,7 @@ RSpec.describe Ci::Build do
context 'when there is a requirements report' do context 'when there is a requirements report' do
before do before do
create(:ee_ci_job_artifact, :requirements, job: job, project: job.project) create(:ee_ci_job_artifact, :all_passing_requirements, job: job, project: job.project)
end end
context 'when requirements are available' do context 'when requirements are available' do
......
...@@ -50,26 +50,68 @@ RSpec.describe RequirementsManagement::TestReport do ...@@ -50,26 +50,68 @@ RSpec.describe RequirementsManagement::TestReport do
end end
end end
describe '.persist_all_requirement_reports_as_passed' do describe '.persist_requirement_reports' do
let_it_be(:project) { create(:project) } let_it_be(:project) { create(:project) }
let_it_be(:build) { create(:ee_ci_build, :requirements_report, project: project) } let_it_be(:build) { create(:ee_ci_build, :requirements_report, project: project) }
subject { described_class.persist_all_requirement_reports_as_passed(build) } subject { described_class.persist_requirement_reports(build, ci_report) }
it 'creates test report with passed status for each open requirement' do context 'if the CI report contains no entries' do
requirement = create(:requirement, state: :opened, project: project) let(:ci_report) { Gitlab::Ci::Reports::RequirementsManagement::Report.new }
create(:requirement, state: :opened)
create(:requirement, state: :archived, project: project)
expect { subject }.to change { RequirementsManagement::TestReport.count }.by(1) it 'does not create any test reports' do
expect { subject }.not_to change { RequirementsManagement::TestReport.count }
end
end
reports = RequirementsManagement::TestReport.where(pipeline: build.pipeline) context 'if the CI report contains some entries' do
expect(reports.size).to eq(1) context 'and the entries are valid' do
expect(reports.first).to have_attributes( let(:ci_report) do
requirement: requirement, Gitlab::Ci::Reports::RequirementsManagement::Report.new.tap do |report|
author: build.user, report.add_requirement('requirement_iid1', 'passed')
state: 'passed' report.add_requirement('requirement_iid2', 'failed')
) report.add_requirement('requirement_iid3', 'passed')
end
end
it 'creates test report with expected status for each open requirement' do
requirement1 = create(:requirement, state: :opened, project: project)
requirement2 = create(:requirement, state: :opened, project: project)
create(:requirement, state: :opened) # different project
create(:requirement, state: :archived, project: project) # archived
expect { subject }.to change { RequirementsManagement::TestReport.count }.by(2)
reports = RequirementsManagement::TestReport.where(pipeline: build.pipeline)
expect(reports).to match_array([
have_attributes(
requirement: requirement1,
author: build.user,
state: 'passed'),
have_attributes(requirement: requirement2,
author: build.user,
state: 'failed')
])
end
end
context 'and the entries are not valid' do
let(:ci_report) do
Gitlab::Ci::Reports::RequirementsManagement::Report.new.tap do |report|
report.add_requirement('requirement_iid0', 'passed')
report.add_requirement('requirement_iid1', 'nonsense')
report.add_requirement('requirement_iid2', nil)
end
end
it 'creates test report with expected status for each open requirement' do
# ignore requirement IIDs that appear in the test but are missing
create(:requirement, state: :opened, project: project, iid: 1)
create(:requirement, state: :opened, project: project, iid: 2)
expect { subject }.not_to change { RequirementsManagement::TestReport.count }
end
end
end end
end end
end end
...@@ -8,45 +8,62 @@ describe RequirementsManagement::ProcessTestReportsService do ...@@ -8,45 +8,62 @@ describe RequirementsManagement::ProcessTestReportsService do
let_it_be(:build) { create(:ee_ci_build, :requirements_report, project: project, user: user) } let_it_be(:build) { create(:ee_ci_build, :requirements_report, project: project, user: user) }
describe '#execute' do describe '#execute' do
let_it_be(:requirement1) { create(:requirement, state: :opened, project: project) }
let_it_be(:requirement2) { create(:requirement, state: :opened, project: project) }
let_it_be(:requirement3) { create(:requirement, state: :archived, project: project) }
subject { described_class.new(build).execute } subject { described_class.new(build).execute }
before do context 'when requirements feature is available' do
stub_licensed_features(requirements: true)
end
context 'when user can create requirements test reports' do
before do before do
project.add_reporter(user) stub_licensed_features(requirements: true)
end end
it 'creates new test report for each open requirement' do context 'when there are no requirements in the project' do
expect(RequirementsManagement::TestReport).to receive(:persist_all_requirement_reports_as_passed).with(build).and_call_original it 'does not create any test report' do
expect { subject }.not_to change { RequirementsManagement::TestReport }
expect { subject }.to change { RequirementsManagement::TestReport.count }.by(2) end
end end
it 'does not create test report for the same pipeline and user twice' do context 'when there are requirements' do
expect { subject }.to change { RequirementsManagement::TestReport.count }.by(2) let_it_be(:requirement1) { create(:requirement, state: :opened, project: project) }
let_it_be(:requirement2) { create(:requirement, state: :opened, project: project) }
let_it_be(:requirement3) { create(:requirement, state: :archived, project: project) }
expect { subject }.not_to change { RequirementsManagement::TestReport } context 'when user is not allowed to create requirements test reports' do
end it 'raises an exception' do
expect { subject }.to raise_exception(Gitlab::Access::AccessDeniedError)
end
end
context 'when build does not contain any requirements report' do context 'when user can create requirements test reports' do
let(:build) { create(:ee_ci_build, project: project, user: user) } before do
project.add_reporter(user)
end
it 'does not create any test report' do it 'creates new test report for each open requirement' do
expect { subject }.not_to change { RequirementsManagement::TestReport } expect(RequirementsManagement::TestReport).to receive(:persist_requirement_reports)
.with(build, an_instance_of(Gitlab::Ci::Reports::RequirementsManagement::Report)).and_call_original
expect { subject }.to change { RequirementsManagement::TestReport.count }.by(2)
end
it 'does not create test report for the same pipeline and user twice' do
expect { subject }.to change { RequirementsManagement::TestReport.count }.by(2)
expect { subject }.not_to change { RequirementsManagement::TestReport }
end
context 'when build does not contain any requirements report' do
let(:build) { create(:ee_ci_build, project: project, user: user) }
it 'does not create any test report' do
expect { subject }.not_to change { RequirementsManagement::TestReport }
end
end
end end
end end
end end
context 'when user is not allowed to create requirements test reports' do context 'when requirements feature is not available' do
it 'raises an exception' do it 'does not create any test report' do
expect { subject }.to raise_exception(Gitlab::Access::AccessDeniedError) expect { subject }.not_to change { RequirementsManagement::TestReport }
end end
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment