Commit 4324cffc authored by Mehmet Emin INAC's avatar Mehmet Emin INAC

Extend the test cases for deduplication logic

Also made some cosmetic changes.
parent 3be15804
...@@ -58,7 +58,7 @@ module Security ...@@ -58,7 +58,7 @@ module Security
def deduplicate_findings! def deduplicate_findings!
@findings, * = @findings.each_with_object([[], Set.new]) do |finding, (deduplicated, seen_identifiers)| @findings, * = @findings.each_with_object([[], Set.new]) do |finding, (deduplicated, seen_identifiers)|
next if finding.keys.any? { |key| seen_identifiers.member?(key) } next if seen_identifiers.intersect?(finding.keys.to_set)
seen_identifiers.merge(finding.keys) seen_identifiers.merge(finding.keys)
deduplicated << finding deduplicated << finding
......
...@@ -57,7 +57,7 @@ module Security ...@@ -57,7 +57,7 @@ module Security
end end
def register_keys(keys) def register_keys(keys)
return false if keys.any? { |key| known_keys.member?(key) } return false if known_keys.intersect?(keys.to_set)
known_keys.merge(keys) known_keys.merge(keys)
end end
......
...@@ -11,6 +11,9 @@ RSpec.describe Gitlab::Ci::Reports::Security::FindingKey do ...@@ -11,6 +11,9 @@ RSpec.describe Gitlab::Ci::Reports::Security::FindingKey do
'location fp' | nil | 'identifier fp' | 'different identifier fp' | false 'location fp' | nil | 'identifier fp' | 'different identifier fp' | false
'location fp' | 'different location fp' | nil | 'different identifier fp' | false 'location fp' | 'different location fp' | nil | 'different identifier fp' | false
'location fp' | 'different location fp' | 'identifier fp' | nil | false 'location fp' | 'different location fp' | 'identifier fp' | nil | false
nil | nil | 'identifier fp' | 'identifier fp' | false
'location fp' | 'location fp' | nil | nil | false
nil | nil | nil | nil | false
'location fp' | 'different location fp' | 'identifier fp' | 'different identifier fp' | false 'location fp' | 'different location fp' | 'identifier fp' | 'different identifier fp' | false
'location fp' | 'different location fp' | 'identifier fp' | 'identifier fp' | false 'location fp' | 'different location fp' | 'identifier fp' | 'identifier fp' | false
'location fp' | 'location fp' | 'identifier fp' | 'different identifier fp' | false 'location fp' | 'location fp' | 'identifier fp' | 'different identifier fp' | false
......
...@@ -3,6 +3,8 @@ ...@@ -3,6 +3,8 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Gitlab::Ci::Reports::Security::Identifier do RSpec.describe Gitlab::Ci::Reports::Security::Identifier do
using RSpec::Parameterized::TableSyntax
describe '#initialize' do describe '#initialize' do
subject { described_class.new(**params) } subject { described_class.new(**params) }
...@@ -52,39 +54,39 @@ RSpec.describe Gitlab::Ci::Reports::Security::Identifier do ...@@ -52,39 +54,39 @@ RSpec.describe Gitlab::Ci::Reports::Security::Identifier do
end end
end end
describe '#cve?' do describe '#type_identifier?' do
let(:identifier) { create(:ci_reports_security_identifier, external_type: external_type) } where(:external_type, :expected_result) do
'cve' | false
subject { identifier.cve? } 'foo' | false
'cwe' | true
context 'when has cve as external type' do 'wasc' | true
let(:external_type) { 'Cve' }
it { is_expected.to eq(true) }
end end
context 'when does not have cve as external type' do with_them do
let(:external_type) { 'Cwe' } let(:identifier) { create(:ci_reports_security_identifier, external_type: external_type) }
subject { identifier.type_identifier? }
it { is_expected.to eq(false) } it { is_expected.to be(expected_result) }
end end
end end
describe '#cwe?' do describe 'external type check methods' do
let(:identifier) { create(:ci_reports_security_identifier, external_type: external_type) } where(:external_type, :is_cve?, :is_cwe?, :is_wasc?) do
'Foo' | false | false | false
subject { identifier.cwe? } 'Cve' | true | false | false
'Cwe' | false | true | false
context 'when has cwe as external type' do 'Wasc' | false | false | true
let(:external_type) { 'Cwe' }
it { is_expected.to eq(true) }
end end
context 'when does not have cwe as external type' do with_them do
let(:external_type) { 'Cve' } let(:identifier) { create(:ci_reports_security_identifier, external_type: external_type) }
it { is_expected.to eq(false) } it 'returns correct result for the type check method' do
expect(identifier.cve?).to be(is_cve?)
expect(identifier.cwe?).to be(is_cwe?)
expect(identifier.wasc?).to be(is_wasc?)
end
end end
end end
...@@ -105,8 +107,6 @@ RSpec.describe Gitlab::Ci::Reports::Security::Identifier do ...@@ -105,8 +107,6 @@ RSpec.describe Gitlab::Ci::Reports::Security::Identifier do
end end
describe '#==' do describe '#==' do
using RSpec::Parameterized::TableSyntax
where(:type_1, :id_1, :type_2, :id_2, :equal, :case_name) do where(:type_1, :id_1, :type_2, :id_2, :equal, :case_name) do
'CVE' | '2018-1234' | 'CVE' | '2018-1234' | true | 'when external_type and external_id are equal' 'CVE' | '2018-1234' | 'CVE' | '2018-1234' | true | 'when external_type and external_id are equal'
'CVE' | '2018-1234' | 'brakeman_code' | '2018-1234' | false | 'when external_type is different' 'CVE' | '2018-1234' | 'brakeman_code' | '2018-1234' | false | 'when external_type is different'
......
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