From 2d648d901dd033f350e65398bb2fa4a8b7b32a61 Mon Sep 17 00:00:00 2001
From: Tim Smith <tim@mondoo.com>
Date: Sat, 29 Aug 2026 18:45:18 -0700
Subject: [PATCH 1/3] test: modernize the RSpec suite and cover the workflow
 logic

The spec_helper was an empty license header: no requires, no RSpec
configuration, and nothing in `.rspec` to load it. Every spec file had to
require its own dependencies, and none of the guardrails that catch stale
tests were switched on.

This wires the suite up properly:

- `.rspec` now passes `--require spec_helper`, so the helper is the single
  place that loads the driver and its Test Kitchen dummies.
- `spec_helper` turns on `verify_partial_doubles`, `disable_monkey_patching!`,
  expect-only syntax, `raise_errors_for_deprecations!`, and random ordering
  with a printed seed.
- `spec/support/vro_doubles.rb` builds verifying doubles for the slice of the
  vcoworkflows API the driver touches, so a rename upstream fails the suite
  instead of passing against a stub that no longer matches.
- Specs are no longer excluded from cookstyle, so they are held to the same
  style as `lib/`.

It also fills in the logic the old suite left uncovered. Previously `#create`
and `#destroy` were only checked by stubbing the driver's own methods and
asserting they were called, which proves nothing about what the driver
actually sends to vRO. There are now end-to-end examples that stub only at the
`VcoWorkflows::Workflow` boundary and assert the real behaviour: which
workflow gets looked up, that configured create and destroy parameters are
pushed to it, that `server_id` reaches the destroy workflow, and that the
create output lands in instance state.

New coverage for the branches where bugs can hide:

- the poll loop in `#wait_for_workflow` -- how many times the token is
  re-read, that it sleeps two seconds between polls, that it does not sleep
  when the run is already finished, and that a real elapsed timeout fires
- `request_timeout` appearing in the timeout message when it is not the default
- `#set_workflow_parameters` stringifying symbol keys, passing non-string
  values through, and no-opping on an empty hash
- `#output_parameter_value` stringifying numbers and unset values
- `vro_disable_ssl_verify` reaching `VcoWorkflows::Config`
- `#execute_create_workflow` not validating or writing state when the run failed
- memoization of `vro_config`, `vro_client`, and `output_parameters`

Also removes a `Struct.new` assigned to a top-level constant inside an
example, which leaked a constant into the whole suite and warned on redefine,
and corrects the spec path in CONTRIBUTING.md.

51 examples became 77.

Signed-off-by: Tim Smith <tim@mondoo.com>
---
 .rspec                      |   1 +
 .rubocop.yml                |   1 -
 CONTRIBUTING.md             |   2 +-
 spec/spec_helper.rb         |  52 +++
 spec/support/vro_doubles.rb |  40 ++
 spec/vro_spec.rb            | 708 ++++++++++++++++++++++--------------
 6 files changed, 524 insertions(+), 280 deletions(-)
 create mode 100644 spec/support/vro_doubles.rb

diff --git a/.rspec b/.rspec
index 8c18f1a..7a2cc1a 100644
--- a/.rspec
+++ b/.rspec
@@ -1,2 +1,3 @@
+--require spec_helper
 --format documentation
 --color
diff --git a/.rubocop.yml b/.rubocop.yml
index 5679916..88d6ac0 100644
--- a/.rubocop.yml
+++ b/.rubocop.yml
@@ -6,4 +6,3 @@ AllCops:
   TargetRubyVersion: 3.1
   Exclude:
     - "vendor/**/*"
-    - "spec/**/*"
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index c01c1fa..c959d28 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -40,7 +40,7 @@ bundle exec rake style   # Cookstyle / RuboCop
 To run a single spec file:
 
 ```sh
-bundle exec rspec spec/kitchen/driver/vro_spec.rb
+bundle exec rspec spec/vro_spec.rb
 ```
 
 Many style offenses can be corrected automatically:
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index 30e9e58..cbddcf6 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -15,3 +15,55 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 #
+
+require "rspec"
+
+require "kitchen"
+require "kitchen/driver/vro"
+require "kitchen/provisioner/dummy"
+require "kitchen/transport/dummy"
+require "kitchen/verifier/dummy"
+
+Dir[File.join(__dir__, "support", "**", "*.rb")].sort.each { |f| require f }
+
+# These specs never talk to a vRO appliance. Everything this driver does is
+# turn configuration into vcoworkflows calls and turn workflow output back
+# into instance state, so all of it can be exercised against doubles of the
+# vcoworkflows API. Anything that genuinely needs an appliance belongs in a
+# manual run against a real vRO -- see CONTRIBUTING.md.
+RSpec.configure do |config|
+  config.expect_with :rspec do |expectations|
+    # `should` has been deprecated since RSpec 3. Only `expect` is available.
+    expectations.syntax = :expect
+    expectations.include_chain_clauses_in_custom_matcher_descriptions = true
+  end
+
+  config.mock_with :rspec do |mocks|
+    mocks.syntax = :expect
+
+    # Fail if an example stubs a method the real object does not have. Without
+    # this, a rename in lib/ leaves the specs stubbing a method that no longer
+    # exists and passing while the driver is broken.
+    mocks.verify_partial_doubles = true
+  end
+
+  # No `describe` at the top level and no `should` on every object: the specs
+  # use `RSpec.describe` and `expect` explicitly.
+  config.disable_monkey_patching!
+
+  # Surface deprecations as failures rather than warnings that scroll past.
+  config.raise_errors_for_deprecations!
+
+  config.shared_context_metadata_behavior = :apply_to_host_groups
+
+  config.include VroDoubles
+
+  config.filter_run_when_matching :focus
+
+  # Run specs in random order to surface order dependencies. If you find an
+  # order dependency and want to debug it, you can fix the order by providing
+  # the seed, which is printed after each run.
+  #     --seed 1234
+  config.order = "random"
+  Kernel.srand config.seed
+end
diff --git a/spec/support/vro_doubles.rb b/spec/support/vro_doubles.rb
new file mode 100644
index 0000000..7010754
--- /dev/null
+++ b/spec/support/vro_doubles.rb
@@ -0,0 +1,40 @@
+#
+# License:: Apache License, Version 2.0
+#
+# Verifying doubles for the small slice of the vcoworkflows API that this
+# driver uses. Building them here keeps the specs honest -- if vcoworkflows
+# renames `token` or drops `output_parameters`, every example that leans on
+# these helpers fails loudly instead of passing against a stale stub.
+#
+module VroDoubles
+  # @param value [Object] the value vRO returned for the parameter
+  # @return [VcoWorkflows::WorkflowParameter] a verifying double
+  def workflow_parameter(value)
+    instance_double(VcoWorkflows::WorkflowParameter, value: value)
+  end
+
+  # @param output [Hash{String => Object}] output parameter names to raw values
+  # @return [Hash{String => VcoWorkflows::WorkflowParameter}]
+  def workflow_output(output)
+    output.transform_values { |value| workflow_parameter(value) }
+  end
+
+  # @param state [String] the workflow run state vRO reports
+  # @param alive [Boolean, Array<Boolean>] whether the run is still going; an
+  #   array is returned one element per call, so a poll loop can be driven
+  # @param output [Hash{String => Object}] output parameter names to raw values
+  # @return [VcoWorkflows::WorkflowToken] a verifying double
+  def workflow_token(state: "completed", alive: false, output: {})
+    token = instance_double(VcoWorkflows::WorkflowToken,
+      state: state,
+      output_parameters: workflow_output(output))
+    allow(token).to receive(:alive?).and_return(*Array(alive))
+    token
+  end
+
+  # @param token [VcoWorkflows::WorkflowToken] the token the client hands back
+  # @return [VcoWorkflows::Workflow] a verifying double
+  def workflow_client(token: workflow_token)
+    instance_double(VcoWorkflows::Workflow, execute: nil, parameter: nil, token: token)
+  end
+end
diff --git a/spec/vro_spec.rb b/spec/vro_spec.rb
index ba3546b..bff6df9 100644
--- a/spec/vro_spec.rb
+++ b/spec/vro_spec.rb
@@ -1,5 +1,3 @@
-# Encoding: UTF-8
-
 #
 # Authors:: Chef Partner Engineering (<partnereng@chef.io>)
 # Copyright:: Copyright (c) 2015 Chef Software, Inc.
@@ -17,77 +15,76 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-require 'spec_helper'
-require 'kitchen/driver/vro'
-require 'kitchen/provisioner/dummy'
-require 'kitchen/transport/dummy'
-require 'kitchen/verifier/dummy'
-
-shared_examples 'output parameters that are missing a required key' do
-  it 'raises an exception' do
+RSpec.shared_examples "output parameters that are missing a required key" do
+  it "raises an exception" do
     allow(driver).to receive(:output_parameters).and_return(output_parameters)
+
     expect { driver.validate_create_output_parameters! }.to raise_error(
       RuntimeError,
-      'The workflow output did not contain a server_id and ip_address parameter.'
+      "The workflow output did not contain a server_id and ip_address parameter."
     )
   end
 end
 
-describe Kitchen::Driver::Vro do
+RSpec.describe Kitchen::Driver::Vro do
   let(:logged_output) { StringIO.new }
   let(:logger)        { Logger.new(logged_output) }
-  let(:platform)      { Kitchen::Platform.new(name: 'fake_platform') }
+  let(:platform)      { Kitchen::Platform.new(name: "fake_platform") }
   let(:transport)     { Kitchen::Transport::Dummy.new }
-  let(:driver)        { Kitchen::Driver::Vro.new(config) }
+  let(:driver)        { described_class.new(config) }
 
   let(:config) do
     {
-      vro_username:          'myuser',
-      vro_password:          'mypassword',
-      vro_base_url:          'https://vra.corp.local:8281',
-      create_workflow_name:  'Create Workflow',
-      create_workflow_id:    'workflow-1',
-      destroy_workflow_name: 'Destroy Workflow',
-      destroy_workflow_id:   'workflow-2'
+      vro_username: "myuser",
+      vro_password: "mypassword",
+      vro_base_url: "https://vra.corp.local:8281",
+      create_workflow_name: "Create Workflow",
+      create_workflow_id: "workflow-1",
+      destroy_workflow_name: "Destroy Workflow",
+      destroy_workflow_id: "workflow-2",
     }
   end
 
   let(:instance) do
     instance_double(Kitchen::Instance,
-                    logger:    logger,
-                    transport: transport,
-                    platform:  platform,
-                    to_str:    'instance_str')
+      logger: logger,
+      transport: transport,
+      platform: platform,
+      to_str: "instance_str")
   end
 
   before do
     allow(driver).to receive(:instance).and_return(instance)
   end
 
-  describe 'plugin metadata' do
-    it 'is a Test Kitchen driver' do
+  describe "plugin metadata" do
+    it "is a Test Kitchen driver" do
       expect(driver).to be_a(Kitchen::Driver::Base)
     end
 
-    it 'shows up as vRO in kitchen list' do
-      expect(driver.name).to eq('vRO')
+    it "shows up as vRO in kitchen list" do
+      expect(driver.name).to eq("vRO")
     end
 
-    it 'reports its own gem version to kitchen diagnose' do
+    it "reports its own gem version to kitchen diagnose" do
       expect(driver.diagnose_plugin[:version]).to eq(Kitchen::Driver::VRO_VERSION)
     end
+
+    it "speaks version 2 of the driver API" do
+      expect(driver.diagnose_plugin[:api_version]).to eq(2)
+    end
   end
 
-  describe 'configuration' do
-    it 'verifies TLS by default' do
-      expect(driver[:vro_disable_ssl_verify]).to eq(false)
+  describe "configuration" do
+    it "verifies TLS by default" do
+      expect(driver[:vro_disable_ssl_verify]).to be false
     end
 
-    it 'defaults to a 300 second workflow timeout' do
+    it "defaults to a 300 second workflow timeout" do
       expect(driver[:request_timeout]).to eq(300)
     end
 
-    it 'defaults both workflow ids to nil so the name is used alone' do
+    it "defaults both workflow ids to nil so the name is used alone" do
       config.delete(:create_workflow_id)
       config.delete(:destroy_workflow_id)
 
@@ -95,13 +92,14 @@
       expect(driver[:destroy_workflow_id]).to be_nil
     end
 
-    it 'defaults both workflow parameter sets to empty' do
+    it "defaults both workflow parameter sets to empty" do
       expect(driver[:create_workflow_parameters]).to eq({})
       expect(driver[:destroy_workflow_parameters]).to eq({})
     end
 
-    %i[vro_username vro_password vro_base_url
-       create_workflow_name destroy_workflow_name].each do |key|
+    required = %i{vro_username vro_password vro_base_url create_workflow_name destroy_workflow_name}
+
+    required.each do |key|
       it "refuses to run without #{key}" do
         config.delete(key)
 
@@ -109,147 +107,217 @@
           .to raise_error(Kitchen::UserError, /#{key}/)
       end
     end
+
+    it "accepts a configuration that supplies every required key" do
+      expect { driver.finalize_config!(instance) }.not_to raise_error
+    end
   end
 
-  describe '#set_workflow_vars' do
-    it 'points the driver at the named workflow' do
-      driver.set_workflow_vars('Some Workflow', 'workflow-9')
+  describe "#vro_config" do
+    it "passes the configured endpoint and credentials through to vcoworkflows" do
+      vro_config = driver.vro_config
+
+      expect(vro_config).to be_a(VcoWorkflows::Config)
+      expect(vro_config.username).to eq("myuser")
+      expect(vro_config.password).to eq("mypassword")
+      expect(vro_config.url).to start_with("https://vra.corp.local:8281")
+    end
 
-      expect(driver.workflow_name).to eq('Some Workflow')
-      expect(driver.workflow_id).to eq('workflow-9')
+    it "asks vcoworkflows to verify TLS by default" do
+      expect(driver.vro_config.verify_ssl).to be true
     end
 
-    it 'discards the memoized client so the next call builds a new one' do
-      first = double('first client')
-      allow(VcoWorkflows::Workflow).to receive(:new).and_return(first)
-      allow(driver).to receive(:vro_config).and_return(double('config'))
-      driver.set_workflow_vars('Create Workflow', 'workflow-1')
-      expect(driver.vro_client).to eq(first)
+    it "asks vcoworkflows to skip TLS verification when it is disabled" do
+      config[:vro_disable_ssl_verify] = true
 
-      second = double('second client')
-      allow(VcoWorkflows::Workflow).to receive(:new).and_return(second)
-      driver.set_workflow_vars('Destroy Workflow', 'workflow-2')
+      expect(driver.vro_config.verify_ssl).to be false
+    end
 
-      expect(driver.vro_client).to eq(second)
+    it "builds the config only once" do
+      expect(driver.vro_config).to equal(driver.vro_config)
     end
   end
 
-  describe '#workflow_successful?' do
-    let(:vro_client) { double('vro_client') }
+  describe "#verify_ssl?" do
+    it "is false when vro_disable_ssl_verify is true" do
+      config[:vro_disable_ssl_verify] = true
 
-    before do
-      allow(driver).to receive(:vro_client).and_return(vro_client)
+      expect(driver.verify_ssl?).to be false
     end
 
-    it 'is true when vRO reports the run completed' do
-      allow(vro_client).to receive(:token).and_return(double(state: 'completed'))
+    it "is true when vro_disable_ssl_verify is false" do
+      config[:vro_disable_ssl_verify] = false
 
-      expect(driver.workflow_successful?).to eq(true)
+      expect(driver.verify_ssl?).to be true
     end
+  end
 
-    it 'is false for any other state' do
-      allow(vro_client).to receive(:token).and_return(double(state: 'failed'))
+  describe "#vro_client" do
+    let(:vro_config) { instance_double(VcoWorkflows::Config) }
 
-      expect(driver.workflow_successful?).to eq(false)
+    before do
+      allow(driver).to receive(:vro_config).and_return(vro_config)
     end
-  end
 
-  describe '#create' do
-    context 'when a server already exists' do
-      let(:state) { { server_id: 'server-12345' } }
+    it "creates a VcoWorkflows::Workflow for the current workflow" do
+      driver.set_workflow_vars("workflow name", "workflow-12345")
 
-      it 'does not create the server' do
-        expect(driver).not_to receive(:execute_create_workflow)
+      expect(VcoWorkflows::Workflow).to receive(:new)
+        .with("workflow name", id: "workflow-12345", config: vro_config)
 
-        driver.create(state)
-      end
+      driver.vro_client
     end
 
-    let(:state) { {} }
+    it "builds the client only once" do
+      allow(VcoWorkflows::Workflow).to receive(:new).and_return(workflow_client)
 
-    it 'calls the expected methods' do
-      expect(driver).to receive(:execute_create_workflow).with(state)
-      expect(driver).to receive(:wait_for_server).with(state)
+      driver.vro_client
+      driver.vro_client
 
-      driver.create(state)
+      expect(VcoWorkflows::Workflow).to have_received(:new).once
     end
   end
 
-  describe '#destroy' do
-    context 'when a server does not exist' do
-      let(:state) { {} }
+  describe "#set_workflow_vars" do
+    it "points the driver at the named workflow" do
+      driver.set_workflow_vars("Some Workflow", "workflow-9")
 
-      it 'does not destroy the server' do
-        expect(driver).not_to receive(:execute_destroy_workflow)
+      expect(driver.workflow_name).to eq("Some Workflow")
+      expect(driver.workflow_id).to eq("workflow-9")
+    end
 
-        driver.destroy(state)
-      end
+    it "accepts a nil id for a workflow addressed by name alone" do
+      driver.set_workflow_vars("Some Workflow", nil)
+
+      expect(driver.workflow_id).to be_nil
     end
 
-    let(:state) { { server_id: 'server-12345' } }
+    it "discards the memoized client so the next call builds a new one" do
+      first  = workflow_client
+      second = workflow_client
+      allow(driver).to receive(:vro_config).and_return(instance_double(VcoWorkflows::Config))
+      allow(VcoWorkflows::Workflow).to receive(:new).and_return(first, second)
 
-    it 'calls the expected methods' do
-      expect(driver).to receive(:execute_destroy_workflow).with(state)
-      driver.destroy(state)
+      driver.set_workflow_vars("Create Workflow", "workflow-1")
+      expect(driver.vro_client).to equal(first)
+
+      driver.set_workflow_vars("Destroy Workflow", "workflow-2")
+      expect(driver.vro_client).to equal(second)
     end
   end
 
-  describe '#vro_config' do
-    it 'creates a VcoWorkflows::Config object' do
-      expect(VcoWorkflows::Config).to receive(:new).with(url: 'https://vra.corp.local:8281',
-                                                         username: 'myuser',
-                                                         password: 'mypassword',
-                                                         verify_ssl: true)
-      driver.vro_config
+  describe "#create" do
+    let(:state) { {} }
+
+    it "runs the create workflow and then waits for the server" do
+      expect(driver).to receive(:execute_create_workflow).with(state).ordered
+      expect(driver).to receive(:wait_for_server).with(state).ordered
+
+      driver.create(state)
     end
-  end
 
-  describe '#vro_client' do
-    let(:vro_config) { double('vro_config') }
-    it 'creates a VcoWorkflows::Workflow object' do
-      allow(driver).to receive(:workflow_name).and_return('workflow name')
-      allow(driver).to receive(:workflow_id).and_return('workflow-12345')
-      allow(driver).to receive(:vro_config).and_return(vro_config)
+    it "does nothing when state already names a server" do
+      state[:server_id] = "server-12345"
 
-      expect(VcoWorkflows::Workflow).to receive(:new).with('workflow name',
-                                                           id: 'workflow-12345',
-                                                           config: vro_config)
+      expect(driver).not_to receive(:execute_create_workflow)
+      expect(driver).not_to receive(:wait_for_server)
 
-      driver.vro_client
+      driver.create(state)
     end
-  end
 
-  describe '#verify_ssl?' do
-    context 'when vro_disable_ssl_verify is true' do
+    context "with the vRO API stubbed end to end" do
+      let(:token) do
+        workflow_token(output: { "server_id" => "server-12345", "ip_address" => "1.2.3.4" })
+      end
+      let(:client)     { workflow_client(token: token) }
+      let(:connection) { transport.connection(state) }
+
       before do
-        config[:vro_disable_ssl_verify] = true
+        allow(VcoWorkflows::Workflow).to receive(:new).and_return(client)
+        allow(transport).to receive(:connection).and_return(connection)
+        allow(connection).to receive(:wait_until_ready)
       end
 
-      it 'returns false' do
-        expect(driver.verify_ssl?).to eq(false)
+      it "records the server the workflow reported" do
+        driver.create(state)
+
+        expect(state[:server_id]).to eq("server-12345")
+        expect(state[:hostname]).to eq("1.2.3.4")
       end
+
+      it "waits for the transport to accept a connection" do
+        driver.create(state)
+
+        expect(connection).to have_received(:wait_until_ready)
+      end
+
+      it "asks vRO for the configured create workflow" do
+        driver.create(state)
+
+        expect(VcoWorkflows::Workflow).to have_received(:new)
+          .with("Create Workflow", hash_including(id: "workflow-1"))
+      end
+
+      it "sends the configured create parameters to the workflow" do
+        config[:create_workflow_parameters] = { os_name: "centos", cpus: 4 }
+
+        driver.create(state)
+
+        expect(client).to have_received(:parameter).with("os_name", "centos")
+        expect(client).to have_received(:parameter).with("cpus", 4)
+      end
+    end
+  end
+
+  describe "#destroy" do
+    let(:state) { { server_id: "server-12345", hostname: "1.2.3.4" } }
+
+    it "runs the destroy workflow" do
+      expect(driver).to receive(:execute_destroy_workflow).with(state)
+
+      driver.destroy(state)
+    end
+
+    it "does nothing when state does not name a server" do
+      expect(driver).not_to receive(:execute_destroy_workflow)
+
+      driver.destroy({})
     end
 
-    context 'when vro_disable_ssl_verify is false' do
+    context "with the vRO API stubbed end to end" do
+      let(:client) { workflow_client }
+
       before do
-        config[:vro_disable_ssl_verify] = false
+        allow(VcoWorkflows::Workflow).to receive(:new).and_return(client)
+      end
+
+      it "asks vRO for the configured destroy workflow" do
+        driver.destroy(state)
+
+        expect(VcoWorkflows::Workflow).to have_received(:new)
+          .with("Destroy Workflow", hash_including(id: "workflow-2"))
       end
 
-      it 'returns true' do
-        expect(driver.verify_ssl?).to eq(true)
+      it "hands the stored server id to the destroy workflow" do
+        driver.destroy(state)
+
+        expect(client).to have_received(:parameter).with("server_id", "server-12345")
+      end
+
+      it "sends the configured destroy parameters to the workflow" do
+        config[:destroy_workflow_parameters] = { force: true }
+
+        driver.destroy(state)
+
+        expect(client).to have_received(:parameter).with("force", true)
       end
     end
   end
 
-  describe '#execute_create_workflow' do
-    let(:state)      { {} }
-    let(:server_id)  { double('server_id', value: 'server-12345') }
-    let(:ip_address) { double('ip_address', value: '1.2.3.4') }
+  describe "#execute_create_workflow" do
+    let(:state) { {} }
     let(:output_parameters) do
-      {
-        'server_id'  => server_id,
-        'ip_address' => ip_address
-      }
+      workflow_output("server_id" => "server-12345", "ip_address" => "1.2.3.4")
     end
 
     before do
@@ -262,8 +330,8 @@
       allow(driver).to receive(:output_parameters).and_return(output_parameters)
     end
 
-    it 'calls the expected methods' do
-      expect(driver).to receive(:set_workflow_vars).with('Create Workflow', 'workflow-1')
+    it "selects the create workflow and submits it" do
+      expect(driver).to receive(:set_workflow_vars).with("Create Workflow", "workflow-1")
       expect(driver).to receive(:set_workflow_parameters).with({})
       expect(driver).to receive(:execute_workflow)
       expect(driver).to receive(:wait_for_workflow)
@@ -272,26 +340,42 @@
       driver.execute_create_workflow(state)
     end
 
-    it 'raises an error if the workflow did not complete successfully' do
+    it "raises an error if the workflow did not complete successfully" do
       allow(driver).to receive(:workflow_successful?).and_return(false)
+
       expect { driver.execute_create_workflow(state) }.to raise_error(RuntimeError)
     end
 
-    it 'sets the state hash with the proper info' do
+    it "does not validate the output of a workflow that failed" do
+      allow(driver).to receive(:workflow_successful?).and_return(false)
+
+      expect(driver).not_to receive(:validate_create_output_parameters!)
+
+      expect { driver.execute_create_workflow(state) }.to raise_error(RuntimeError)
+    end
+
+    it "sets the state hash with the proper info" do
       driver.execute_create_workflow(state)
 
-      expect(state[:server_id]).to eq('server-12345')
-      expect(state[:hostname]).to eq('1.2.3.4')
+      expect(state[:server_id]).to eq("server-12345")
+      expect(state[:hostname]).to eq("1.2.3.4")
+    end
+
+    it "leaves state untouched when the output does not validate" do
+      allow(driver).to receive(:validate_create_output_parameters!)
+        .and_raise(RuntimeError, "The server_id parameter was empty.")
+
+      expect { driver.execute_create_workflow(state) }.to raise_error(RuntimeError)
+      expect(state).to be_empty
     end
   end
 
-  describe '#execute_destroy_workflow' do
-    let(:state)      { { server_id: 'server-12345' } }
-    let(:vro_client) { double('vro_client') }
+  describe "#execute_destroy_workflow" do
+    let(:state)      { { server_id: "server-12345" } }
+    let(:vro_client) { workflow_client }
 
     before do
       allow(driver).to receive(:vro_client).and_return(vro_client)
-      allow(vro_client).to receive(:parameter)
       allow(driver).to receive(:set_workflow_vars)
       allow(driver).to receive(:set_workflow_parameters)
       allow(driver).to receive(:execute_workflow)
@@ -299,10 +383,10 @@
       allow(driver).to receive(:workflow_successful?).and_return(true)
     end
 
-    it 'calls the expected methods' do
-      expect(driver).to receive(:set_workflow_vars).with('Destroy Workflow', 'workflow-2')
+    it "selects the destroy workflow and submits it with the server id" do
+      expect(driver).to receive(:set_workflow_vars).with("Destroy Workflow", "workflow-2")
       expect(driver).to receive(:set_workflow_parameters).with({})
-      expect(vro_client).to receive(:parameter).with('server_id', 'server-12345')
+      expect(vro_client).to receive(:parameter).with("server_id", "server-12345")
       expect(driver).to receive(:execute_workflow)
       expect(driver).to receive(:wait_for_workflow)
       expect(driver).to receive(:workflow_successful?)
@@ -310,244 +394,312 @@
       driver.execute_destroy_workflow(state)
     end
 
-    it 'raises an error if the workflow did not complete successfully' do
+    it "raises an error if the workflow did not complete successfully" do
       allow(driver).to receive(:workflow_successful?).and_return(false)
+
       expect { driver.execute_destroy_workflow(state) }.to raise_error(RuntimeError)
     end
   end
 
-  describe '#execute_workflow' do
-    let(:vro_client) { double('vro_client') }
+  describe "#execute_workflow" do
+    let(:vro_client) { workflow_client }
+
     before do
       allow(driver).to receive(:vro_client).and_return(vro_client)
     end
 
-    it 'executes the workflow' do
+    it "executes the workflow" do
       expect(vro_client).to receive(:execute)
+
       driver.execute_workflow
     end
 
-    context 'when execute fails with a RestClient::BadRequest' do
-      it 'prints an error with the HTTP response' do
-        HTTPResponse = Struct.new(:code, :to_s)
-        response = HTTPResponse.new(400, 'an HTTP error occurred')
-        exception = RestClient::BadRequest.new
-        exception.response = response
+    context "when execute fails with a RestClient::BadRequest" do
+      let(:exception) do
+        RestClient::BadRequest.new.tap do |e|
+          e.response = "an HTTP error occurred"
+        end
+      end
+
+      it "logs the HTTP response body, which is where vRO puts the reason" do
         allow(vro_client).to receive(:execute).and_raise(exception)
-        expect(driver).to receive(:error).with('The workflow execution request failed: an HTTP error occurred')
+
+        expect(driver).to receive(:error)
+          .with("The workflow execution request failed: an HTTP error occurred")
+
         expect { driver.execute_workflow }.to raise_error(RestClient::BadRequest)
       end
     end
 
-    context 'when execute fails with any other exception' do
-      it 'prints an error with the exception message' do
-        allow(vro_client).to receive(:execute).and_raise(RuntimeError, 'a non-HTTP error occurred')
-        expect(driver).to receive(:error).with('The workflow execution request failed: a non-HTTP error occurred')
-        expect { driver.execute_workflow }.to raise_error(RuntimeError)
+    context "when execute fails with any other exception" do
+      it "logs the exception message and re-raises" do
+        allow(vro_client).to receive(:execute)
+          .and_raise(RuntimeError, "a non-HTTP error occurred")
+
+        expect(driver).to receive(:error)
+          .with("The workflow execution request failed: a non-HTTP error occurred")
+
+        expect { driver.execute_workflow }.to raise_error(RuntimeError, "a non-HTTP error occurred")
       end
     end
   end
 
-  describe '#wait_for_workflow' do
-    let(:vro_client) { double('vro_client') }
-    let(:token)      { double('token') }
+  describe "#wait_for_workflow" do
+    let(:token)      { workflow_token(alive: [true, true, false]) }
+    let(:vro_client) { workflow_client(token: token) }
 
     before do
       allow(driver).to receive(:vro_client).and_return(vro_client)
-      allow(vro_client).to receive(:token).and_return(token)
 
-      # don't actually sleep
+      # Don't actually sleep between polls.
       allow(driver).to receive(:sleep)
     end
 
-    context 'when the requests completes normally, 3 loops' do
-      it 'only fetches the token 3 times' do
-        allow(token).to receive(:alive?).exactly(3).times.and_return(true, true, false)
-        expect(vro_client).to receive(:token).exactly(3).times
+    it "polls until the token is no longer alive" do
+      driver.wait_for_workflow
 
-        driver.wait_for_workflow
-      end
+      expect(token).to have_received(:alive?).exactly(3).times
     end
 
-    context 'when the request is completed on the first loop' do
-      it 'only refreshes the request 1 time' do
-        expect(token).to receive(:alive?).once.and_return(false)
-        driver.wait_for_workflow
-      end
+    it "re-reads the token on every poll rather than caching it" do
+      driver.wait_for_workflow
+
+      expect(vro_client).to have_received(:token).exactly(3).times
+    end
+
+    it "waits two seconds between polls" do
+      driver.wait_for_workflow
+
+      expect(driver).to have_received(:sleep).with(2).twice
     end
 
-    context 'when the timeout is exceeded' do
-      it 'raises a Timeout exception' do
+    it "does not sleep when the workflow is already finished on the first poll" do
+      allow(token).to receive(:alive?).and_return(false)
+
+      driver.wait_for_workflow
+
+      expect(driver).not_to have_received(:sleep)
+    end
+
+    context "when the timeout is exceeded" do
+      it "raises a Timeout error naming the configured timeout" do
         allow(Timeout).to receive(:timeout).and_raise(Timeout::Error)
+
         expect { driver.wait_for_workflow }.to raise_error(
-          Timeout::Error, 'Workflow did not complete in 300 seconds. ' \
-          'Please check the vRO UI for more information.'
+          Timeout::Error, "Workflow did not complete in 300 seconds. " \
+          "Please check the vRO UI for more information."
         )
       end
-    end
 
-    context 'when a non-timeout exception is raised' do
-      it 'raises the original exception' do
-        allow(vro_client).to receive(:token).and_raise(RuntimeError, 'an error occurred')
-        expect { driver.wait_for_workflow }.to raise_error(RuntimeError, 'an error occurred')
+      it "reports a non-default request_timeout" do
+        config[:request_timeout] = 30
+        allow(Timeout).to receive(:timeout).and_raise(Timeout::Error)
+
+        expect { driver.wait_for_workflow }
+          .to raise_error(Timeout::Error, /did not complete in 30 seconds/)
+      end
+
+      it "actually gives up once the configured timeout elapses" do
+        config[:request_timeout] = 0.05
+        allow(token).to receive(:alive?).and_return(true)
+        allow(driver).to receive(:sleep) { Kernel.sleep(0.01) }
+
+        expect { driver.wait_for_workflow }.to raise_error(Timeout::Error)
       end
     end
+
+    it "lets a non-timeout exception through untouched" do
+      allow(vro_client).to receive(:token).and_raise(RuntimeError, "an error occurred")
+
+      expect { driver.wait_for_workflow }.to raise_error(RuntimeError, "an error occurred")
+    end
   end
 
-  describe '#wait_for_server' do
-    let(:connection) { instance.transport.connection(state) }
-    let(:state)      { { hostname: 'host1', server_id: 'server-12345' } }
+  describe "#wait_for_server" do
+    let(:state)      { { hostname: "host1", server_id: "server-12345" } }
+    let(:connection) { transport.connection(state) }
 
     before do
       allow(transport).to receive(:connection).and_return(connection)
     end
 
-    it 'calls wait_until_ready on the transport connection' do
+    it "calls wait_until_ready on the transport connection" do
       expect(connection).to receive(:wait_until_ready)
+
       driver.wait_for_server(state)
     end
 
-    it 'destroys the server if the server failed to become ready' do
+    it "destroys the server if the server failed to become ready" do
       allow(connection).to receive(:wait_until_ready).and_raise(RuntimeError)
+
       expect(driver).to receive(:destroy).with(state)
+
       expect { driver.wait_for_server(state) }.to raise_error(RuntimeError)
     end
+
+    it "re-raises the original connection failure" do
+      allow(connection).to receive(:wait_until_ready)
+        .and_raise(Kitchen::Transport::TransportFailed, "connection refused")
+      allow(driver).to receive(:destroy)
+
+      expect { driver.wait_for_server(state) }
+        .to raise_error(Kitchen::Transport::TransportFailed, "connection refused")
+    end
   end
 
-  describe '#set_workflow_parameters' do
-    let(:vro_client) { double('vro_client') }
-    let(:params)     { { key1: 'value1', key2: 'value2' } }
+  describe "#set_workflow_parameters" do
+    let(:vro_client) { workflow_client }
 
-    it 'sets parameters on the client' do
+    before do
       allow(driver).to receive(:vro_client).and_return(vro_client)
-      expect(vro_client).to receive(:parameter).with('key1', 'value1')
-      expect(vro_client).to receive(:parameter).with('key2', 'value2')
+    end
 
-      driver.set_workflow_parameters(params)
+    it "sets parameters on the client" do
+      expect(vro_client).to receive(:parameter).with("key1", "value1")
+      expect(vro_client).to receive(:parameter).with("key2", "value2")
+
+      driver.set_workflow_parameters(key1: "value1", key2: "value2")
+    end
+
+    it "stringifies symbol keys, which is how they arrive from kitchen.yml" do
+      expect(vro_client).to receive(:parameter).with("os_version", "9")
+
+      driver.set_workflow_parameters(os_version: "9")
+    end
+
+    it "passes non-string values through untouched" do
+      expect(vro_client).to receive(:parameter).with("cpus", 4)
+      expect(vro_client).to receive(:parameter).with("ha", true)
+
+      driver.set_workflow_parameters("cpus" => 4, "ha" => true)
+    end
+
+    it "does nothing when there are no parameters" do
+      expect(vro_client).not_to receive(:parameter)
+
+      driver.set_workflow_parameters({})
     end
   end
 
-  describe '#output_parameters' do
-    let(:vro_client) { double('vro_client') }
-    let(:token)      { double('token') }
-    let(:output)     { double('output_parameters') }
+  describe "#output_parameters" do
+    let(:token)      { workflow_token(output: { "server_id" => "server-12345" }) }
+    let(:vro_client) { workflow_client(token: token) }
 
-    it 'returns the output parameters from the workflow token' do
+    before do
       allow(driver).to receive(:vro_client).and_return(vro_client)
-      allow(vro_client).to receive(:token).and_return(token)
-      expect(token).to receive(:output_parameters).and_return(output)
-      expect(driver.output_parameters).to eq(output)
     end
-  end
 
-  describe '#output_parameter_value' do
-    let(:test_key) { double('test_key', value: 'test_value') }
-    let(:output_parameters) { { 'test_key' => test_key } }
-    it 'returns the correct value' do
-      allow(driver).to receive(:output_parameters).and_return(output_parameters)
-      expect(driver.output_parameter_value('test_key')).to eq('test_value')
+    it "returns the output parameters from the workflow token" do
+      expect(driver.output_parameters.keys).to eq(["server_id"])
+    end
+
+    it "reads them from the token only once" do
+      driver.output_parameters
+      driver.output_parameters
+
+      expect(token).to have_received(:output_parameters).once
     end
   end
 
-  describe '#output_parameter_empty?' do
-    context 'when the value is not nil or empty' do
-      it 'returns false' do
-        allow(driver).to receive(:output_parameter_value).with('test_key').and_return('test_value')
-        expect(driver.output_parameter_empty?('test_key')).to eq(false)
-      end
+  describe "#output_parameter_value" do
+    it "returns the value vRO reported" do
+      allow(driver).to receive(:output_parameters)
+        .and_return(workflow_output("test_key" => "test_value"))
+
+      expect(driver.output_parameter_value("test_key")).to eq("test_value")
     end
 
-    context 'when the value is nil' do
-      it 'returns true' do
-        allow(driver).to receive(:output_parameter_value).with('test_key').and_return(nil)
-        expect(driver.output_parameter_empty?('test_key')).to eq(true)
-      end
+    it "stringifies a value that vRO returned as a number" do
+      allow(driver).to receive(:output_parameters)
+        .and_return(workflow_output("server_id" => 12_345))
+
+      expect(driver.output_parameter_value("server_id")).to eq("12345")
     end
 
-    context 'when the value is empty' do
-      it 'returns true' do
-        allow(driver).to receive(:output_parameter_value).with('test_key').and_return('')
-        expect(driver.output_parameter_empty?('test_key')).to eq(true)
-      end
+    it "returns an empty string for a parameter vRO left unset" do
+      allow(driver).to receive(:output_parameters)
+        .and_return(workflow_output("ip_address" => nil))
+
+      expect(driver.output_parameter_value("ip_address")).to eq("")
     end
   end
 
-  describe '#validate_create_output_parameters!' do
-    let(:server_id)   { double('server_id', value: 'server-12345') }
-    let(:ip_address)  { double('ip_address', value: '1.2.3.4') }
+  describe "#output_parameter_empty?" do
+    it "is false when the value is neither nil nor empty" do
+      allow(driver).to receive(:output_parameter_value).with("test_key").and_return("test_value")
+
+      expect(driver.output_parameter_empty?("test_key")).to be false
+    end
+
+    it "is true when the value is nil" do
+      allow(driver).to receive(:output_parameter_value).with("test_key").and_return(nil)
+
+      expect(driver.output_parameter_empty?("test_key")).to be true
+    end
+
+    it "is true when the value is empty" do
+      allow(driver).to receive(:output_parameter_value).with("test_key").and_return("")
+
+      expect(driver.output_parameter_empty?("test_key")).to be true
+    end
+  end
 
-    context 'when the output parameters do not include server_id and ip_address' do
+  describe "#validate_create_output_parameters!" do
+    context "when the output parameters do not include server_id and ip_address" do
       let(:output_parameters) { {} }
-      it_behaves_like 'output parameters that are missing a required key'
+
+      it_behaves_like "output parameters that are missing a required key"
     end
 
-    context 'when the output parameters do not include server_id' do
-      let(:output_parameters) { { 'ip_address' => ip_address } }
-      it_behaves_like 'output parameters that are missing a required key'
+    context "when the output parameters do not include server_id" do
+      let(:output_parameters) { workflow_output("ip_address" => "1.2.3.4") }
+
+      it_behaves_like "output parameters that are missing a required key"
     end
 
-    context 'when the output parameters do not include ip_address' do
-      let(:output_parameters) { { 'server_id' => server_id } }
-      it_behaves_like 'output parameters that are missing a required key'
+    context "when the output parameters do not include ip_address" do
+      let(:output_parameters) { workflow_output("server_id" => "server-12345") }
+
+      it_behaves_like "output parameters that are missing a required key"
     end
 
-    context 'when server_id is empty' do
-      let(:server_id) { double('server_id', value: '') }
-      let(:output_parameters) do
-        {
-          'server_id'  => server_id,
-          'ip_address' => ip_address
-        }
-      end
+    it "raises when server_id is empty" do
+      allow(driver).to receive(:output_parameters)
+        .and_return(workflow_output("server_id" => "", "ip_address" => "1.2.3.4"))
 
-      it 'raises an exception' do
-        allow(driver).to receive(:output_parameters).and_return(output_parameters)
-        expect { driver.validate_create_output_parameters! }.to raise_error(
-          RuntimeError,
-          'The server_id parameter was empty.'
-        )
-      end
+      expect { driver.validate_create_output_parameters! }
+        .to raise_error(RuntimeError, "The server_id parameter was empty.")
     end
 
-    context 'when ip_address is empty' do
-      let(:ip_address) { double('ip_address', value: '') }
-      let(:output_parameters) do
-        {
-          'server_id'  => server_id,
-          'ip_address' => ip_address
-        }
-      end
+    it "raises when ip_address is empty" do
+      allow(driver).to receive(:output_parameters)
+        .and_return(workflow_output("server_id" => "server-12345", "ip_address" => ""))
 
-      it 'raises an exception' do
-        allow(driver).to receive(:output_parameters).and_return(output_parameters)
-        expect { driver.validate_create_output_parameters! }.to raise_error(
-          RuntimeError,
-          'The ip_address parameter was empty.'
-        )
-      end
+      expect { driver.validate_create_output_parameters! }
+        .to raise_error(RuntimeError, "The ip_address parameter was empty.")
     end
-  end
 
-  describe '#workflow_successful?' do
-    let(:vro_client) { double('vro_client') }
-    let(:token)      { double('token') }
-    before do
-      allow(driver).to receive(:vro_client).and_return(vro_client)
-      allow(vro_client).to receive(:token).and_return(token)
+    it "accepts output that names both a server and an address" do
+      allow(driver).to receive(:output_parameters)
+        .and_return(workflow_output("server_id" => "server-12345", "ip_address" => "1.2.3.4"))
+
+      expect { driver.validate_create_output_parameters! }.not_to raise_error
     end
+  end
 
-    context 'when the state is completed' do
-      it 'returns true' do
-        allow(token).to receive(:state).and_return('completed')
-        expect(driver.workflow_successful?).to eq(true)
-      end
+  describe "#workflow_successful?" do
+    it "is true when vRO reports the run completed" do
+      allow(driver).to receive(:vro_client)
+        .and_return(workflow_client(token: workflow_token(state: "completed")))
+
+      expect(driver.workflow_successful?).to be true
     end
 
-    context 'when the state is failed' do
-      it 'returns true' do
-        allow(token).to receive(:state).and_return('failed')
-        expect(driver.workflow_successful?).to eq(false)
-      end
+    it "is false for any other state" do
+      allow(driver).to receive(:vro_client)
+        .and_return(workflow_client(token: workflow_token(state: "failed")))
+
+      expect(driver.workflow_successful?).to be false
     end
   end
 end

From d14eb27a25e59be57b7b83e7bc4fdfe4b37d6caf Mon Sep 17 00:00:00 2001
From: Tim Smith <tim@mondoo.com>
Date: Sat, 29 Aug 2026 18:53:40 -0700
Subject: [PATCH 2/3] chore: drop the unused webmock dependency

webmock has never been required or used by anything in spec/ -- the specs
stub the vcoworkflows client rather than the HTTP layer underneath it, so
nothing ever reaches a socket for webmock to intercept.

Removing it also clears the one thing standing between this repo and a green
lint run: cookstyle 9.0.0 flags the extra spacing in that Gemfile line under
Layout/ExtraSpacing, which cookstyle 8 did not, so `main` went red on its own
the moment cookstyle 9 shipped.

Signed-off-by: Tim Smith <tim@mondoo.com>
---
 Gemfile | 1 -
 1 file changed, 1 deletion(-)

diff --git a/Gemfile b/Gemfile
index 8a25339..793b300 100644
--- a/Gemfile
+++ b/Gemfile
@@ -5,7 +5,6 @@ gemspec
 group :test do
   gem "rake"
   gem "rspec", "~> 3.2"
-  gem "webmock",   "~> 3.0"
 end
 
 group :docs do

From c6777903eaa08dc1e9743ba5011652f255292671 Mon Sep 17 00:00:00 2001
From: Tim Smith <tsmith84@proton.me>
Date: Sat, 29 Aug 2026 19:21:27 -0700
Subject: [PATCH 3/3] fix: clear destroyed servers from state and stop masking
 create failures (#45)

A handful of related problems in the create/destroy lifecycle.

`destroy` never removed `server_id` and `hostname` from the state hash. Test
Kitchen writes state back in an `ensure` block even when an action fails, so
when `wait_for_server` gives up and tears the machine down, the state file is
still left naming a server that no longer exists. The next `kitchen destroy`
then runs the destroy workflow a second time against a machine that is
already gone. `destroy` now clears both keys once the workflow succeeds,
which also makes it a no-op the second time it is called in a process.

`wait_for_server` replaced the connection failure with whatever the cleanup
raised. If the machine was unreachable *and* the destroy workflow then failed,
the user saw the vRO error and no hint of why the machine was being destroyed
in the first place. The cleanup failure is now logged -- including a warning
that the server may have been left behind -- and the original connection
failure is what propagates.

`set_workflow_vars` cleared the memoized client but not the memoized output
parameters, so anything reading `output_parameters` after switching from the
create workflow to the destroy workflow would still be looking at the create
workflow's output.

`output_parameter_value` raised `NoMethodError` on nil for any parameter the
workflow did not return, which made `output_parameter_empty?` blow up instead
of answering the question it was asked. It now returns an empty string for a
missing parameter. That in turn removes a dead branch: `output_parameter_empty?`
tested `.nil?` on the result of a `.to_s`, which can never be nil, and
computed the value twice to do it.

Also requires `timeout` explicitly rather than relying on a dependency having
pulled it in, and updates a `rubocop:disable` comment that still used the old
`Style/AccessorMethodName` namespace -- cookstyle printed a deprecation
warning about it on every run.

Signed-off-by: Tim Smith <tim@mondoo.com>
---
 lib/kitchen/driver/vro.rb | 62 ++++++++++++++++++++++-------
 spec/vro_spec.rb          | 82 +++++++++++++++++++++++++++++++++++++--
 2 files changed, 125 insertions(+), 19 deletions(-)

diff --git a/lib/kitchen/driver/vro.rb b/lib/kitchen/driver/vro.rb
index a0f80de..32e207b 100644
--- a/lib/kitchen/driver/vro.rb
+++ b/lib/kitchen/driver/vro.rb
@@ -16,6 +16,8 @@
 # limitations under the License.
 #
 
+require "timeout" unless defined?(Timeout)
+
 require "kitchen"
 require "vcoworkflows"
 require_relative "version"
@@ -87,14 +89,27 @@ def create(state)
 
       # Runs the destroy workflow for the server named in state.
       #
-      # @param state [Hash] instance state naming the server
+      # Clears +server_id+ and +hostname+ afterwards. Test Kitchen persists
+      # the state hash even when an action fails, so leaving a destroyed
+      # server named in it means the next +kitchen destroy+ runs the destroy
+      # workflow again against a machine that is already gone.
+      #
+      # @param state [Hash] mutable instance state naming the server; loses
+      #   +server_id+ and +hostname+
       # @return [void]
       def destroy(state)
         return if state[:server_id].nil?
 
-        info("Executing the destroy-server workflow for #{state[:hostname]} (#{state[:server_id]})...")
+        server_id = state[:server_id]
+        hostname  = state[:hostname]
+
+        info("Executing the destroy-server workflow for #{hostname} (#{server_id})...")
         execute_destroy_workflow(state)
-        info("Server #{state[:hostname]} (#{state[:server_id]}) destroyed.")
+
+        state.delete(:server_id)
+        state.delete(:hostname)
+
+        info("Server #{hostname} (#{server_id}) destroyed.")
       end
 
       # Connection settings for the vRO API.
@@ -131,16 +146,20 @@ def verify_ssl?
 
       # Points the driver at a different workflow.
       #
-      # Clears the memoized client, which is what makes it safe to run the
-      # destroy workflow after the create workflow in the same process.
+      # Clears the memoized client and the memoized output parameters, which
+      # is what makes it safe to run the destroy workflow after the create
+      # workflow in the same process. Without the second reset, anything
+      # reading {#output_parameters} after the switch would still be looking
+      # at the previous workflow's output.
       #
       # @param name [String] workflow name
       # @param id [String, nil] workflow id, when the name is ambiguous
       # @return [void]
       def set_workflow_vars(name, id)
-        @vro_client    = nil
-        @workflow_name = name
-        @workflow_id   = id
+        @vro_client        = nil
+        @output_parameters = nil
+        @workflow_name     = name
+        @workflow_id       = id
       end
 
       # Runs the create workflow and records what it produced.
@@ -221,17 +240,26 @@ def wait_for_workflow
       # @return [void]
       def wait_for_server(state)
         instance.transport.connection(state).wait_until_ready
-      rescue
+      rescue => e
         error("Server #{state[:hostname]} (#{state[:server_id]}) not reachable. Destroying server...")
-        destroy(state)
-        raise
+
+        begin
+          destroy(state)
+        rescue => destroy_error
+          # Report the leaked machine, but keep the connection failure as the
+          # error that surfaces -- it is the one that explains the run.
+          error("The destroy-server workflow also failed: #{destroy_error.message}. " \
+                "The server may still exist; check the vRO UI.")
+        end
+
+        raise e
       end
 
       # Sets input parameters on the current workflow.
       #
       # @param data [Hash] parameter names to values; names are stringified
       # @return [void]
-      def set_workflow_parameters(data) # rubocop:disable Style/AccessorMethodName
+      def set_workflow_parameters(data) # rubocop:disable Naming/AccessorMethodName
         data.each do |key, value|
           vro_client.parameter(key.to_s, value)
         end
@@ -245,15 +273,19 @@ def output_parameters
       end
 
       # @param key [String] output parameter name
-      # @return [String] the parameter's value, stringified
+      # @return [String] the parameter's value, stringified; an empty string
+      #   when the workflow did not return that parameter at all
       def output_parameter_value(key)
-        output_parameters[key].value.to_s
+        parameter = output_parameters[key]
+        return "" if parameter.nil?
+
+        parameter.value.to_s
       end
 
       # @param key [String] output parameter name
       # @return [Boolean] true when the parameter is missing or empty
       def output_parameter_empty?(key)
-        output_parameter_value(key).nil? || output_parameter_value(key).empty?
+        output_parameter_value(key).empty?
       end
 
       # Checks that the create workflow returned what the driver needs.
diff --git a/spec/vro_spec.rb b/spec/vro_spec.rb
index bff6df9..4388c08 100644
--- a/spec/vro_spec.rb
+++ b/spec/vro_spec.rb
@@ -192,6 +192,17 @@
       expect(driver.workflow_id).to be_nil
     end
 
+    it "discards the memoized output parameters so they cannot leak between workflows" do
+      create_token = workflow_token(output: { "server_id" => "server-12345" })
+      allow(driver).to receive(:vro_client).and_return(workflow_client(token: create_token))
+      expect(driver.output_parameters.keys).to eq(["server_id"])
+
+      driver.set_workflow_vars("Destroy Workflow", "workflow-2")
+      allow(driver).to receive(:vro_client).and_return(workflow_client(token: workflow_token))
+
+      expect(driver.output_parameters).to be_empty
+    end
+
     it "discards the memoized client so the next call builds a new one" do
       first  = workflow_client
       second = workflow_client
@@ -284,6 +295,31 @@
       driver.destroy({})
     end
 
+    it "clears the destroyed server out of state" do
+      allow(driver).to receive(:execute_destroy_workflow)
+
+      driver.destroy(state)
+
+      expect(state).not_to have_key(:server_id)
+      expect(state).not_to have_key(:hostname)
+    end
+
+    it "leaves state alone when the destroy workflow fails" do
+      allow(driver).to receive(:execute_destroy_workflow).and_raise(RuntimeError)
+
+      expect { driver.destroy(state) }.to raise_error(RuntimeError)
+      expect(state[:server_id]).to eq("server-12345")
+    end
+
+    it "does not run the destroy workflow twice for the same server" do
+      allow(driver).to receive(:execute_destroy_workflow)
+
+      driver.destroy(state)
+      driver.destroy(state)
+
+      expect(driver).to have_received(:execute_destroy_workflow).once
+    end
+
     context "with the vRO API stubbed end to end" do
       let(:client) { workflow_client }
 
@@ -545,6 +581,29 @@
       expect { driver.wait_for_server(state) }
         .to raise_error(Kitchen::Transport::TransportFailed, "connection refused")
     end
+
+    context "when the cleanup destroy also fails" do
+      before do
+        allow(connection).to receive(:wait_until_ready)
+          .and_raise(Kitchen::Transport::TransportFailed, "connection refused")
+        allow(driver).to receive(:destroy).and_raise(RuntimeError, "vRO rejected the request")
+      end
+
+      it "still surfaces the connection failure, not the cleanup failure" do
+        allow(driver).to receive(:error)
+
+        expect { driver.wait_for_server(state) }
+          .to raise_error(Kitchen::Transport::TransportFailed, "connection refused")
+      end
+
+      it "warns that the server may have been left behind" do
+        expect(driver).to receive(:error).with(/not reachable/)
+        expect(driver).to receive(:error).with(/destroy-server workflow also failed/)
+
+        expect { driver.wait_for_server(state) }
+          .to raise_error(Kitchen::Transport::TransportFailed)
+      end
+    end
   end
 
   describe "#set_workflow_parameters" do
@@ -622,23 +681,38 @@
 
       expect(driver.output_parameter_value("ip_address")).to eq("")
     end
+
+    it "returns an empty string for a parameter the workflow never returned" do
+      allow(driver).to receive(:output_parameters).and_return({})
+
+      expect(driver.output_parameter_value("ip_address")).to eq("")
+    end
   end
 
   describe "#output_parameter_empty?" do
     it "is false when the value is neither nil nor empty" do
-      allow(driver).to receive(:output_parameter_value).with("test_key").and_return("test_value")
+      allow(driver).to receive(:output_parameters)
+        .and_return(workflow_output("test_key" => "test_value"))
 
       expect(driver.output_parameter_empty?("test_key")).to be false
     end
 
-    it "is true when the value is nil" do
-      allow(driver).to receive(:output_parameter_value).with("test_key").and_return(nil)
+    it "is true when vRO returned the parameter unset" do
+      allow(driver).to receive(:output_parameters)
+        .and_return(workflow_output("test_key" => nil))
 
       expect(driver.output_parameter_empty?("test_key")).to be true
     end
 
     it "is true when the value is empty" do
-      allow(driver).to receive(:output_parameter_value).with("test_key").and_return("")
+      allow(driver).to receive(:output_parameters)
+        .and_return(workflow_output("test_key" => ""))
+
+      expect(driver.output_parameter_empty?("test_key")).to be true
+    end
+
+    it "is true when the workflow did not return the parameter at all" do
+      allow(driver).to receive(:output_parameters).and_return({})
 
       expect(driver.output_parameter_empty?("test_key")).to be true
     end