test: cover the branches a mutant could survive

spec/kitchen/driver/gce/disk_build_spec.rb

@@ -93,6 +93,23 @@ def built_disks
           .to eq("projects/test-project/zones/test-zone-1a/disks/tk-test-1-data")
       end
 
+      # A standalone disk is not removed with the instance unless the
+      # attachment says so, so an unsent autoDelete leaves it billing after
+      # `kitchen destroy` has reported success.
+      it "auto-deletes the standalone disk with the instance" do
+        expect(built_disks.last.auto_delete).to be(true)
+      end
+
+      context "with autodelete_disk disabled on it" do
+        let(:driver_config) do
+          { disks: { boot: { boot: true }, data: { disk_size: 50, autodelete_disk: false } } }
+        end
+
+        it "leaves the standalone disk behind" do
+          expect(built_disks.last.auto_delete).to be(false)
+        end
+      end
+
       it "waits for the standalone disk to become READY" do
         expect(compute).to receive(:get_disk).at_least(:once).and_return(ComputeApi.disk(status: "READY"))
 
@@ -238,6 +255,63 @@ def built_disks
           expect(built_disks.first.initialize_params.disk_size_gb).to eq(100)
         end
       end
+
+      # `disk_size:` written bare in kitchen.yml parses to nil rather than to
+      # the default, so the comparison has to cope with having no size at all.
+      context "and the size was left empty in kitchen.yml" do
+        let(:driver_config) { { disks: { disk1: { boot: true, disk_size: nil } } } }
+
+        it "sizes the boot disk to the image rather than raising" do
+          expect(built_disks.first.initialize_params.disk_size_gb).to eq(50)
+        end
+      end
+
+      # The boundary: a request that exactly matches the image is already
+      # legal, so it must be sent unchanged and without a warning.
+      context "and exactly the image's size was requested" do
+        let(:driver_config) { { disks: { disk1: { boot: true, disk_size: 50 } } } }
+
+        it "honours the requested size" do
+          expect(built_disks.first.initialize_params.disk_size_gb).to eq(50)
+        end
+
+        it "does not claim the request was too small" do
+          built_disks
+
+          expect(log).not_to include("smaller than image")
+        end
+      end
+    end
+
+    describe "reading the image's size" do
+      # The boot image is looked up more than once during a single create --
+      # once to validate it, once to size the disk from it.
+      it "asks GCE for an image's size only once" do
+        expect(compute).to receive(:get_image).at_most(:once).and_return(ComputeApi.image)
+
+        driver.image_disk_size_gb("test-image")
+        driver.image_disk_size_gb("test-image")
+      end
+
+      it "has no size to report for no image" do
+        expect(driver.image_disk_size_gb(nil)).to be_nil
+      end
+
+      # A size that cannot be read is not a reason to fail the run: the
+      # requested size is sent as-is and GCE decides.
+      context "when the image lookup fails" do
+        before { allow(compute).to receive(:get_image).and_raise(ComputeApi.client_error) }
+
+        it "reports no size rather than raising" do
+          expect(driver.image_disk_size_gb("test-image")).to be_nil
+        end
+
+        it "says why in the debug log" do
+          driver.image_disk_size_gb("test-image")
+
+          expect(log).to include("Unable to read the size of image test-image")
+        end
+      end
     end
 
     context "when an extra disk's custom image is larger than its requested size" do

spec/kitchen/driver/gce/instance_payload_spec.rb

@@ -40,9 +40,71 @@
       end
     end
 
-    it "builds an instance the Google client recognises" do
+  end
+
+  # Every builder below is exercised in isolation further down this file. This
+  # block proves each one is actually wired into the object that reaches
+  # `insert_instance`, which is the one failure an isolated example cannot see:
+  # a dropped assignment ships an instance with no tags, no labels and no
+  # scheduling, and every per-builder example still passes.
+  describe "the payload handed to insert_instance" do
+    subject(:payload) { created_instance_payload }
+
+    let(:driver_config) do
+      {
+        tags: %w{web db},
+        labels: { "team" => "platform" },
+        metadata: { "owner" => "platform" },
+        guest_accelerators: [{ type: "nvidia-tesla-t4", count: 2 }],
+        service_account_scopes: ["storage-ro"],
+      }
+    end
+
+    it "is an instance the Google client recognises" do
       expect(payload).to be_a(Google::Apis::ComputeV1::Instance)
     end
+
+    it "attaches the disks" do
+      expect(payload.disks).not_to be_empty
+      expect(payload.disks).to all(be_a(Google::Apis::ComputeV1::AttachedDisk))
+    end
+
+    it "attaches the guest accelerators" do
+      expect(payload.guest_accelerators.map(&:accelerator_count)).to eq([2])
+    end
+
+    it "attaches the metadata" do
+      expect(payload.metadata.items.map(&:key)).to include("owner", "created-by")
+    end
+
+    it "attaches the network interface" do
+      expect(payload.network_interfaces.size).to eq(1)
+    end
+
+    it "attaches the scheduling options" do
+      expect(payload.scheduling).to be_a(Google::Apis::ComputeV1::Scheduling)
+    end
+
+    it "attaches the service account and its scopes" do
+      expect(payload.service_accounts.first.scopes)
+        .to eq(["https://www.googleapis.com/auth/devstorage.read_only"])
+    end
+
+    it "attaches the network tags" do
+      expect(payload.tags.items).to eq(%w{web db})
+    end
+
+    it "attaches the labels" do
+      expect(payload.labels).to eq("team" => "platform")
+    end
+
+    context "with no service account scopes" do
+      let(:driver_config) { { service_account_scopes: [] } }
+
+      it "attaches no service account at all" do
+        expect(payload.service_accounts).to be_nil
+      end
+    end
   end
 
   describe "#instance_scheduling" do
@@ -385,6 +447,16 @@
         expect(driver.instance_service_accounts).to be_nil
       end
     end
+
+    # `service_account_scopes:` written bare in kitchen.yml parses to nil
+    # rather than to an empty array, and nil has no #empty?.
+    context "with the scope list present but empty in kitchen.yml" do
+      let(:driver_config) { { service_account_scopes: nil } }
+
+      it "is nil rather than raising" do
+        expect(driver.instance_service_accounts).to be_nil
+      end
+    end
   end
 
   describe "#service_account_scope_url" do

spec/kitchen/driver/gce/lifecycle_spec.rb

@@ -56,6 +56,45 @@
       driver.create(server_name: "already-created")
     end
 
+    # The instance name is budgeted against the longest configured disk name,
+    # so the disk configuration has to be settled before the name is generated.
+    # Only a create can catch a reordering: `generate_server_name` called on
+    # its own is always handed a config that has already been normalised.
+    # The default disk configuration is the case that matters: `config[:disks]`
+    # is nil until `create_disks_config` fills it in, so a name generated first
+    # is budgeted against no disk name at all.
+    context "with names that only fit if the disks are configured first" do
+      let(:kitchen_instance_name) { "long-suite-name-to-overflow-disk-naming-ubuntu-2204" }
+
+      it "sends no disk name longer than GCE allows" do
+        disk_names = created_instance_payload.disks.map { |d| d.initialize_params.disk_name }.compact
+
+        expect(disk_names).not_to be_empty
+        expect(disk_names.map(&:length))
+          .to all(be <= Kitchen::Driver::Gce::MAX_INSTANCE_NAME_LENGTH)
+      end
+    end
+
+    # update_windows_password is covered on its own in windows_spec.rb; what
+    # this adds is that a create actually calls it, without which a Windows
+    # suite comes up and then fails WinRM auth with no password in state.
+    context "with a WinRM transport" do
+      let(:transport_name) { "winrm" }
+      let(:transport_username) { "kitchen" }
+      let(:driver_config) { { email: "user@example.com" } }
+
+      it "resets the Windows password and records it in the state file" do
+        allow_successful_create
+        allow(Kitchen::Driver::Gce::WindowsPassword).to receive(:new).and_return(
+          instance_double(Kitchen::Driver::Gce::WindowsPassword, new_password: "s3cret")
+        )
+
+        driver.create(state)
+
+        expect(state[:password]).to eq("s3cret")
+      end
+    end
+
     context "when use_private_ip is set" do
       let(:driver_config) { { use_private_ip: true } }
 
@@ -219,6 +258,23 @@
       driver.destroy({})
     end
 
+    # An instance holds its disks until it is gone, so deleting them first
+    # fails and leaves them behind, billing.
+    it "deletes the instance before the disks it is holding" do
+      order = []
+      allow(compute).to receive(:get_instance).and_return(ComputeApi.instance)
+      allow(compute).to receive(:get_disk).and_return(ComputeApi.disk)
+      allow(compute).to receive(:get_zone_operation).and_return(ComputeApi.operation)
+      allow(compute).to receive(:delete_instance) { order << :instance; ComputeApi.operation }
+      allow(compute).to receive(:delete_disk) { order << :disk; ComputeApi.operation }
+
+      driver.destroy(
+        server_name: "tk-test-1", zone: "test-zone-1a", created_disks: ["tk-test-1-data"]
+      )
+
+      expect(order).to eq(%i{instance disk})
+    end
+
     it "deletes the instance and clears the state file" do
       state = { server_name: "tk-test-1", hostname: "203.0.113.4", zone: "test-zone-1a" }
       allow(compute).to receive(:get_instance).and_return(ComputeApi.instance)
@@ -430,6 +486,27 @@ def generated_name
         end
       end
 
+      # The budget is inclusive: a name exactly as long as it allows is legal,
+      # and only one character more has to give way to the fallback. Off by one
+      # in either direction is a 64-character disk name or a Test Kitchen name
+      # discarded for no reason.
+      context "with a name exactly as long as the budget allows" do
+        let(:kitchen_instance_name) { "a" * 47 }
+
+        it "keeps the Test Kitchen name" do
+          expect(generated_name).to match(/\Atk-a{47}-[0-9a-f]{6}\z/)
+        end
+      end
+
+      context "with a name one character longer than the budget allows" do
+        let(:kitchen_instance_name) { "a" * 48 }
+
+        it "gives way to the fallback so the disk name still fits" do
+          expect("#{generated_name}-disk1".length)
+            .to be <= Kitchen::Driver::Gce::MAX_INSTANCE_NAME_LENGTH
+        end
+      end
+
       context "with a disk name that leaves room for no instance name at all" do
         let(:driver_config) { { disks: { "#{"d" * 60}": { boot: true } } } }
 

spec/kitchen/driver/gce/validation_spec.rb

@@ -203,6 +203,20 @@
         end
       end
 
+      # GCE will not live-migrate an instance with a GPU attached, so the
+      # driver turns auto-migrate off whatever the user asked for.
+      context "with guest accelerators and auto-migrate left on" do
+        let(:driver_config) do
+          { guest_accelerators: [{ type: "nvidia-tesla-t4", count: 1 }], auto_migrate: true }
+        end
+
+        it "warns that auto-migrate is disabled" do
+          driver.validate!
+
+          expect(log).to include("Auto-migrate disabled for instance with guest accelerators")
+        end
+      end
+
       context "with a WinRM transport logging in as the built-in Administrator" do
         let(:transport_name) { "winrm" }
         let(:transport_username) { "Administrator" }

spec/kitchen/driver/gce/windows_password_spec.rb

@@ -82,29 +82,70 @@ def serial_port_returns(*lines)
       expect(winpass.new_password).to eq("pässwörd-✓")
     end
 
+    # The port is scanned newest line first, so the noise that has to be
+    # survived is the noise *after* the response: those are the lines actually
+    # parsed on the way back to it. Noise before it is never even read.
     it "ignores serial port noise surrounding the response" do
       serial_port_returns(
         "SeaBIOS (version 1.8.9-google)",
+        agent_response(password: "found-me").to_json,
         "not json at all {{{",
-        "12345",
         '{"unrelated":"event"}',
-        agent_response(password: "found-me").to_json,
         "Booting from Hard Disk..."
       )
 
       expect(winpass.new_password).to eq("found-me")
     end
 
+    # A bare scalar parses as valid JSON but is not a Hash, so indexing it for
+    # a modulus raises TypeError rather than simply failing to match.
+    it "skips a line that is valid JSON but not an object" do
+      serial_port_returns(
+        agent_response(password: "found-me").to_json,
+        "12345",
+        '"a bare string"',
+        "null"
+      )
+
+      expect(winpass.new_password).to eq("found-me")
+    end
+
+    # Newest last, so the stale response is the one examined first. Were the
+    # key check dropped, this would decrypt to the stale entry's password.
     it "ignores a response generated for a different key" do
       other_key = OpenSSL::PKey::RSA.new(2048)
-      stale = agent_response.merge(
+      stale = agent_response(password: "not-mine").merge(
         "modulus" => Base64.strict_encode64(other_key.public_key.n.to_s(2))
       )
-      serial_port_returns(stale.to_json, agent_response(password: "mine").to_json)
+      serial_port_returns(agent_response(password: "mine").to_json, stale.to_json)
 
       expect(winpass.new_password).to eq("mine")
     end
 
+    # Same again for the exponent half of the pair, which is the easier of the
+    # two to drop by accident because every RSA key here shares one.
+    it "ignores a response whose exponent does not match" do
+      stale = agent_response(password: "not-mine").merge(
+        "exponent" => Base64.strict_encode64(OpenSSL::BN.new(3).to_s(2))
+      )
+      serial_port_returns(agent_response(password: "mine").to_json, stale.to_json)
+
+      expect(winpass.new_password).to eq("mine")
+    end
+
+    it "reads the serial port the Windows agent writes to" do
+      expect(compute).to receive(:get_instance_serial_port_output)
+        .with("test-project", "test-zone-1a", "tk-win-1", port: described_class::SERIAL_PORT)
+        .and_return(
+          instance_double(
+            Google::Apis::ComputeV1::SerialPortOutput,
+            contents: agent_response(password: "on-port-4").to_json
+          )
+        )
+
+      expect(winpass.new_password).to eq("on-port-4")
+    end
+
     it "raises when the agent reports it could not reset the password" do
       serial_port_returns(agent_response(found: false).to_json)
 
@@ -189,6 +230,21 @@ def serial_port_returns(*lines)
       expect(request["userName"]).to eq("Administrator")
     end
 
+    # Google's images ship the built-in Administrator disabled and the agent
+    # will not enable it, so a configured username reaching the agent
+    # unchanged is what makes a Windows suite work at all.
+    context "with an account other than the built-in Administrator" do
+      subject(:winpass) do
+        described_class.new(
+          driver, instance_name: "tk-win-1", email: "user@example.com", username: "kitchen"
+        )
+      end
+
+      it "asks the agent to reset that account instead" do
+        expect(request["userName"]).to eq("kitchen")
+      end
+    end
+
     it "carries the requesting user's email" do
       expect(request["email"]).to eq("user@example.com")
     end