Skip to content

Commit

Permalink
code review updates
Browse files Browse the repository at this point in the history
  • Loading branch information
lslezak committed Oct 13, 2014
1 parent c527e49 commit 1feef7a
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 11 deletions.
8 changes: 4 additions & 4 deletions src/modules/AutoInstallRules.rb
Expand Up @@ -1181,14 +1181,14 @@ def distro_map(distro)
end

# split at the first comma, resulting in 2 parts at max.
parsed = distro.split(",", 2)
cpeid, name = distro.split(",", 2)

if parsed.size != 2
if !name
log.warn "Cannot parse DISTRO value: #{distro}"
return nil
end

{"cpeid" => parsed[0], "name" => parsed[1]}
{"cpeid" => cpeid, "name" => name}
end

# parse CPE ID in URI syntax
Expand All @@ -1197,7 +1197,7 @@ def distro_map(distro)
# @return [Hash<String,String>] parsed values, the keys are "part", "vendor", "product",
# "version", "update", "edition", "lang", nil is returned for missing values
def cpeid_map(cpeid)
return nil unless cpeid
return nil unless cpeid && cpeid.start_with?("cpe:/")

This comment has been minimized.

Copy link
@jreidinger

jreidinger Oct 13, 2014

Member

NTH: I find quite hard to read unless together with && maybe better to split it to separate lines like

return nil unless cpeid
return nil unless cpeid.start_with?("cpe:/")

# remove the "cpe:/" prefix
raw_cpe = cpeid.sub(/^cpe:\//, "")
Expand Down
39 changes: 32 additions & 7 deletions test/AutoInstallRules_test.rb
Expand Up @@ -21,26 +21,51 @@
"lang" => nil
)
end

it "parses Adv. mgmt module CPE ID" do
machinery_cpeid = "cpe:/o:suse:sle-module-adv-systems-management:12"
expect(subject.send(:cpeid_map, machinery_cpeid)).to eq(
"part" => "o",
"vendor" => "suse",
"product" => "sle-module-adv-systems-management",
"version" => "12",
"update" => nil,
"edition" => nil,
"lang" => nil
)
end

it "return nil when CPE ID is does not start with 'cpe:/'" do
expect(subject.send(:cpeid_map, "invalid")).to be_nil
end
end

describe "#distro_map" do
it "returns CPEID and product name" do
expect(subject.send(:distro_map, "cpe:/o:suse:sles:12,SUSE Linux Enterprise Server 12")).
to eq({"cpeid" => "cpe:/o:suse:sles:12",
"name" => "SUSE Linux Enterprise Server 12"})
param = "cpe:/o:suse:sles:12,SUSE Linux Enterprise Server 12"
expected = {
"cpeid" => "cpe:/o:suse:sles:12",
"name" => "SUSE Linux Enterprise Server 12"
}

expect(subject.send(:distro_map, param)).to eq(expected)
end

it "returns product name with comma" do
expect(subject.send(:distro_map, "cpe:/o:suse:sles:12,SLES12, Mini edition")).
to eq({"cpeid" => "cpe:/o:suse:sles:12",
"name" => "SLES12, Mini edition"})
param = "cpe:/o:suse:sles:12,SLES12, Mini edition"
expected = {
"cpeid" => "cpe:/o:suse:sles:12",
"name" => "SLES12, Mini edition"
}

expect(subject.send(:distro_map, param)).to eq(expected)
end

it "returns nil if input is nil" do
expect(subject.send(:distro_map, nil)).to be_nil
end

it "returns nil if input is invalid" do
it "returns nil if the input does not contain comma" do
expect(subject.send(:distro_map, "foo")).to be_nil
end
end
Expand Down

0 comments on commit 1feef7a

Please sign in to comment.