Commit 36e541fc authored by Tetiana Chupryna's avatar Tetiana Chupryna Committed by Rémy Coutable

Update tests for Approve feature

parent 972e5e68
...@@ -53,14 +53,14 @@ class ApprovalMergeRequestRule < ApplicationRecord ...@@ -53,14 +53,14 @@ class ApprovalMergeRequestRule < ApplicationRecord
enum report_type: { enum report_type: {
security: 1, security: 1,
license_management: 2 license_scanning: 2
} }
# Deprecated scope until code_owner column has been migrated to rule_type # Deprecated scope until code_owner column has been migrated to rule_type
# To be removed with https://gitlab.com/gitlab-org/gitlab/issues/11834 # 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 :code_owner, -> { where(code_owner: true).or(where(rule_type: :code_owner)) }
scope :security_report, -> { report_approver.where(report_type: :security) } 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 :with_head_pipeline, -> { includes(merge_request: [:head_pipeline]) }
scope :open_merge_requests, -> { merge(MergeRequest.opened) } scope :open_merge_requests, -> { merge(MergeRequest.opened) }
scope :for_checks_that_can_be_refreshed, -> { license_compliance.open_merge_requests.with_head_pipeline } scope :for_checks_that_can_be_refreshed, -> { license_compliance.open_merge_requests.with_head_pipeline }
...@@ -136,7 +136,7 @@ class ApprovalMergeRequestRule < ApplicationRecord ...@@ -136,7 +136,7 @@ class ApprovalMergeRequestRule < ApplicationRecord
def refresh_required_approvals!(project_approval_rule) def refresh_required_approvals!(project_approval_rule)
return unless report_approver? 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 end
private private
...@@ -148,7 +148,7 @@ class ApprovalMergeRequestRule < ApplicationRecord ...@@ -148,7 +148,7 @@ class ApprovalMergeRequestRule < ApplicationRecord
errors.add(:approval_project_rule, 'must be for the same project') errors.add(:approval_project_rule, 'must be for the same project')
end 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 license_report = merge_request.head_pipeline&.license_scanning_report
return if license_report.blank? return if license_report.blank?
......
...@@ -7,7 +7,7 @@ module ApprovalRuleLike ...@@ -7,7 +7,7 @@ module ApprovalRuleLike
DEFAULT_NAME_FOR_LICENSE_REPORT = 'License-Check' DEFAULT_NAME_FOR_LICENSE_REPORT = 'License-Check'
DEFAULT_NAME_FOR_SECURITY_REPORT = 'Vulnerability-Check' DEFAULT_NAME_FOR_SECURITY_REPORT = 'Vulnerability-Check'
REPORT_TYPES_BY_DEFAULT_NAME = { 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 DEFAULT_NAME_FOR_SECURITY_REPORT => :security
}.freeze }.freeze
APPROVALS_REQUIRED_MAX = 100 APPROVALS_REQUIRED_MAX = 100
......
...@@ -33,7 +33,7 @@ module Security ...@@ -33,7 +33,7 @@ module Security
return if report.empty? && !pipeline.complete? return if report.empty? && !pipeline.complete?
return if report.violates?(project.software_license_policies) 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 end
def sync_vulnerability_rules def sync_vulnerability_rules
......
...@@ -23,9 +23,9 @@ FactoryBot.define do ...@@ -23,9 +23,9 @@ FactoryBot.define do
approvals_required { rand(1..ApprovalProjectRule::APPROVALS_REQUIRED_MAX) } approvals_required { rand(1..ApprovalProjectRule::APPROVALS_REQUIRED_MAX) }
end end
trait :license_management do trait :license_scanning do
name { ApprovalRuleLike::DEFAULT_NAME_FOR_LICENSE_REPORT } name { ApprovalRuleLike::DEFAULT_NAME_FOR_LICENSE_REPORT }
report_type { :license_management } report_type { :license_scanning }
end end
end end
...@@ -52,7 +52,7 @@ FactoryBot.define do ...@@ -52,7 +52,7 @@ FactoryBot.define do
security_report security_report
end end
trait :license_management do trait :license_scanning do
name { ApprovalRuleLike::DEFAULT_NAME_FOR_LICENSE_REPORT } name { ApprovalRuleLike::DEFAULT_NAME_FOR_LICENSE_REPORT }
rule_type { :report_approver } rule_type { :report_approver }
end end
......
...@@ -373,10 +373,10 @@ describe ApprovalMergeRequestRule do ...@@ -373,10 +373,10 @@ describe ApprovalMergeRequestRule do
end end
context "when the rule is a `#{ApprovalRuleLike::DEFAULT_NAME_FOR_LICENSE_REPORT}` rule" do 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(: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(: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!(: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) } let!(:denied_policy) { create(:software_license_policy, project: project, software_license: license, classification: :denied) }
......
...@@ -106,7 +106,7 @@ describe ApprovalProjectRule do ...@@ -106,7 +106,7 @@ describe ApprovalProjectRule do
describe "validation" do describe "validation" do
let(:project_approval_rule) { create(:approval_project_rule) } 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) } let(:vulnerability_check_rule) { create(:approval_project_rule, :security) }
context "when creating a new rule" do context "when creating a new rule" do
......
...@@ -46,7 +46,7 @@ describe MergeRequests::SyncReportApproverApprovalRules do ...@@ -46,7 +46,7 @@ describe MergeRequests::SyncReportApproverApprovalRules do
end end
context "when a project has a single `#{ApprovalProjectRule::DEFAULT_NAME_FOR_LICENSE_REPORT}` approval rule" do 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 context "when the rule has not been synchronized to the merge request yet" do
let(:result) { merge_request.reload.approval_rules.last } let(:result) { merge_request.reload.approval_rules.last }
...@@ -57,14 +57,14 @@ describe MergeRequests::SyncReportApproverApprovalRules do ...@@ -57,14 +57,14 @@ describe MergeRequests::SyncReportApproverApprovalRules do
specify { expect(merge_request.reload.approval_rules.count).to be(1) } specify { expect(merge_request.reload.approval_rules.count).to be(1) }
specify { expect(result).to be_report_approver } 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.name).to eq(project_rule.name) }
specify { expect(result.approval_project_rule).to eq(project_rule) } specify { expect(result.approval_project_rule).to eq(project_rule) }
specify { expect(result.approvals_required).to eql(project_rule.approvals_required) } specify { expect(result.approvals_required).to eql(project_rule.approvals_required) }
end end
context "when the rule had previously been synchronized" do 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 before do
service.execute service.execute
...@@ -77,7 +77,7 @@ describe MergeRequests::SyncReportApproverApprovalRules do ...@@ -77,7 +77,7 @@ describe MergeRequests::SyncReportApproverApprovalRules do
context "when a project has multiple report approval rules" 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!(: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 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 } let(:vulnerability_check_rule) { merge_request.reload.approval_rules.security.last }
...@@ -100,13 +100,13 @@ describe MergeRequests::SyncReportApproverApprovalRules do ...@@ -100,13 +100,13 @@ describe MergeRequests::SyncReportApproverApprovalRules do
specify { expect(vulnerability_check_rule.approval_project_rule).to eq(vulnerability_project_rule) } 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).to be_report_approver }
specify { expect(license_check_rule.approvals_required).to eql(license_compliance_project_rule.approvals_required) } 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.name).to eq(license_compliance_project_rule.name) }
specify { expect(license_check_rule.approval_project_rule).to eq(license_compliance_project_rule) } specify { expect(license_check_rule.approval_project_rule).to eq(license_compliance_project_rule) }
end end
context "when some of the rules have been synchronized to the merge request" do 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 before do
service.execute service.execute
...@@ -114,7 +114,7 @@ describe MergeRequests::SyncReportApproverApprovalRules do ...@@ -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.count).to be(2) }
specify { expect(merge_request.reload.approval_rules.security_report.count).to be(1) } 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
end end
......
...@@ -62,24 +62,42 @@ describe Security::SyncReportsToApprovalRulesService, '#execute' do ...@@ -62,24 +62,42 @@ describe Security::SyncReportsToApprovalRulesService, '#execute' do
end end
context "license compliance policy" 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_scanning, merge_request: merge_request, approvals_required: 1) }
let!(:license_compliance_rule) { create(:report_approver_rule, :license_management, merge_request: merge_request, approvals_required: 1) }
let!(:denied_license) { create(:software_license) }
context "when a license violates the license compliance policy" do 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!(:ci_build) { create(:ee_ci_build, :success, :license_management, pipeline: pipeline, project: project) } let(:denied_license) { create(:software_license, name: license_name) }
let!(:license_name) { ci_build.pipeline.license_scanning_report.license_names[0] } let(:license_name) { ci_build.pipeline.license_scanning_report.license_names[0] }
specify { expect { subject }.not_to change { license_compliance_rule.reload.approvals_required } } context 'with a new report' do
specify { expect(subject[:status]).to be(:success) } 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) }
specify { expect { subject }.not_to change { license_compliance_rule.reload.approvals_required } }
specify { expect(subject[:status]).to be(:success) }
end
end end
context "when no licenses violate the license compliance policy" do context "when no licenses violate the license compliance policy" do
let!(:ci_build) { create(:ee_ci_build, :success, :license_management, pipeline: pipeline, project: project) } 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 }.to change { license_compliance_rule.reload.approvals_required }.from(1).to(0) }
specify { expect(subject[:status]).to be(:success) } 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 end
context "when an unexpected error occurs" do context "when an unexpected error occurs" do
...@@ -142,7 +160,7 @@ describe Security::SyncReportsToApprovalRulesService, '#execute' do ...@@ -142,7 +160,7 @@ describe Security::SyncReportsToApprovalRulesService, '#execute' do
context "license compliance policy" do context "license compliance policy" do
let!(:software_license_policy) { create(:software_license_policy, :denied, project: project, software_license: denied_license) } 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) } let!(:denied_license) { create(:software_license) }
specify { expect { subject }.not_to change { license_compliance_rule.reload.approvals_required } } specify { expect { subject }.not_to change { license_compliance_rule.reload.approvals_required } }
......
...@@ -17,13 +17,13 @@ describe RefreshLicenseComplianceChecksWorker do ...@@ -17,13 +17,13 @@ describe RefreshLicenseComplianceChecksWorker do
let!(:closed_merge_request) { create(:merge_request, :closed, target_project: project, source_project: project) } 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 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!(: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_management, merge_request: closed_merge_request, approvals_required: 0) } 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_management, project: project) } 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 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_management_report, project: project, merge_requests_as_head_pipeline: [open_merge_request]) } 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_management_report, project: project, merge_requests_as_head_pipeline: [closed_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!(: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) { create(:software_license, name: license_report.license_names[0]) }
let(:license_report) { open_pipeline.license_scanning_report } let(:license_report) { open_pipeline.license_scanning_report }
...@@ -36,9 +36,9 @@ describe RefreshLicenseComplianceChecksWorker do ...@@ -36,9 +36,9 @@ describe RefreshLicenseComplianceChecksWorker do
specify { expect(closed_merge_request_approval_rule.reload.approvals_required).to be_zero } specify { expect(closed_merge_request_approval_rule.reload.approvals_required).to be_zero }
end end
context "when none of the denied licenses appear in the most recent license management reports" do 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_management_report, project: project, merge_requests_as_head_pipeline: [open_merge_request]) } 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_management_report, project: project, merge_requests_as_head_pipeline: [closed_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!(:denied_policy) { create(:software_license_policy, :denied, project: project, software_license: license) }
let(:license) { create(:software_license, name: SecureRandom.uuid) } let(:license) { create(:software_license, name: SecureRandom.uuid) }
......
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