From 1ab3423094989e57747a2a217987ec443e9e4682 Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Fri, 10 Feb 2023 15:16:52 +0100 Subject: [PATCH] Yast::Execute escapes too much, use native Ruby instead --- src/lib/y2iscsi_client/finish_client.rb | 10 +++-- test/y2iscsi_client/finish_client_test.rb | 50 +++++++++++------------ 2 files changed, 29 insertions(+), 31 deletions(-) diff --git a/src/lib/y2iscsi_client/finish_client.rb b/src/lib/y2iscsi_client/finish_client.rb index 9aa0923..0c7356c 100644 --- a/src/lib/y2iscsi_client/finish_client.rb +++ b/src/lib/y2iscsi_client/finish_client.rb @@ -24,6 +24,7 @@ require "installation/finish_client" require "yast2/systemd/socket" require "yast2/execute" +require "fileutils" Yast.import "IscsiClientLib" Yast.import "Installation" @@ -68,11 +69,12 @@ def copy_configuration def copy_directory(dir) return unless File.directory?(dir) - Yast::Execute.locally!( - "mkdir", "-p", File.join(Yast::Installation.destdir, dir), "&&", - "cp", "-a", File.join(dir, "*"), File.join(Installation.destdir, dir, "/") + log.info "Copying iSCSI configuration from #{dir}" + ::FileUtils.mkdir_p(File.join(Yast::Installation.destdir, dir)) + ::FileUtils.cp_r( + Dir[File.join(dir, "*")], File.join(Installation.destdir, dir, "/"), preserve: true ) - rescue Cheetah::ExecutionFailed + rescue SystemCallError log.error "Failed to copy the iSCSI configuration from #{dir}" end diff --git a/test/y2iscsi_client/finish_client_test.rb b/test/y2iscsi_client/finish_client_test.rb index 894301b..9736262 100755 --- a/test/y2iscsi_client/finish_client_test.rb +++ b/test/y2iscsi_client/finish_client_test.rb @@ -10,11 +10,15 @@ allow(Yast::IscsiClientLib).to receive(:sessions).and_return sessions allow(File).to receive(:directory?).with("/etc/iscsi").and_return etc_exists allow(File).to receive(:directory?).with("/var/lib/iscsi").and_return var_exists + allow(Dir).to receive(:[]).with("/etc/iscsi/*").and_return etc_content + allow(Dir).to receive(:[]).with("/var/lib/iscsi/*").and_return var_content end let(:sessions) { [] } let(:etc_exists) { false } let(:var_exists) { false } + let(:etc_content) { ["one", "two"] } + let(:var_content) { ["three", "four"] } context "if there is an /etc/iscsi directory in the int-sys" do let(:etc_exists) { true } @@ -23,14 +27,12 @@ let(:var_exists) { true } it "copies the content of both directories to the target system" do - expect(Yast::Execute).to receive(:locally!) do |*args| - expect(args.join(" ")) - .to eq "mkdir -p /mnt/etc/iscsi && cp -a /etc/iscsi/* /mnt/etc/iscsi/" - end - expect(Yast::Execute).to receive(:locally!) do |*args| - expect(args.join(" ")) - .to eq "mkdir -p /mnt/var/lib/iscsi && cp -a /var/lib/iscsi/* /mnt/var/lib/iscsi/" - end + expect(::FileUtils).to receive(:mkdir_p).with("/mnt/etc/iscsi") + expect(::FileUtils).to receive(:cp_r) + .with(etc_content, "/mnt/etc/iscsi/", preserve: true) + expect(::FileUtils).to receive(:mkdir_p).with("/mnt/var/lib/iscsi") + expect(::FileUtils).to receive(:cp_r) + .with(var_content, "/mnt/var/lib/iscsi/", preserve: true) subject.write end @@ -40,19 +42,15 @@ let(:var_exists) { false } it "copies the content of /etc/iscsi to the target system" do - expect(Yast::Execute).to receive(:locally!) do |*args| - expect(args.join(" ")) - .to eq "mkdir -p /mnt/etc/iscsi && cp -a /etc/iscsi/* /mnt/etc/iscsi/" - end + expect(::FileUtils).to receive(:mkdir_p).with("/mnt/etc/iscsi") + expect(::FileUtils).to receive(:cp_r).with(etc_content, "/mnt/etc/iscsi/", preserve: true) subject.write end it "does not copy /var/lib/iscsi" do - allow(Yast::Execute).to receive(:locally!) do |*args| - expect(args).to_not include "/mnt/var/lib/iscsi" - expect(args).to_not include "/var/lib/iscsi/*" - end + expect(::FileUtils).to_not receive(:mkdir_p).with("/mnt/var/lib/iscsi") + expect(::FileUtils).to_not receive(:cp_r).with(anything, "/mnt/var/lib/iscsi/", any_args) subject.write end @@ -66,19 +64,16 @@ let(:var_exists) { true } it "copies the content of /var/lib/iscsi to the target system" do - expect(Yast::Execute).to receive(:locally!) do |*args| - expect(args.join(" ")) - .to eq "mkdir -p /mnt/var/lib/iscsi && cp -a /var/lib/iscsi/* /mnt/var/lib/iscsi/" - end + expect(::FileUtils).to receive(:mkdir_p).with("/mnt/var/lib/iscsi") + expect(::FileUtils).to receive(:cp_r) + .with(var_content, "/mnt/var/lib/iscsi/", preserve: true) subject.write end it "does not copy /etc/iscsi" do - allow(Yast::Execute).to receive(:locally!) do |*args| - expect(args).to_not include "/mnt/etc/iscsi" - expect(args).to_not include "/etc/iscsi/*" - end + expect(::FileUtils).to_not receive(:mkdir_p).with("/mnt/etc/iscsi") + expect(::FileUtils).to_not receive(:cp_r).with(anything, "/mnt/etc/iscsi/", any_args) subject.write end @@ -88,7 +83,8 @@ let(:var_exists) { false } it "does not copy any file" do - expect(Yast::Execute).to_not receive(:locally!) + expect(::FileUtils).to_not receive(:mkdir_p) + expect(::FileUtils).to_not receive(:cp_r) subject.write end end @@ -99,8 +95,8 @@ let(:var_exists) { true } it "fails gracefully (no exception or crash) if copying fails" do - allow(Yast::Execute).to receive(:locally!).with(any_args, "cp", "-a", anything, anything) - .and_raise Cheetah::ExecutionFailed.new("cmd", 1, "", "Something went wrong") + allow(::FileUtils).to receive(:mkdir_p) + allow(::FileUtils).to receive(:cp_r).and_raise Errno::ENOENT expect { subject.write }.to_not raise_error end