bolt icon indicating copy to clipboard operation
bolt copied to clipboard

BoltSpec's `expect_plan()` doesn't match `_catch_error` parameter

Open jay7x opened this issue 1 year ago • 3 comments

Describe the Bug

When trying to mock a run_plan() call with BoltSpec's expect_plan() it seems that _catch_error parameter is ignored completely.

Expected Behavior

I'd expect expect_plan() to expect the _catch_error parameter to match the expectation 😄

Steps to Reproduce

Imagine PDK-managed example module with the following:

example/plans/test.pp:

plan example::test {
  return run_plan('example::test_test', nodes => 'node.example.com', _catch_errors => true)
}

example/spec/plans/test_spec.rb:

# frozen_string_literal: true

require 'spec_helper'

describe 'example::test' do
  let(:plan) { 'example::test' }

  it 'runs successfully' do
    execute_no_plan
    expect_plan('example::test_test').with_params('nodes' => 'node.example.com', '_catch_errors' => true)

    result = run_plan(plan, {})
    expect(result.ok?).to be(true)
  end
end

Now let's run the test:

pdk test unit --tests spec/plans/test_spec.pp

You'll see the following

  1) example::test runs successfully
     Failure/Error: result = run_plan(plan, {})
     
     RuntimeError:
       Expected example::test_test to be called 1 times with parameters {"nodes"=> "node.example.com", "_catch_errors"=>true}
       Plan result: null
[...]

With the expect_plan above commented out following error is raised:

     RuntimeError:
       Unexpected call to 'run_plan(example::test_test, {"nodes"=>"node.example.com"})'

Here it's clear that _catch_error parameter is not even considered while matching (as I read it).

Environment

  • PDK 3.0.1, tried with both Puppet 7 & 8
  • Bolt 3.28.0
  • Platform
    os:
      architecture: x86_64
      family: "Darwin"
      hardware: x86_64
      macosx:
        build: 23E224
        product: macOS
        version:
          full: 14.4.1
          major: '14'
          minor: '4'
          patch: '1'
      name: "Darwin"
      release:
        full: 23.4.0
        major: '23'
        minor: '4'
    

jay7x avatar Apr 11 '24 12:04 jay7x

Are you trying to test whether or not _catch_errors is conditionally in the call to run_plan? Ultimately the special _ prefixed parameters in the run_plan function are explicitly filtered out as options. https://github.com/puppetlabs/bolt/blob/82662933538513a72bd46652c5de52a958d22dbc/bolt-modules/boltlib/lib/puppet/functions/run_plan.rb#L67-L68

So if i have a plan that expects a single parameters targets that should expect only to be called with targets when invoked as a sub-plan, whether or not the parent plan uses the _catch_error option.

So for you example, the expect should be:

expect_plan('example::test_test').with_params('nodes' => 'node.example.com')

The expectation in this case is specifically what parameters are provided to the sub plan.

donoghuc avatar Apr 11 '24 15:04 donoghuc

The plan under the test must have _catch_errors set to true. I was trying to ensure this in the BoltSpec test. If anyone will change it, then the unit test should fail.

jay7x avatar Apr 11 '24 16:04 jay7x

While the explained behavior is discuss-able, I'd prefer to see this stated explicitly in the documentation (if not done yet).. It'd save me few hours of try-and-fail game...

jay7x avatar Apr 11 '24 16:04 jay7x