test: modernize the RSpec suite and cover the workflow logic

.rspec

@@ -1,2 +1,3 @@
+--require spec_helper
 --format documentation
 --color

.rubocop.yml

@@ -6,4 +6,3 @@ AllCops:
   TargetRubyVersion: 3.1
   Exclude:
     - "vendor/**/*"
-    - "spec/**/*"

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:

Gemfile

@@ -5,7 +5,6 @@ gemspec
 group :test do
   gem "rake"
   gem "rspec", "~> 3.2"
-  gem "webmock",   "~> 3.0"
 end
 
 group :docs do

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.

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

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