Improve FileSystems module tests

src/modules/FileSystems.rb

@@ -35,8 +35,14 @@
 
 module Yast
   class FileSystemsClass < Module
+    include Yast::Logger
+
+    # @return [Array<String>] Supported default subvolume names
     SUPPORTED_DEFAULT_SUBVOLUME_NAMES = ["", "@"].freeze
 
+    # @return [String] Default subvolume name.
+    attr_reader :default_subvol
+
     def main
 
       textdomain "storage"
@@ -2049,13 +2055,6 @@ def default_subvol=(name)
       end
     end
 
-    # Default subvolume name
-    #
-    # @return [String] Default subvolume name.
-    def default_subvol
-      @default_subvol
-    end
-
     # Try to find the default subvolume name in the target system
     #
     # * Root partition takes precedence

test/filesystems_test.rb

@@ -8,10 +8,12 @@
 describe Yast::FileSystems do
   subject { Yast::FileSystems }
 
-  describe "#default_subvol_from_filesystem" do
-    let(:target_map) { YAML.load_file(File.join(FIXTURES_PATH, "subvolumes.yml")) }
-    let(:btrfs_list) { File.read(File.join(FIXTURES_PATH, "btrfs_list.out")) }
-    let(:btrfs_list_no_at) { File.read(File.join(FIXTURES_PATH, "btrfs_list_no_at.out")) }
+  def btrfs_list_fixture(name)
+    fixture_name = "btrfs_list_#{name}.out"
+    File.read(File.join(FIXTURES_PATH, fixture_name))
+  end
+
+  describe "#default_subvol_from_target" do
     let(:default_subvol) { "@" }
 
     before do
@@ -21,44 +23,74 @@
     end
 
     context "when root partition uses the default subvolume name (@)" do
-      let(:btrfs_list) { File.read(File.join(FIXTURES_PATH, "btrfs_list.out")) }
+      let(:target_map) { YAML.load_file(File.join(FIXTURES_PATH, "subvolumes.yml")) }
 
-      it "returns the default subvolume name" do
+      before do
         allow(Yast::Execute).to receive(:on_target).with("btrfs", "subvol", "list", "/", anything)
-          .and_return(btrfs_list)
+          .and_return(btrfs_list_fixture("root"))
         allow(Yast::Execute).to receive(:on_target).with("btrfs", "subvol", "list", "/srv", anything)
-          .and_return(btrfs_list_no_at)
-        expect(subject.default_subvol_from_filesystem).to eq(default_subvol)
+          .and_return(btrfs_list_fixture("root_no_at"))
+      end
+
+      it "returns the default subvolume name" do
+        expect(subject.default_subvol_from_target).to eq(default_subvol)
       end
     end
 
-    context "when root partitions does not use the default subvolume name (@)" do
+    context "when root partitions does not use btrfs" do
+      let(:target_map) { YAML.load_file(File.join(FIXTURES_PATH, "subvolumes-no-root.yml")) }
+
       before do
         allow(Yast::Execute).to receive(:on_target).with("btrfs", "subvol", "list", "/", anything)
-          .and_return(btrfs_list_no_at)
+          .and_return(btrfs_list_fixture("root_no_at"))
       end
 
-      context "but all partitions uses the same subvolume name" do
+      context "but all btrfs partitions uses the same subvolume name" do
         before do
           allow(Yast::Execute).to receive(:on_target).with("btrfs", "subvol", "list", "/srv", anything)
-            .and_return(btrfs_list_no_at)
+            .and_return(btrfs_list_fixture("srv_no_at"))
+          allow(Yast::Execute).to receive(:on_target).with("btrfs", "subvol", "list", "/data", anything)
+            .and_return(btrfs_list_fixture("data_no_at"))
         end
 
         it "returns the used name ('' in this case)" do
-          expect(subject.default_subvol_from_filesystem).to eq("")
+          expect(subject.default_subvol_from_target).to eq("")
         end
       end
 
-      context "and partitions uses different subvolume names" do
+      context "and btrfs partitions uses different subvolume names" do
         before do
-          allow(Yast::Execute).to receive(:on_target).with("btrfs", "subvol", "list", "/srv", anything)
-            .and_return(btrfs_list)
+          expect(Yast::Execute).to receive(:on_target).with("btrfs", "subvol", "list", "/srv", anything)
+            .and_return(btrfs_list_fixture("srv"))
+          allow(Yast::Execute).to receive(:on_target).with("btrfs", "subvol", "list", "/data", anything)
+            .and_return(btrfs_list_fixture("data_no_at"))
         end
 
         it "returns the distribution default" do
-          expect(subject.default_subvol_from_filesystem).to eq("")
+          expect(subject.default_subvol_from_target).to eq(default_subvol)
         end
       end
     end
   end
+
+  describe "#read_default_subvol_from_target" do
+    it "sets the default_subvol using the value from target" do
+      subject.default_subvol = ""
+      expect(subject).to receive(:default_subvol_from_target).and_return("@")
+      expect { subject.read_default_subvol_from_target }.to change { subject.default_subvol }
+        .from("").to("@")
+    end
+  end
+
+  describe "#default_subvol=" do
+    it "sets the default_subvol if a valid value is given" do
+      subject.default_subvol = ""
+      expect { subject.default_subvol = "@" }.to change { subject.default_subvol }
+        .to("@")
+    end
+
+    it "refuses to set default_subvol if an invalid value is given" do
+      expect { subject.default_subvol = "whatever" }.to_not change { subject.default_subvol }
+    end
+  end
 end

test/fixtures/btrfs_list_data_no_at.out

@@ -0,0 +1,3 @@
+ID 257 gen 16 top level 5 path shared
+ID 258 gen 16 top level 5 path private
+

test/fixtures/btrfs_list_srv.out

@@ -0,0 +1,5 @@
+ID 257 gen 16 top level 5 path @
+ID 258 gen 16 top level 257 path www
+ID 259 gen 16 top level 257 path mirror
+ID 260 gen 16 top level 257 path ftp
+ID 261 gen 16 top level 257 path tftpboot

test/fixtures/btrfs_list_srv_no_at.out

@@ -0,0 +1,5 @@
+ID 257 gen 16 top level 5 path www
+ID 258 gen 16 top level 5 path mirror
+ID 259 gen 16 top level 5 path ftp
+ID 260 gen 16 top level 5 path tftpboot
+

test/fixtures/subvolumes-no-root.yml

@@ -0,0 +1,180 @@
+---
+"/dev/vda":
+  unique: KSbE.Fxp0d3BezAE
+  bus: None
+  device: "/dev/vda"
+  bios_id: '0x80'
+  driver: virtio-pci
+  driver_module: virtio_pci
+  partitions:
+  - device: "/dev/vda1"
+    name: vda1
+    used_by_type: :UB_NONE
+    used_by_device: ''
+    size_k: 264192
+    used_fs: :swap
+    detected_fs: :swap
+    mount: swap
+    mountby: :uuid
+    fstopt: defaults
+    uuid: 559e2279-4012-4174-a746-e4664031851d
+    nr: 1
+    fsid: 130
+    fstype: Linux swap
+    region:
+    - 0
+    - 34
+    type: :primary
+  - device: "/dev/vda2"
+    name: vda2
+    used_by_type: :UB_NONE
+    used_by_device: ''
+    size_k: 5244928
+    used_fs: :ext4
+    detected_fs: :ext4
+    mount: "/"
+    mountby: :uuid
+    fstopt: acl,user_xattr
+    uuid: 323a8681-4229-4e3f-ac67-c0b70b148ecd
+    nr: 2
+    fsid: 131
+    fstype: Linux native
+    region:
+    - 33
+    - 653
+    type: :primary
+    boot: true
+  - device: "/dev/vda3"
+    name: vda3
+    used_by:
+    - type: :UB_BTRFS
+      device: e71f8bec-08b6-441c-a2d0-b25cc7bc856a
+    used_by_type: :UB_BTRFS
+    used_by_device: e71f8bec-08b6-441c-a2d0-b25cc7bc856a
+    size_k: 2618368
+    used_fs: :btrfs
+    detected_fs: :btrfs
+    fstopt: defaults
+    nr: 3
+    fsid: 131
+    fstype: Linux native
+    region:
+    - 685
+    - 327
+    type: :primary
+    subvol:
+    - name: "@/www"
+    - name: "@/ftp"
+    - name: "@/mirror"
+    uuid: e71f8bec-08b6-441c-a2d0-b25cc7bc856a
+    mount: "/srv"
+    mountby: :uuid
+  - device: "/dev/vda4"
+    name: vda4
+    used_by:
+    - type: :UB_BTRFS
+      device: aa2af2cd-d43e-44b1-8025-e3d119070d47
+    used_by_type: :UB_BTRFS
+    used_by_device: aa2af2cd-d43e-44b1-8025-e3d119070d47
+    size_k: 2357248
+    used_fs: :btrfs
+    detected_fs: :btrfs
+    fstopt: defaults
+    nr: 4
+    fsid: 131
+    fstype: Linux native
+    region:
+    - 1011
+    - 294
+    type: :primary
+    uuid: aa2af2cd-d43e-44b1-8025-e3d119070d47
+    mount: "/data"
+    mountby: :uuid
+  size_k: 10485760
+  cyl_size: 8225280
+  cyl_count: 1305
+  sector_size: 512
+  label: msdos
+  name: vda
+  max_logical: 255
+  max_primary: 4
+  type: :CT_DISK
+  transport: :unknown
+  used_by_type: :UB_NONE
+  used_by_device: ''
+  dasd_format: 0
+  dasd_type: 0
+"/dev/btrfs":
+  device: "/dev/btrfs"
+  name: btrfs
+  used_by_type: :UB_NONE
+  used_by_device: ''
+  type: :CT_BTRFS
+  partitions: []
+"/dev/tmpfs":
+  device: "/dev/tmpfs"
+  name: tmpfs
+  used_by_type: :UB_NONE
+  used_by_device: ''
+  type: :CT_TMPFS
+  partitions:
+  - device: tmpfs
+    name: none
+    used_by_type: :UB_NONE
+    used_by_device: ''
+    size_k: 507800
+    used_fs: :tmpfs
+    detected_fs: :tmpfs
+    mount: "/dev/shm"
+    mountby: :uuid
+    ignore_fstab: true
+    type: :tmpfs
+    fstype: TMPFS
+  - device: tmpfs
+    name: none
+    used_by_type: :UB_NONE
+    used_by_device: ''
+    size_k: 507800
+    used_fs: :tmpfs
+    detected_fs: :tmpfs
+    mount: "/run"
+    mountby: :uuid
+    ignore_fstab: true
+    type: :tmpfs
+    fstype: TMPFS
+  - device: tmpfs
+    name: none
+    used_by_type: :UB_NONE
+    used_by_device: ''
+    size_k: 507800
+    used_fs: :tmpfs
+    detected_fs: :tmpfs
+    mount: "/sys/fs/cgroup"
+    mountby: :uuid
+    ignore_fstab: true
+    type: :tmpfs
+    fstype: TMPFS
+  - device: tmpfs
+    name: none
+    used_by_type: :UB_NONE
+    used_by_device: ''
+    size_k: 101564
+    used_fs: :tmpfs
+    detected_fs: :tmpfs
+    mount: "/run/user/483"
+    mountby: :uuid
+    ignore_fstab: true
+    type: :tmpfs
+    fstype: TMPFS
+  - device: tmpfs
+    name: none
+    used_by_type: :UB_NONE
+    used_by_device: ''
+    size_k: 101564
+    used_fs: :tmpfs
+    detected_fs: :tmpfs
+    mount: "/run/user/0"
+    mountby: :uuid
+    ignore_fstab: true
+    type: :tmpfs
+    fstype: TMPFS