Merge branch '215514-passing-status-per-requirement' into 'master'

Parse and persist separate requirement states

Closes #215514

See merge request gitlab-org/gitlab!34162

doc/user/project/requirements/index.md

@@ -76,7 +76,7 @@ As soon as a requirement is reopened, it no longer appears in the **Archived** t
 
 ## Search for a requirement from the requirements list page
 
-> - Introduced in [GitLab Ultimate](https://about.gitlab.com/pricing/) 13.1.
+> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/212543) in [GitLab Ultimate](https://about.gitlab.com/pricing/) 13.1.
 
 You can search for a requirement from the list of requirements using filtered search bar (similar to
 that of Issues and Merge Requests) based on following parameters:
@@ -96,7 +96,8 @@ You can also sort requirements list by:
 
 ## Allow requirements to be satisfied from a CI job
 
-> [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/2859) in [GitLab Ultimate](https://about.gitlab.com/pricing/) 13.1.
+> - [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/2859) in [GitLab Ultimate](https://about.gitlab.com/pricing/) 13.1.
+> - [Added](https://gitlab.com/gitlab-org/gitlab/-/issues/215514) ability to specify individual requirements and their statuses in [GitLab Ultimate](https://about.gitlab.com/pricing/) 13.2.
 
 GitLab supports [requirements test
 reports](../../../ci/pipelines/job_artifacts.md#artifactsreportsrequirements-ultimate) now.
@@ -132,6 +133,32 @@ the requirement report is checked for the "all passed" record
 (`{"*":"passed"}`), and on success, it marks all existing open requirements as
 Satisfied.
 
+#### Specifying individual requirements
+
+It is possible to specify individual requirements and their statuses.
+
+If the following requirements exist:
+
+- `REQ-1` (with IID `1`)
+- `REQ-2` (with IID `2`)
+- `REQ-3` (with IID `3`)
+
+It is possible to specify that the first requirement passed, and the second failed.
+Valid values are "passed" and "failed".
+By omitting a requirement IID (in this case `REQ-3`'s IID `3`), no result is noted.
+
+```yaml
+requirements_confirmation:
+  when: manual
+  allow_failure: false
+  script:
+    - mkdir tmp
+    - echo "{\"1\":\"passed\", \"2\":\"failed\"}" > tmp/requirements.json
+  artifacts:
+    reports:
+      requirements: tmp/requirements.json
+```
+
 ### Add the manual job to CI conditionally
 
 To configure your CI to include the manual job only when there are some open

ee/app/models/requirements_management/test_report.rb

@@ -13,15 +13,50 @@ module RequirementsManagement
     validates :requirement, :state, presence: true
     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) }
 
-    def self.persist_all_requirement_reports_as_passed(build)
-      reports = []
-      timestamp = Time.current
-      build.project.requirements.opened.select(:id).find_each do |requirement|
-        reports << new(
+    class << self
+      def persist_requirement_reports(build, ci_report)
+        timestamp = Time.current
+
+        reports = if ci_report.all_passed?
+                    passed_reports_for_all_requirements(build, timestamp)
+                  else
+                    individual_reports(build, ci_report, timestamp)
+                  end
+
+        bulk_insert!(reports)
+      end
+
+      private
+
+      def passed_reports_for_all_requirements(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 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,
           # pipeline_reference will be removed:
           # https://gitlab.com/gitlab-org/gitlab/-/issues/219999
@@ -29,11 +64,9 @@ module RequirementsManagement
           build_id: build.id,
           author_id: build.user_id,
           created_at: timestamp,
-          state: :passed
+          state: state
         )
       end
-
-      bulk_insert!(reports)
     end
 
     private

ee/app/services/requirements_management/process_test_reports_service.rb

@@ -12,12 +12,13 @@ module RequirementsManagement
     end
 
     def execute
+      return unless @build.project.feature_available?(:requirements)
+      return if @build.project.requirements.empty?
       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)
 
-      RequirementsManagement::TestReport.persist_all_requirement_reports_as_passed(@build)
+      RequirementsManagement::TestReport.persist_requirement_reports(@build, report)
     end
 
     private

ee/changelogs/unreleased/215514-passing-status-per-requirement.yml

+---
+title: Persist individual requirement report results to test report
+merge_request: 34162
+author:
+type: added

ee/spec/factories/ci/builds.rb

@@ -124,7 +124,7 @@ FactoryBot.define do
 
     trait :requirements_report do
       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

ee/spec/factories/ci/job_artifacts.rb

@@ -333,13 +333,23 @@ FactoryBot.define do
       end
     end
 
-    trait :requirements do
+    trait :all_passing_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.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

ee/spec/fixtures/requirements_management/report_by_requirement.json

+{
+  "1": "passed",
+  "2": "failed",
+  "3": "passed"
+}

ee/spec/models/ci/build_spec.rb

@@ -432,7 +432,7 @@ RSpec.describe Ci::Build do
 
     context 'when there is a requirements report' 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
 
       context 'when requirements are available' do

ee/spec/models/requirements_management/test_report_spec.rb

@@ -50,26 +50,68 @@ RSpec.describe RequirementsManagement::TestReport do
     end
   end
 
-  describe '.persist_all_requirement_reports_as_passed' do
+  describe '.persist_requirement_reports' do
     let_it_be(:project) { create(: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
-      requirement = create(:requirement, state: :opened, project: project)
-      create(:requirement, state: :opened)
-      create(:requirement, state: :archived, project: project)
+    context 'if the CI report contains no entries' do
+      let(:ci_report) { Gitlab::Ci::Reports::RequirementsManagement::Report.new }
 
-      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)
-      expect(reports.size).to eq(1)
-      expect(reports.first).to have_attributes(
-        requirement: requirement,
-        author: build.user,
-        state: 'passed'
-      )
+    context 'if the CI report contains some entries' do
+      context 'and the entries are valid' do
+        let(:ci_report) do
+          Gitlab::Ci::Reports::RequirementsManagement::Report.new.tap do |report|
+            report.add_requirement('1', 'passed')
+            report.add_requirement('2', 'failed')
+            report.add_requirement('3', '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('0', 'passed')
+            report.add_requirement('1', 'nonsense')
+            report.add_requirement('2', 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

ee/spec/services/requirements_management/process_test_reports_service_spec.rb

@@ -8,45 +8,62 @@ describe RequirementsManagement::ProcessTestReportsService do
   let_it_be(:build) { create(:ee_ci_build, :requirements_report, project: project, user: user) }
 
   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 }
 
-    before do
-      stub_licensed_features(requirements: true)
-    end
-
-    context 'when user can create requirements test reports' do
+    context 'when requirements feature is available' do
       before do
-        project.add_reporter(user)
+        stub_licensed_features(requirements: true)
       end
 
-      it 'creates new test report for each open requirement' do
-        expect(RequirementsManagement::TestReport).to receive(:persist_all_requirement_reports_as_passed).with(build).and_call_original
-
-        expect { subject }.to change { RequirementsManagement::TestReport.count }.by(2)
+      context 'when there are no requirements in the project' do
+        it 'does not create any test report' do
+          expect { subject }.not_to change { RequirementsManagement::TestReport }
+        end
       end
 
-      it 'does not create test report for the same pipeline and user twice' do
-        expect { subject }.to change { RequirementsManagement::TestReport.count }.by(2)
+      context 'when there are requirements' 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) }
 
-        expect { subject }.not_to change { RequirementsManagement::TestReport }
-      end
+        context 'when user is not allowed to create requirements test reports' do
+          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
-        let(:build) { create(:ee_ci_build, project: project, user: user) }
+        context 'when user can create requirements test reports' do
+          before do
+            project.add_reporter(user)
+          end
 
-        it 'does not create any test report' do
-          expect { subject }.not_to change { RequirementsManagement::TestReport }
+          it 'creates new test report for each open requirement' do
+            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
 
-    context 'when user is not allowed to create requirements test reports' do
-      it 'raises an exception' do
-        expect { subject }.to raise_exception(Gitlab::Access::AccessDeniedError)
+    context 'when requirements feature is not available' do
+      it 'does not create any test report' do
+        expect { subject }.not_to change { RequirementsManagement::TestReport }
       end
     end
   end