Skip to content

Commit

Permalink
Implemented mvidner's code review suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
shundhammer committed Aug 23, 2021
1 parent b3d767d commit ae360f1
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 2 deletions.
15 changes: 13 additions & 2 deletions src/lib/installation/unmounter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class Unmounter
include Yast::Logger
# @return [Array<Mount>] Relevant mounts to unmount
attr_reader :mounts
# @return [Array<Mount>] Ignored mounts
# @return [Array<Mount>] Ignored mounts (not starting with the mount prefix)
attr_reader :ignored_mounts

# Helper class to represent one mount, i.e. one entry in /proc/mounts.
Expand Down Expand Up @@ -102,7 +102,7 @@ def add_mount(line)
mount = parse_mount(line)
return nil if mount.nil? # Empty or comment

if !mount.mount_path.start_with?(@mnt_prefix)
if ignore?(mount)
@ignored_mounts << mount
return nil
end
Expand All @@ -112,6 +112,17 @@ def add_mount(line)
mount
end

# Check if a mount should be ignored, i.e. if the path doesn't start with
# the mount prefix (usually "/mnt").
#
# @return [Boolean] ignore
#
def ignore?(mount)
return false if mount.mount_path == @mnt_prefix

!mount.mount_path.start_with?(@mnt_prefix + "/")
end

# Parse one entry of /proc/mounts.
#
# @param line [String] one line of /proc/mounts
Expand Down
28 changes: 28 additions & 0 deletions test/unmounter_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ def stored_proc_mounts(scenario)
File.join(PROC_MOUNTS_PATH, "proc-mounts-#{scenario}-raw.txt")
end

def mount(mount_path)
Installation::Unmounter::Mount.new("/dev/something", mount_path, "FooFS")
end

describe Installation::Unmounter do
let(:proc_mounts) { "" }

Expand Down Expand Up @@ -42,6 +46,30 @@ def stored_proc_mounts(scenario)
end
end

describe "#ignore?" do
let(:subject) { described_class.new("/mnt", "") }

it "does not ignore /mnt" do
expect(subject.ignore?(mount("/mnt"))).to eq false
end

it "does not ignore /mnt/foo" do
expect(subject.ignore?(mount("/mnt/foo"))).to eq false
end

it "ignores /mnt2" do
expect(subject.ignore?(mount("/mnt2"))).to eq true
end

it "ignores /mnt2/foo" do
expect(subject.ignore?(mount("/mnt2"))).to eq true
end

it "ignores an empty path" do
expect(subject.ignore?(mount(""))).to eq true
end
end

describe "#add_mount and #clear" do
before(:all) do
# Start with a completely empty unmounter
Expand Down

0 comments on commit ae360f1

Please sign in to comment.