Merge branch '205302-update-mr-and-approve-tests' into 'master'

Update tests for Approve feature

See merge request gitlab-org/gitlab!28044

ee/app/models/approval_merge_request_rule.rb

@@ -53,14 +53,14 @@ class ApprovalMergeRequestRule < ApplicationRecord
 
   enum report_type: {
     security: 1,
-    license_management: 2
+    license_scanning: 2
   }
 
   # Deprecated scope until code_owner column has been migrated to rule_type
   # To be removed with https://gitlab.com/gitlab-org/gitlab/issues/11834
   scope :code_owner, -> { where(code_owner: true).or(where(rule_type: :code_owner)) }
   scope :security_report, -> { report_approver.where(report_type: :security) }
-  scope :license_compliance, -> { report_approver.where(report_type: :license_management) }
+  scope :license_compliance, -> { report_approver.where(report_type: :license_scanning) }
   scope :with_head_pipeline, -> { includes(merge_request: [:head_pipeline]) }
   scope :open_merge_requests, -> { merge(MergeRequest.opened) }
   scope :for_checks_that_can_be_refreshed, -> { license_compliance.open_merge_requests.with_head_pipeline }
@@ -136,7 +136,7 @@ class ApprovalMergeRequestRule < ApplicationRecord
   def refresh_required_approvals!(project_approval_rule)
     return unless report_approver?
 
-    refresh_license_management_approvals(project_approval_rule) if license_management?
+    refresh_license_scanning_approvals(project_approval_rule) if license_scanning?
   end
 
   private
@@ -148,7 +148,7 @@ class ApprovalMergeRequestRule < ApplicationRecord
     errors.add(:approval_project_rule, 'must be for the same project')
   end
 
-  def refresh_license_management_approvals(project_approval_rule)
+  def refresh_license_scanning_approvals(project_approval_rule)
     license_report = merge_request.head_pipeline&.license_scanning_report
     return if license_report.blank?
 

ee/app/models/concerns/approval_rule_like.rb

@@ -7,7 +7,7 @@ module ApprovalRuleLike
   DEFAULT_NAME_FOR_LICENSE_REPORT = 'License-Check'
   DEFAULT_NAME_FOR_SECURITY_REPORT = 'Vulnerability-Check'
   REPORT_TYPES_BY_DEFAULT_NAME = {
-    DEFAULT_NAME_FOR_LICENSE_REPORT => :license_management,
+    DEFAULT_NAME_FOR_LICENSE_REPORT => :license_scanning,
     DEFAULT_NAME_FOR_SECURITY_REPORT => :security
   }.freeze
   APPROVALS_REQUIRED_MAX = 100

ee/app/services/security/sync_reports_to_approval_rules_service.rb

@@ -33,7 +33,7 @@ module Security
       return if report.empty? && !pipeline.complete?
       return if report.violates?(project.software_license_policies)
 
-      remove_required_approvals_for(ApprovalMergeRequestRule.report_approver.license_management)
+      remove_required_approvals_for(ApprovalMergeRequestRule.report_approver.license_scanning)
     end
 
     def sync_vulnerability_rules

ee/spec/factories/approval_rules.rb

@@ -23,9 +23,9 @@ FactoryBot.define do
       approvals_required { rand(1..ApprovalProjectRule::APPROVALS_REQUIRED_MAX) }
     end
 
-    trait :license_management do
+    trait :license_scanning do
       name { ApprovalRuleLike::DEFAULT_NAME_FOR_LICENSE_REPORT }
-      report_type { :license_management }
+      report_type { :license_scanning }
     end
   end
 
@@ -52,7 +52,7 @@ FactoryBot.define do
       security_report
     end
 
-    trait :license_management do
+    trait :license_scanning do
       name { ApprovalRuleLike::DEFAULT_NAME_FOR_LICENSE_REPORT }
       rule_type { :report_approver }
     end

ee/spec/models/approval_merge_request_rule_spec.rb

@@ -373,10 +373,10 @@ describe ApprovalMergeRequestRule do
     end
 
     context "when the rule is a `#{ApprovalRuleLike::DEFAULT_NAME_FOR_LICENSE_REPORT}` rule" do
-      subject { create(:report_approver_rule, :requires_approval, :license_management, merge_request: open_merge_request) }
+      subject { create(:report_approver_rule, :requires_approval, :license_scanning, merge_request: open_merge_request) }
 
       let(:open_merge_request) { create(:merge_request, :opened, target_project: project, source_project: project) }
-      let!(:project_approval_rule) { create(:approval_project_rule, :requires_approval, :license_management, project: project) }
+      let!(:project_approval_rule) { create(:approval_project_rule, :requires_approval, :license_scanning, project: project) }
       let(:project) { create(:project) }
       let!(:open_pipeline) { create(:ee_ci_pipeline, :success, :with_license_management_report, project: project, merge_requests_as_head_pipeline: [open_merge_request]) }
       let!(:denied_policy) { create(:software_license_policy, project: project, software_license: license, classification: :denied) }

ee/spec/models/approval_project_rule_spec.rb

@@ -106,7 +106,7 @@ describe ApprovalProjectRule do
 
   describe "validation" do
     let(:project_approval_rule) { create(:approval_project_rule) }
-    let(:license_compliance_rule) { create(:approval_project_rule, :license_management) }
+    let(:license_compliance_rule) { create(:approval_project_rule, :license_scanning) }
     let(:vulnerability_check_rule) { create(:approval_project_rule, :security) }
 
     context "when creating a new rule" do

ee/spec/services/merge_requests/sync_report_approver_approval_rules_spec.rb

@@ -46,7 +46,7 @@ describe MergeRequests::SyncReportApproverApprovalRules do
     end
 
     context "when a project has a single `#{ApprovalProjectRule::DEFAULT_NAME_FOR_LICENSE_REPORT}` approval rule" do
-      let!(:project_rule) { create(:approval_project_rule, :license_management, project: merge_request.target_project) }
+      let!(:project_rule) { create(:approval_project_rule, :license_scanning, project: merge_request.target_project) }
 
       context "when the rule has not been synchronized to the merge request yet" do
         let(:result) { merge_request.reload.approval_rules.last }
@@ -57,14 +57,14 @@ describe MergeRequests::SyncReportApproverApprovalRules do
 
         specify { expect(merge_request.reload.approval_rules.count).to be(1) }
         specify { expect(result).to be_report_approver }
-        specify { expect(result.report_type).to eq('license_management') }
+        specify { expect(result.report_type).to eq('license_scanning') }
         specify { expect(result.name).to eq(project_rule.name) }
         specify { expect(result.approval_project_rule).to eq(project_rule) }
         specify { expect(result.approvals_required).to eql(project_rule.approvals_required) }
       end
 
       context "when the rule had previously been synchronized" do
-        let!(:previous_rule) { create(:report_approver_rule, :license_management, merge_request: merge_request) }
+        let!(:previous_rule) { create(:report_approver_rule, :license_scanning, merge_request: merge_request) }
 
         before do
           service.execute
@@ -77,7 +77,7 @@ describe MergeRequests::SyncReportApproverApprovalRules do
 
     context "when a project has multiple report approval rules" do
       let!(:vulnerability_project_rule) { create(:approval_project_rule, :security_report, project: merge_request.target_project) }
-      let!(:license_compliance_project_rule) { create(:approval_project_rule, :license_management, project: merge_request.target_project) }
+      let!(:license_compliance_project_rule) { create(:approval_project_rule, :license_scanning, project: merge_request.target_project) }
 
       context "when none of the rules have been synchronized to the merge request yet" do
         let(:vulnerability_check_rule) { merge_request.reload.approval_rules.security.last }
@@ -100,13 +100,13 @@ describe MergeRequests::SyncReportApproverApprovalRules do
         specify { expect(vulnerability_check_rule.approval_project_rule).to eq(vulnerability_project_rule) }
         specify { expect(license_check_rule).to be_report_approver }
         specify { expect(license_check_rule.approvals_required).to eql(license_compliance_project_rule.approvals_required) }
-        specify { expect(license_check_rule).to be_license_management }
+        specify { expect(license_check_rule).to be_license_scanning }
         specify { expect(license_check_rule.name).to eq(license_compliance_project_rule.name) }
         specify { expect(license_check_rule.approval_project_rule).to eq(license_compliance_project_rule) }
       end
 
       context "when some of the rules have been synchronized to the merge request" do
-        let!(:previous_rule) { create(:report_approver_rule, :license_management, merge_request: merge_request) }
+        let!(:previous_rule) { create(:report_approver_rule, :license_scanning, merge_request: merge_request) }
 
         before do
           service.execute
@@ -114,7 +114,7 @@ describe MergeRequests::SyncReportApproverApprovalRules do
 
         specify { expect(merge_request.reload.approval_rules.count).to be(2) }
         specify { expect(merge_request.reload.approval_rules.security_report.count).to be(1) }
-        specify { expect(merge_request.reload.approval_rules.where(report_type: :license_management)).to match_array([previous_rule]) }
+        specify { expect(merge_request.reload.approval_rules.where(report_type: :license_scanning)).to match_array([previous_rule]) }
       end
     end
 

ee/spec/services/security/sync_reports_to_approval_rules_service_spec.rb

@@ -62,25 +62,43 @@ describe Security::SyncReportsToApprovalRulesService, '#execute' do
       end
 
       context "license compliance policy" do
-        let!(:software_license_policy) { create(:software_license_policy, :denied, project: project, software_license: denied_license) }
-        let!(:license_compliance_rule) { create(:report_approver_rule, :license_management, merge_request: merge_request, approvals_required: 1) }
-        let!(:denied_license) { create(:software_license) }
+        let!(:license_compliance_rule) { create(:report_approver_rule, :license_scanning, merge_request: merge_request, approvals_required: 1) }
 
         context "when a license violates the license compliance policy" do
-          let!(:denied_license) { create(:software_license, name: license_name) }
+          let!(:software_license_policy) { create(:software_license_policy, :denied, project: project, software_license: denied_license) }
+          let(:denied_license) { create(:software_license, name: license_name) }
+          let(:license_name) { ci_build.pipeline.license_scanning_report.license_names[0] }
+
+          context 'with a new report' do
+            let!(:ci_build) { create(:ee_ci_build, :success, :license_scanning, pipeline: pipeline, project: project) }
+
+            specify { expect { subject }.not_to change { license_compliance_rule.reload.approvals_required } }
+            specify { expect(subject[:status]).to be(:success) }
+          end
+
+          context 'with an old report' do
             let!(:ci_build) { create(:ee_ci_build, :success, :license_management, pipeline: pipeline, project: project) }
-          let!(:license_name) { ci_build.pipeline.license_scanning_report.license_names[0] }
 
             specify { expect { subject }.not_to change { license_compliance_rule.reload.approvals_required } }
             specify { expect(subject[:status]).to be(:success) }
           end
+        end
 
         context "when no licenses violate the license compliance policy" do
+          context 'with a new report' do
+            let!(:ci_build) { create(:ee_ci_build, :success, :license_scanning, pipeline: pipeline, project: project) }
+
+            specify { expect { subject }.to change { license_compliance_rule.reload.approvals_required }.from(1).to(0) }
+            specify { expect(subject[:status]).to be(:success) }
+          end
+
+          context 'with an old report' do
             let!(:ci_build) { create(:ee_ci_build, :success, :license_management, pipeline: pipeline, project: project) }
 
             specify { expect { subject }.to change { license_compliance_rule.reload.approvals_required }.from(1).to(0) }
             specify { expect(subject[:status]).to be(:success) }
           end
+        end
 
         context "when an unexpected error occurs" do
           before do
@@ -142,7 +160,7 @@ describe Security::SyncReportsToApprovalRulesService, '#execute' do
 
     context "license compliance policy" do
       let!(:software_license_policy) { create(:software_license_policy, :denied, project: project, software_license: denied_license) }
-      let!(:license_compliance_rule) { create(:report_approver_rule, :license_management, merge_request: merge_request, approvals_required: 1) }
+      let!(:license_compliance_rule) { create(:report_approver_rule, :license_scanning, merge_request: merge_request, approvals_required: 1) }
       let!(:denied_license) { create(:software_license) }
 
       specify { expect { subject }.not_to change { license_compliance_rule.reload.approvals_required } }

ee/spec/workers/refresh_license_compliance_checks_worker_spec.rb

@@ -17,13 +17,13 @@ describe RefreshLicenseComplianceChecksWorker do
       let!(:closed_merge_request) { create(:merge_request, :closed, target_project: project, source_project: project) }
 
       context "when the `#{ApprovalRuleLike::DEFAULT_NAME_FOR_LICENSE_REPORT}` approval rule is enabled" do
-        let!(:open_merge_request_approval_rule) { create(:report_approver_rule, :requires_approval, :license_management, merge_request: open_merge_request) }
-        let!(:closed_merge_request_approval_rule) { create(:report_approver_rule, :license_management, merge_request: closed_merge_request, approvals_required: 0) }
-        let!(:project_approval_rule) { create(:approval_project_rule, :requires_approval, :license_management, project: project) }
+        let!(:open_merge_request_approval_rule) { create(:report_approver_rule, :requires_approval, :license_scanning, merge_request: open_merge_request) }
+        let!(:closed_merge_request_approval_rule) { create(:report_approver_rule, :license_scanning, merge_request: closed_merge_request, approvals_required: 0) }
+        let!(:project_approval_rule) { create(:approval_project_rule, :requires_approval, :license_scanning, project: project) }
 
-        context "when a license is denied, that appears in some of the license management reports" do
-          let!(:open_pipeline) { create(:ee_ci_pipeline, :success, :with_license_management_report, project: project, merge_requests_as_head_pipeline: [open_merge_request]) }
-          let!(:closed_pipeline) { create(:ee_ci_pipeline, :success, :with_license_management_report, project: project, merge_requests_as_head_pipeline: [closed_merge_request]) }
+        context "when a license is denied, that appears in some of the license compliance reports" do
+          let!(:open_pipeline) { create(:ee_ci_pipeline, :success, :with_license_scanning_report, project: project, merge_requests_as_head_pipeline: [open_merge_request]) }
+          let!(:closed_pipeline) { create(:ee_ci_pipeline, :success, :with_license_scanning_report, project: project, merge_requests_as_head_pipeline: [closed_merge_request]) }
           let!(:denied_policy) { create(:software_license_policy, :denied, project: project, software_license: license) }
           let(:license) { create(:software_license, name: license_report.license_names[0]) }
           let(:license_report) { open_pipeline.license_scanning_report }
@@ -36,9 +36,9 @@ describe RefreshLicenseComplianceChecksWorker do
           specify { expect(closed_merge_request_approval_rule.reload.approvals_required).to be_zero }
         end
 
-        context "when none of the denied licenses appear in the most recent license management reports" do
-          let!(:open_pipeline) { create(:ee_ci_pipeline, :success, :with_license_management_report, project: project, merge_requests_as_head_pipeline: [open_merge_request]) }
-          let!(:closed_pipeline) { create(:ee_ci_pipeline, :success, :with_license_management_report, project: project, merge_requests_as_head_pipeline: [closed_merge_request]) }
+        context "when none of the denied licenses appear in the most recent license compliance reports" do
+          let!(:open_pipeline) { create(:ee_ci_pipeline, :success, :with_license_scanning_report, project: project, merge_requests_as_head_pipeline: [open_merge_request]) }
+          let!(:closed_pipeline) { create(:ee_ci_pipeline, :success, :with_license_scanning_report, project: project, merge_requests_as_head_pipeline: [closed_merge_request]) }
           let!(:denied_policy) { create(:software_license_policy, :denied, project: project, software_license: license) }
           let(:license) { create(:software_license, name: SecureRandom.uuid) }