From 84d3a5fe47ed936d1347e4fad3ee557c0854c376 Mon Sep 17 00:00:00 2001
From: Chris Roberts <croberts@hashicorp.com>
Date: Mon, 12 Jul 2021 09:54:56 -0700
Subject: [PATCH] Update compose tests to pass fully through Util::Subprocess

---
 plugins/providers/docker/driver.rb            |   4 +-
 .../providers/docker/driver_compose_test.rb   |  90 ++++++-----
 .../plugins/providers/docker/driver_test.rb   | 144 ++++++++++--------
 3 files changed, 138 insertions(+), 100 deletions(-)

diff --git a/plugins/providers/docker/driver.rb b/plugins/providers/docker/driver.rb
index 090544eeb..659d8c3cc 100644
--- a/plugins/providers/docker/driver.rb
+++ b/plugins/providers/docker/driver.rb
@@ -47,7 +47,7 @@ module VagrantPlugins
         end
 
         # Return the matched group `id`
-        matches[0]
+        matches[0].strip
       end
 
       # Check if podman emulating docker CLI is enabled.
@@ -79,7 +79,7 @@ module VagrantPlugins
             if v.index(":") != v.rindex(":")
               # If we have 2 colons, the host path is an absolute Windows URL
               # and we need to remove the colon from it
-              host, colon, guest = v.rpartition(":")
+              host, _, guest = v.rpartition(":")
               host = "//" + host[0].downcase + host[2..-1]
               v = [host, guest].join(":")
             else
diff --git a/test/unit/plugins/providers/docker/driver_compose_test.rb b/test/unit/plugins/providers/docker/driver_compose_test.rb
index d8b3006ca..2d9aa1fdb 100644
--- a/test/unit/plugins/providers/docker/driver_compose_test.rb
+++ b/test/unit/plugins/providers/docker/driver_compose_test.rb
@@ -1,3 +1,4 @@
+require "yaml"
 require_relative "../../../base"
 
 require Vagrant.source_root.join("lib/vagrant/util/deep_merge")
@@ -5,6 +6,17 @@ require Vagrant.source_root.join("plugins/providers/docker/driver")
 
 describe VagrantPlugins::DockerProvider::Driver::Compose do
   let(:cmd_executed) { @cmd }
+  let(:execute_result) {
+    double("execute_result",
+      exit_code: exit_code,
+      stderr: stderr,
+      stdout: stdout
+    )
+  }
+  let(:exit_code) { 0 }
+  let(:stderr) { "" }
+  let(:stdout) { "" }
+
   let(:cid)          { 'side-1-song-10' }
   let(:docker_yml){ double("docker-yml", path: "/tmp-file") }
   let(:machine){ double("machine", env: env, name: :docker_1, id: :docker_id, provider_config: provider_config) }
@@ -33,11 +45,26 @@ describe VagrantPlugins::DockerProvider::Driver::Compose do
   let(:data_directory){ double("data-directory", join: composition_path) }
   let(:local_data_path){ double("local-data-path") }
   let(:compose_execute_up){ ["docker-compose", "-f", "docker-compose.yml", "-p", "cwd", "up", "--remove-orphans", "-d", any_args] }
-
+  let(:compose_execute_up_regex) { /docker-compose -f docker-compose.yml -p cwd up --remove-orphans -d/ }
 
   subject{ described_class.new(machine) }
 
   before do
+    @cmd = []
+    allow(Vagrant::Util::Subprocess).to receive(:execute) { |*args|
+      if args.last.is_a?(Hash)
+        args = args[0, args.size - 1]
+      end
+      invalid = args.detect { |a| !a.is_a?(String) }
+      if invalid
+        raise TypeError,
+          "Vagrant::Util::Subprocess#execute only accepts signle option Hash and String arguments, received `#{invalid.class}'"
+      end
+      @cmd << args.join(" ")
+    }.and_return(execute_result)
+    allow_any_instance_of(Vagrant::Errors::VagrantError).
+      to receive(:translate_error) { |*args| args.join(" ") }
+
     allow(Vagrant::Util::Which).to receive(:which).and_return("/dev/null/docker-compose")
     allow(env).to receive(:lock).and_yield
     allow(Pathname).to receive(:new).with(local_data_path).and_return(local_data_path)
@@ -48,10 +75,6 @@ describe VagrantPlugins::DockerProvider::Driver::Compose do
     allow(Tempfile).to receive(:new).with("vagrant-docker-compose").and_return(docker_yml)
     allow(docker_yml).to receive(:write)
     allow(docker_yml).to receive(:close)
-    allow(subject).to receive(:execute) do |*args|
-      args.delete_if{|i| i.is_a?(Hash) }
-      @cmd = args.join(' ')
-    end
   end
 
   describe '#build' do
@@ -85,8 +108,10 @@ describe VagrantPlugins::DockerProvider::Driver::Compose do
       privileged: true
     } }
 
-    before { expect(subject).to receive(:execute).with(*compose_execute_up) }
-    after { subject.create(params) }
+    after {
+      subject.create(params)
+      expect(cmd_executed.first).to match(compose_execute_up_regex)
+    }
 
     it 'sets container name' do
       expect(docker_yml).to receive(:write).with(/#{machine.name}/)
@@ -168,26 +193,24 @@ describe VagrantPlugins::DockerProvider::Driver::Compose do
 
     it 'performs the check on all containers list' do
       subject.created?(cid)
-      expect(cmd_executed).to match(/docker ps \-a \-q/)
+      expect(cmd_executed.first).to match(/docker ps \-a \-q/)
     end
 
     context 'when container exists' do
-      before { allow(subject).to receive(:execute)
-        .and_return("foo\n#{cid}\nbar") }
+      let(:stdout) { "foo\n#{cid}\nbar" }
       it { expect(result).to be_truthy }
     end
 
     context 'when container does not exist' do
-      before { allow(subject).to receive(:execute)
-        .and_return("foo\n#{cid}extra\nbar") }
+      let(:stdout) { "foo\n#{cid}extra\nbar" }
       it { expect(result).to be_falsey }
     end
   end
 
   describe '#pull' do
     it 'should pull images' do
-      expect(subject).to receive(:execute).with('docker', 'pull', 'foo')
       subject.pull('foo')
+      expect(cmd_executed.first).to eq("docker pull foo")
     end
   end
 
@@ -196,19 +219,17 @@ describe VagrantPlugins::DockerProvider::Driver::Compose do
 
     it 'performs the check on the running containers list' do
       subject.running?(cid)
-      expect(cmd_executed).to match(/docker ps \-q/)
-      expect(cmd_executed).to_not include('-a')
+      expect(cmd_executed.first).to match(/docker ps \-q/)
+      expect(cmd_executed.first).to_not include('-a')
     end
 
     context 'when container exists' do
-      before { allow(subject).to receive(:execute)
-        .and_return("foo\n#{cid}\nbar") }
+      let(:stdout) { "foo\n#{cid}\nbar" }
       it { expect(result).to be_truthy }
     end
 
     context 'when container does not exist' do
-      before { allow(subject).to receive(:execute)
-        .and_return("foo\n#{cid}extra\nbar") }
+      let(:stdout) { "foo\n#{cid}extra\nbar" }
       it { expect(result).to be_falsey }
     end
   end
@@ -232,8 +253,8 @@ describe VagrantPlugins::DockerProvider::Driver::Compose do
       before { allow(subject).to receive(:running?).and_return(true) }
 
       it 'does not start the container' do
-        expect(subject).not_to receive(:execute).with('docker', 'start', cid)
         subject.start(cid)
+        expect(cmd_executed).to be_empty
       end
     end
 
@@ -241,8 +262,8 @@ describe VagrantPlugins::DockerProvider::Driver::Compose do
       before { allow(subject).to receive(:running?).and_return(false) }
 
       it 'starts the container' do
-        expect(subject).to receive(:execute).with('docker', 'start', cid)
         subject.start(cid)
+        expect(cmd_executed.first).to eq("docker start #{cid}")
       end
     end
   end
@@ -252,13 +273,13 @@ describe VagrantPlugins::DockerProvider::Driver::Compose do
       before { allow(subject).to receive(:running?).and_return(true) }
 
       it 'stops the container' do
-        expect(subject).to receive(:execute).with('docker', 'stop', '-t', '1', cid)
         subject.stop(cid, 1)
+        expect(cmd_executed.first).to eq("docker stop -t 1 #{cid}")
       end
 
       it "stops the container with the set timeout" do
-        expect(subject).to receive(:execute).with('docker', 'stop', '-t', '5', cid)
         subject.stop(cid, 5)
+        expect(cmd_executed.first).to eq("docker stop -t 5 #{cid}")
       end
     end
 
@@ -268,6 +289,7 @@ describe VagrantPlugins::DockerProvider::Driver::Compose do
       it 'does not stop container' do
         expect(subject).not_to receive(:execute).with('docker', 'stop', '-t', '1', cid)
         subject.stop(cid, 1)
+        expect(cmd_executed).to be_empty
       end
     end
   end
@@ -277,8 +299,8 @@ describe VagrantPlugins::DockerProvider::Driver::Compose do
       before { allow(subject).to receive(:created?).and_return(true) }
 
       it 'removes the container' do
-        expect(subject).to receive(:execute).with("docker-compose", "-f", "docker-compose.yml", "-p", "cwd", "rm", "-f", "docker_1", any_args)
         subject.rm(cid)
+        expect(cmd_executed.first).to match(/docker-compose -f docker-compose.yml -p cwd rm -f docker_1/)
       end
     end
 
@@ -286,20 +308,18 @@ describe VagrantPlugins::DockerProvider::Driver::Compose do
       before { allow(subject).to receive(:created?).and_return(false) }
 
       it 'does not attempt to remove the container' do
-        expect(subject).not_to receive(:execute).with("docker-compose", "-f", "docker-compose.yml", "-p", "cwd", "rm", "-f", "docker_1", {})
         subject.rm(cid)
+        expect(cmd_executed).to be_empty
       end
     end
   end
 
   describe '#inspect_container' do
-    let(:data) { '[{"json": "value"}]' }
-
-    before { allow(subject).to receive(:execute).and_return(data) }
+    let(:stdout) { '[{"json": "value"}]' }
 
     it 'inspects the container' do
-      expect(subject).to receive(:execute).with('docker', 'inspect', cid)
       subject.inspect_container(cid)
+      expect(cmd_executed.first).to eq("docker inspect #{cid}")
     end
 
     it 'parses the json output' do
@@ -308,24 +328,20 @@ describe VagrantPlugins::DockerProvider::Driver::Compose do
   end
 
   describe '#all_containers' do
-    let(:containers) { "container1\ncontainer2" }
-
-    before { allow(subject).to receive(:execute).and_return(containers) }
+    let(:stdout) { "container1\ncontainer2" }
 
     it 'returns an array of all known containers' do
-      expect(subject).to receive(:execute).with('docker', 'ps', '-a', '-q', '--no-trunc')
       expect(subject.all_containers).to eq(['container1', 'container2'])
+      expect(cmd_executed.first).to eq("docker ps -a -q --no-trunc")
     end
   end
 
   describe '#docker_bridge_ip' do
-    let(:containers) { " inet 123.456.789.012/16 " }
-
-    before { allow(subject).to receive(:execute).and_return(containers) }
+    let(:stdout) { " inet 123.456.789.012/16 " }
 
     it 'returns an array of all known containers' do
-      expect(subject).to receive(:execute).with('/sbin/ip', '-4', 'addr', 'show', 'scope', 'global', 'docker0')
       expect(subject.docker_bridge_ip).to eq('123.456.789.012')
+      expect(cmd_executed.first).to eq("/sbin/ip -4 addr show scope global docker0")
     end
   end
 end
diff --git a/test/unit/plugins/providers/docker/driver_test.rb b/test/unit/plugins/providers/docker/driver_test.rb
index bc0dc1b9d..35003bd94 100644
--- a/test/unit/plugins/providers/docker/driver_test.rb
+++ b/test/unit/plugins/providers/docker/driver_test.rb
@@ -5,9 +5,31 @@ require Vagrant.source_root.join("plugins/providers/docker/driver")
 describe VagrantPlugins::DockerProvider::Driver do
   let(:cmd_executed) { @cmd }
   let(:cid)          { 'side-1-song-10' }
+  let(:execute_result) {
+    double("execute_result",
+      exit_code: exit_code,
+      stderr: stderr,
+      stdout: stdout
+    )
+  }
+  let(:exit_code) { 0 }
+  let(:stderr) { "" }
+  let(:stdout) { "" }
 
   before do
-    allow(subject).to receive(:execute) { |*args| @cmd = args.join(' ') }
+    allow(Vagrant::Util::Subprocess).to receive(:execute) { |*args|
+      if args.last.is_a?(Hash)
+        args = args[0, args.size - 1]
+      end
+      invalid = args.detect { |a| !a.is_a?(String) }
+      if invalid
+        raise TypeError,
+          "Vagrant::Util::Subprocess#execute only accepts signle option Hash and String arguments, received `#{invalid.class}'"
+      end
+      @cmd = args.join(" ")
+    }.and_return(execute_result)
+    allow_any_instance_of(Vagrant::Errors::VagrantError).
+      to receive(:translate_error) { |*args| args.join(" ") }
   end
 
   let(:docker_network_struct) {
@@ -153,60 +175,62 @@ describe VagrantPlugins::DockerProvider::Driver do
 
 
   describe '#build' do
-    let(:result) { "Successfully built other_package\nSuccessfully built 1a2b3c4d" }
-    let(:buildkit_result_old) { "writing image sha256:1a2b3c4d done" }
-    let(:buildkit_result) { "writing image sha256:1a2b3c4d 0.0s done" }
-    let(:podman_result) { "1a2b3c4d5e6f7g8h9i10j11k12l13m14n16o17p18q19r20s21t22u23v24w25x2" }
+    let(:stdout) { "Successfully built other_package\nSuccessfully built 1a2b3c4d" }
     let(:cid) { "1a2b3c4d" }
 
     it "builds a container with standard docker" do
-      allow(subject).to receive(:execute).and_return(result)
-
       container_id = subject.build("/tmp/fakedir")
 
       expect(container_id).to eq(cid)
     end
 
-    it "builds a container with buildkit docker" do
-      allow(subject).to receive(:execute).and_return(buildkit_result)
+    context "using buildkit" do
+      let(:stdout) { "writing image sha256:1a2b3c4d 0.0s done" }
 
-      container_id = subject.build("/tmp/fakedir")
+      it "builds a container with buildkit docker" do
+        container_id = subject.build("/tmp/fakedir")
 
-      expect(container_id).to eq(cid)
+        expect(container_id).to eq(cid)
+      end
     end
 
-    it "builds a container with buildkit docker (old output)" do
-      allow(subject).to receive(:execute).and_return(buildkit_result_old)
+    context "using buildkit with old output" do
+      let(:stdout) { "writing image sha256:1a2b3c4d done" }
 
-      container_id = subject.build("/tmp/fakedir")
+      it "builds a container with buildkit docker (old output)" do
+        container_id = subject.build("/tmp/fakedir")
 
-      expect(container_id).to eq(cid)
+        expect(container_id).to eq(cid)
+      end
     end
 
-    it "builds a container with podman emulating docker CLI" do
-      allow(subject).to receive(:execute).and_return(podman_result)
-      allow(subject).to receive(:podman?).and_return(true)
+    context "using podman emulating docker CLI" do
+      let(:stdout) { "1a2b3c4d5e6f7g8h9i10j11k12l13m14n16o17p18q19r20s21t22u23v24w25x2" }
 
-      container_id = subject.build("/tmp/fakedir")
+      it "builds a container with podman emulating docker CLI" do
+        allow(subject).to receive(:podman?).and_return(true)
 
-      expect(container_id).to eq(cid)
+        container_id = subject.build("/tmp/fakedir")
+
+        expect(container_id).to eq(cid)
+      end
     end
   end
 
   describe '#podman?' do
-    let(:emulating_docker_output) { "podman version 1.7.1-dev" }
-    let(:real_docker_output) { "Docker version 1.8.1, build d12ea79" }
+    context "when docker is used" do
+      let(:stdout) { "Docker version 1.8.1, build d12ea79" }
 
-    it 'returns false when docker is used' do
-      allow(subject).to receive(:execute).and_return(real_docker_output)
-
-      expect(subject.podman?).to be false
+      it 'returns false' do
+        expect(subject.podman?).to be false
+      end
     end
+    context "when podman is used" do
+      let(:stdout) { "podman version 1.7.1-dev" }
 
-    it 'returns true when podman is used' do
-      allow(subject).to receive(:execute).and_return(emulating_docker_output)
-
-      expect(subject.podman?).to be true
+      it 'returns true' do
+        expect(subject.podman?).to be true
+      end
     end
   end
 
@@ -301,20 +325,22 @@ describe VagrantPlugins::DockerProvider::Driver do
     end
 
     context 'when container exists' do
-      before { allow(subject).to receive(:execute).and_return("foo\n#{cid}\nbar") }
+      let(:stdout) { "foo\n#{cid}\nbar" }
+
       it { expect(result).to be_truthy }
     end
 
     context 'when container does not exist' do
-      before { allow(subject).to receive(:execute).and_return("foo\n#{cid}extra\nbar") }
+      let(:stdout) { "foo\n#{cid}extra\nbar" }
+
       it { expect(result).to be_falsey }
     end
   end
 
   describe '#pull' do
     it 'should pull images' do
-      expect(subject).to receive(:execute).with('docker', 'pull', 'foo')
       subject.pull('foo')
+      expect(cmd_executed).to match(/docker pull foo/)
     end
   end
 
@@ -355,12 +381,12 @@ describe VagrantPlugins::DockerProvider::Driver do
     end
 
     context 'when container exists' do
-      before { allow(subject).to receive(:execute).and_return("foo\n#{cid}\nbar") }
+      let(:stdout) { "foo\n#{cid}\nbar" }
       it { expect(result).to be_truthy }
     end
 
     context 'when container does not exist' do
-      before { allow(subject).to receive(:execute).and_return("foo\n#{cid}extra\nbar") }
+      let(:stdout) { "foo\n#{cid}extra\nbar" }
       it { expect(result).to be_falsey }
     end
   end
@@ -382,8 +408,8 @@ describe VagrantPlugins::DockerProvider::Driver do
       before { allow(subject).to receive(:running?).and_return(true) }
 
       it 'does not start the container' do
-        expect(subject).to_not receive(:execute).with('docker', 'start', cid)
         subject.start(cid)
+        expect(cmd_executed).to be_nil
       end
     end
 
@@ -391,8 +417,8 @@ describe VagrantPlugins::DockerProvider::Driver do
       before { allow(subject).to receive(:running?).and_return(false) }
 
       it 'starts the container' do
-        expect(subject).to receive(:execute).with('docker', 'start', cid)
         subject.start(cid)
+        expect(cmd_executed).to eq("docker start #{cid}")
       end
     end
   end
@@ -402,13 +428,13 @@ describe VagrantPlugins::DockerProvider::Driver do
       before { allow(subject).to receive(:running?).and_return(true) }
 
       it 'stops the container' do
-        expect(subject).to receive(:execute).with('docker', 'stop', '-t', '1', cid)
         subject.stop(cid, 1)
+        expect(cmd_executed).to eq("docker stop -t 1 #{cid}")
       end
 
       it "stops the container with the set timeout" do
-        expect(subject).to receive(:execute).with('docker', 'stop', '-t', '5', cid)
         subject.stop(cid, 5)
+        expect(cmd_executed).to eq("docker stop -t 5 #{cid}")
       end
     end
 
@@ -416,8 +442,8 @@ describe VagrantPlugins::DockerProvider::Driver do
       before { allow(subject).to receive(:running?).and_return(false) }
 
       it 'does not stop container' do
-        expect(subject).to_not receive(:execute).with('docker', 'stop', '-t', '1', cid)
         subject.stop(cid, 1)
+        expect(cmd_executed).to be_nil
       end
     end
   end
@@ -427,8 +453,8 @@ describe VagrantPlugins::DockerProvider::Driver do
       before { allow(subject).to receive(:created?).and_return(true) }
 
       it 'removes the container' do
-        expect(subject).to receive(:execute).with('docker', 'rm', '-f', '-v', cid)
         subject.rm(cid)
+        expect(cmd_executed).to eq("docker rm -f -v #{cid}")
       end
     end
 
@@ -436,8 +462,8 @@ describe VagrantPlugins::DockerProvider::Driver do
       before { allow(subject).to receive(:created?).and_return(false) }
 
       it 'does not attempt to remove the container' do
-        expect(subject).to_not receive(:execute).with('docker', 'rm', '-f', '-v', cid)
         subject.rm(cid)
+        expect(cmd_executed).to be_nil
       end
     end
   end
@@ -447,8 +473,8 @@ describe VagrantPlugins::DockerProvider::Driver do
 
     context 'image exists' do
       it "removes the image" do
-        expect(subject).to receive(:execute).with('docker', 'rmi', id)
         subject.rmi(id)
+        expect(cmd_executed).to eq("docker rmi #{id}")
       end
     end
 
@@ -490,13 +516,11 @@ describe VagrantPlugins::DockerProvider::Driver do
   end
 
   describe '#inspect_container' do
-    let(:data) { '[{"json": "value"}]' }
-
-    before { allow(subject).to receive(:execute).and_return(data) }
+    let(:stdout) { '[{"json": "value"}]' }
 
     it 'inspects the container' do
-      expect(subject).to receive(:execute).with('docker', 'inspect', cid)
       subject.inspect_container(cid)
+      expect(cmd_executed).to eq("docker inspect #{cid}")
     end
 
     it 'parses the json output' do
@@ -505,68 +529,66 @@ describe VagrantPlugins::DockerProvider::Driver do
   end
 
   describe '#all_containers' do
-    let(:containers) { "container1\ncontainer2" }
-
-    before { allow(subject).to receive(:execute).and_return(containers) }
+    let(:stdout) { "container1\ncontainer2" }
 
     it 'returns an array of all known containers' do
-      expect(subject).to receive(:execute).with('docker', 'ps', '-a', '-q', '--no-trunc')
       expect(subject.all_containers).to eq(['container1', 'container2'])
+      expect(cmd_executed).to eq("docker ps -a -q --no-trunc")
     end
   end
 
   describe '#docker_bridge_ip' do
-    let(:containers) { " inet 123.456.789.012/16 " }
-
-    before { allow(subject).to receive(:execute).and_return(containers) }
+    let(:stdout) { " inet 123.456.789.012/16 " }
 
     it 'returns an array of all known containers' do
-      expect(subject).to receive(:execute).with('/sbin/ip', '-4', 'addr', 'show', 'scope', 'global', 'docker0')
       expect(subject.docker_bridge_ip).to eq('123.456.789.012')
+      expect(cmd_executed).to eq("/sbin/ip -4 addr show scope global docker0")
     end
   end
 
   describe '#docker_connect_network' do
     let(:opts) { ["--ip", "172.20.128.2"] }
+
     it 'connects a network to a container' do
-      expect(subject).to receive(:execute).with("docker", "network", "connect", "vagrant_network", cid, "--ip", "172.20.128.2")
       subject.connect_network("vagrant_network", cid, opts)
+      expect(cmd_executed).to eq("docker network connect vagrant_network #{cid} --ip 172.20.128.2")
     end
   end
 
   describe '#docker_create_network' do
     let(:opts) { ["--subnet", "172.20.0.0/16"] }
+
     it 'creates a network' do
-      expect(subject).to receive(:execute).with("docker", "network", "create", "vagrant_network", "--subnet", "172.20.0.0/16")
       subject.create_network("vagrant_network", opts)
+      expect(cmd_executed).to eq("docker network create vagrant_network --subnet 172.20.0.0/16")
     end
   end
 
   describe '#docker_disconnet_network' do
     it 'disconnects a network from a container' do
-      expect(subject).to receive(:execute).with("docker", "network", "disconnect", "vagrant_network", cid, "--force")
       subject.disconnect_network("vagrant_network", cid)
+      expect(cmd_executed).to eq("docker network disconnect vagrant_network #{cid} --force")
     end
   end
 
   describe '#docker_inspect_network' do
     it 'gets info about a network' do
-      expect(subject).to receive(:execute).with("docker", "network", "inspect", "vagrant_network")
       subject.inspect_network("vagrant_network")
+      expect(cmd_executed).to eq("docker network inspect vagrant_network")
     end
   end
 
   describe '#docker_list_network' do
     it 'lists docker networks' do
-      expect(subject).to receive(:execute).with("docker", "network", "ls")
       subject.list_network()
+      expect(cmd_executed).to eq("docker network ls")
     end
   end
 
   describe '#docker_rm_network' do
     it 'deletes a docker network' do
-      expect(subject).to receive(:execute).with("docker", "network", "rm", "vagrant_network")
       subject.rm_network("vagrant_network")
+      expect(cmd_executed).to eq("docker network rm vagrant_network")
     end
   end