Add BoltSpec plan-testing suite (#76)

.github/workflows/ci.yml

@@ -62,6 +62,15 @@ jobs:
       - name: managed_file bootstrap/enforce contract test
         run: ./scripts/ci-managed-file-contract.sh
 
+      - name: BoltSpec plan specs (under Bolt's Ruby)
+        # -O /dev/null skips .rspec, whose exclude-pattern hides spec/plans
+        # from the plain-Ruby spec job (bolt_spec needs Bolt's Ruby).
+        # The .gems binstub is invoked explicitly: `ruby -S rspec` only works
+        # when some rspec is already on PATH
+        run: |
+          GEM_HOME=.gems /opt/puppetlabs/bolt/bin/gem install rspec --no-document
+          GEM_HOME=.gems /opt/puppetlabs/bolt/bin/ruby .gems/bin/rspec -O /dev/null spec/plans
+
       - name: List pipeline stages (plan dry run)
         env:
           # The plan requires these to be set, but nothing calls the APIs in

.rspec

@@ -0,0 +1,6 @@
+# The plan specs under spec/plans/ require Bolt's Ruby (bolt_spec ships with
+# the openbolt package) and are run separately — see spec/plans/plan_spec_helper.rb:
+#
+#   GEM_HOME=.gems /opt/puppetlabs/bolt/bin/ruby .gems/bin/rspec -O /dev/null spec/plans
+#
+--exclude-pattern plans/**/*_spec.rb

AGENTS.md

@@ -30,7 +30,7 @@ bolt plan run puppetsync options='{"list_pipeline_stages": true}' config=... rep
 - `config=` / `repolist=` name YAML files in `data/sync/configs/` and `data/sync/repolists/`; both default to the `latest.yaml` symlink in each directory.
 - Use `--log-level info` to watch progress (the `apply()` step can look hung otherwise).
 - Required environment variables and their purposes are documented in README.md ("Environment variables").
-- Unit tests: `rspec` from the repo root (plain rspec; no bundle needed). The specs in `spec/` run the Bolt tasks as standalone scripts and sanity-check the Hiera data. CI (`.github/workflows/ci.yml`) also validates Puppet syntax and smoke-tests plan loading with `bolt plan show` / `list_pipeline_stages`. End-to-end validation is still running a plan against a test repolist (e.g. the `*_test.yaml` repolists).
+- Unit tests: `rspec` from the repo root (plain rspec; no bundle needed). The specs in `spec/` run the Bolt tasks as standalone scripts and sanity-check the Hiera data. Plan logic is unit-tested with BoltSpec under Bolt's Ruby: `GEM_HOME=.gems /opt/puppetlabs/bolt/bin/ruby .gems/bin/rspec -O /dev/null spec/plans` (excluded from the plain `rspec` run via `.rspec`; conventions in `spec/plans/plan_spec_helper.rb`). CI (`.github/workflows/ci.yml`) also validates Puppet syntax, smoke-tests plan loading, and runs e2e/contract scripts from `scripts/`. End-to-end validation is still running a plan against a test repolist (e.g. the `*_test.yaml` repolists).
 
 ## Architecture
 

spec/fixtures/modules/puppetsync_test/plans/stage_bad_result.pp

@@ -0,0 +1,13 @@
+# Test-support plan: a stage block returning something other than Bolt
+# results must fail the plan (see #52/#64)
+plan puppetsync_test::stage_bad_result() {
+  $t = Target.new('name' => 'some-repo')
+  $t.set_var('puppetsync_stage_results', {})
+
+  [$t].puppetsync::pipeline_stage(
+    'bad_stage',
+    { 'stages' => ['bad_stage'] }
+  ) |$ok_repos, $stage_name| {
+    'this is not a Bolt::Result'
+  }
+}

spec/fixtures/modules/puppetsync_test/plans/stage_holdback.pp

@@ -0,0 +1,23 @@
+# Test-support plan: a target that fails a stage must be held back from
+# later stages while other targets proceed
+plan puppetsync_test::stage_holdback() {
+  $good = Target.new('name' => 'good-repo')
+  $bad = Target.new('name' => 'bad-repo')
+  [$good, $bad].each |$t| { $t.set_var('puppetsync_stage_results', {}) }
+
+  [$good, $bad].puppetsync::pipeline_stage(
+    'first_stage',
+    { 'stages' => ['first_stage', 'second_stage'] }
+  ) |$ok_repos, $stage_name| {
+    run_command('stage-one', $ok_repos, { '_catch_errors' => true })
+  }
+
+  $survivors = [$good, $bad].puppetsync::pipeline_stage(
+    'second_stage',
+    { 'stages' => ['first_stage', 'second_stage'] }
+  ) |$ok_repos, $stage_name| {
+    run_command('stage-two', $ok_repos, { '_catch_errors' => true })
+  }
+
+  return($survivors.map |$t| { $t.name })
+}

spec/fixtures/modules/puppetsync_test/plans/stage_smoke.pp

@@ -0,0 +1,23 @@
+# Test-support plan: exercises puppetsync::pipeline_stage gating
+plan puppetsync_test::stage_smoke() {
+  $changed = Target.new('name' => 'changed-repo')
+  $unchanged = Target.new('name' => 'unchanged-repo')
+  [$changed, $unchanged].each |$t| { $t.set_var('puppetsync_stage_results', {}) }
+  $unchanged.set_var('puppetsync_unchanged', true)
+
+  $ran = [$changed, $unchanged].puppetsync::pipeline_stage(
+    'gated_stage',
+    { 'stages' => ['gated_stage'], 'skip_unchanged_targets' => true }
+  ) |$ok_repos, $stage_name| {
+    run_command('true', $ok_repos)
+  }
+
+  $skipped = [$changed, $unchanged].puppetsync::pipeline_stage(
+    'not_in_stage_list',
+    { 'stages' => ['gated_stage'] }
+  ) |$ok_repos, $stage_name| {
+    run_command('true', $ok_repos)
+  }
+
+  return({ 'ran' => $ran.map |$t| { $t.name }, 'skipped_stage_result' => $skipped })
+}

spec/plans/approve_github_prs_spec.rb

@@ -0,0 +1,96 @@
+require_relative 'plan_spec_helper'
+
+describe 'plan: puppetsync::approve_github_prs' do
+  include_context 'puppetsync plan specs'
+
+  let(:repos_config) do
+    {
+      'https://github.com/simp/repo-a' => { 'branch' => 'master' },
+      'https://github.com/simp/repo-b' => { 'branch' => 'main' },
+    }
+  end
+
+  let(:puppetsync_config) do
+    {
+      'puppetsync' => {
+        'plans' => {
+          'approve_github_prs' => {
+            'github_api_delay_seconds' => 0,
+            'stages' => ['approve_github_pr_for_each_repo'],
+          },
+        },
+      },
+      'git' => { 'feature_branch' => 'SIMP-TEST' },
+    }
+  end
+
+  def plan_params
+    {
+      'project_dir'       => PROJECT_ROOT,
+      'puppetsync_config' => puppetsync_config,
+      'repos_config'      => repos_config,
+      'pr_user'           => 'bot-user',
+      'approval_message'  => ':+1: specs',
+    }
+  end
+
+  # Task metadata marks some params sensitive, so Bolt wraps them before the
+  # stub sees them
+  def unwrap(value)
+    value.respond_to?(:unwrap) ? value.unwrap : value
+  end
+
+  def task_result(target, task, value)
+    Bolt::Result.new(target, value: value, action: 'task', object: task)
+  end
+
+  it 'approves the PR for every repo with per-repo params from the repolist' do
+    allow_out_message
+    calls = []
+    expect_task('puppetsync::approve_github_pr').be_called_times(2).return do |targets:, task:, params:|
+      calls << { 'target' => targets.first.name }.merge(params.transform_values { |v| unwrap(v) })
+      Bolt::ResultSet.new(targets.map { |t| task_result(t, task, 'approved' => true) })
+    end
+
+    result = run_plan('puppetsync::approve_github_prs', plan_params)
+
+    expect(result.ok?).to be(true), result.value.to_s
+    expect(calls.map { |c| c['target'] }).to contain_exactly('repo-a', 'repo-b')
+    repo_a = calls.find { |c| c['target'] == 'repo-a' }
+    expect(repo_a).to include(
+      'target_repo'      => 'simp/repo-a',
+      'target_branch'    => 'master',
+      'fork_user'        => 'bot-user',
+      'fork_branch'      => 'SIMP-TEST',
+      'approval_message' => ':+1: specs',
+      'github_authtoken' => ENV.fetch('GITHUB_API_TOKEN'),
+    )
+    repo_b = calls.find { |c| c['target'] == 'repo-b' }
+    expect(repo_b).to include('target_repo' => 'simp/repo-b', 'target_branch' => 'main')
+  end
+
+  it 'summarizes and fails the plan when approval fails for a repo' do
+    allow_out_message
+    allow_task('puppetsync::approve_github_pr').with_targets(['repo-a']).return do |targets:, task:, params:|
+      Bolt::ResultSet.new(targets.map { |t| task_result(t, task, 'approved' => true) })
+    end
+    allow_task('puppetsync::approve_github_pr').with_targets(['repo-b'])
+                                               .error_with('kind' => 'spec/approval-denied', 'msg' => 'PR not found')
+
+    result = run_plan('puppetsync::approve_github_prs', plan_params)
+
+    expect(result.ok?).to be(false)
+    expect(result.value.msg).to match(/failures occured/)
+  end
+
+  it 'skips the approval stage entirely when the config stage list omits it' do
+    allow_out_message
+    config = puppetsync_config.dup
+    config['puppetsync'] = { 'plans' => { 'approve_github_prs' => { 'stages' => [] } } }
+
+    result = run_plan('puppetsync::approve_github_prs', plan_params.merge('puppetsync_config' => config))
+
+    # No task stubs declared: any approve/install task call would raise
+    expect(result.ok?).to be(true), result.value.to_s
+  end
+end

spec/plans/batch_spec.rb

@@ -0,0 +1,22 @@
+require_relative 'plan_spec_helper'
+
+describe 'plan: puppetsync::batch' do
+  include_context 'puppetsync plan specs'
+
+  it 'runs the sync plan once per repolist in the batch, in order' do
+    allow_out_message
+    repolists_run = []
+    expect_plan('puppetsync').be_called_times(2).return do |plan:, params:|
+      repolists_run << params['repolist']
+      Bolt::PlanResult.new("ran #{params['repolist']}", 'success')
+    end
+
+    result = run_plan('puppetsync::batch', {
+      'project_dir'    => PROJECT_ROOT,
+      'batches_config' => { 'repolists' => ['batch-one', 'batch-two'], 'delay' => 0 },
+    })
+
+    expect(result.ok?).to be(true), result.value.to_s
+    expect(repolists_run).to eq(['batch-one', 'batch-two'])
+  end
+end

spec/plans/merge_github_prs_spec.rb

@@ -0,0 +1,78 @@
+require_relative 'plan_spec_helper'
+
+describe 'plan: puppetsync::merge_github_prs' do
+  include_context 'puppetsync plan specs'
+
+  let(:repos_config) do
+    {
+      'https://github.com/simp/repo-a' => { 'branch' => 'master' },
+      'https://github.com/simp/repo-b' => { 'branch' => 'main' },
+    }
+  end
+
+  let(:puppetsync_config) do
+    {
+      'puppetsync' => {
+        'plans' => {
+          'merge_github_prs' => {
+            'github_api_delay_seconds' => 0,
+            'stages' => ['merge_github_pr_for_each_repo'],
+          },
+        },
+      },
+      'git' => { 'feature_branch' => 'SIMP-TEST' },
+    }
+  end
+
+  def plan_params
+    {
+      'project_dir'       => PROJECT_ROOT,
+      'puppetsync_config' => puppetsync_config,
+      'repos_config'      => repos_config,
+      'pr_user'           => 'bot-user',
+    }
+  end
+
+  def unwrap(value)
+    value.respond_to?(:unwrap) ? value.unwrap : value
+  end
+
+  def task_result(target, task, value)
+    Bolt::Result.new(target, value: value, action: 'task', object: task)
+  end
+
+  it 'merges the PR for every repo with per-repo params from the repolist' do
+    allow_out_message
+    calls = []
+    expect_task('puppetsync::merge_github_pr').be_called_times(2).return do |targets:, task:, params:|
+      calls << { 'target' => targets.first.name }.merge(params.transform_values { |v| unwrap(v) })
+      Bolt::ResultSet.new(targets.map { |t| task_result(t, task, 'merged' => true) })
+    end
+
+    result = run_plan('puppetsync::merge_github_prs', plan_params)
+
+    expect(result.ok?).to be(true), result.value.to_s
+    expect(calls.map { |c| c['target'] }).to contain_exactly('repo-a', 'repo-b')
+    expect(calls.find { |c| c['target'] == 'repo-a' }).to include(
+      'target_repo'   => 'simp/repo-a',
+      'target_branch' => 'master',
+      'fork_user'     => 'bot-user',
+      'fork_branch'   => 'SIMP-TEST',
+    )
+    expect(calls.find { |c| c['target'] == 'repo-b' }).to include('target_branch' => 'main')
+  end
+
+  it 'fails the plan at the summary when a merge fails' do
+    allow_out_message
+    allow_task('puppetsync::merge_github_pr').with_targets(['repo-a']).return do |targets:, task:, params:|
+      Bolt::ResultSet.new(targets.map { |t| task_result(t, task, 'merged' => true) })
+    end
+    allow_task('puppetsync::merge_github_pr').with_targets(['repo-b'])
+                                             .error_with('kind' => 'spec/merge-conflict', 'msg' => 'cannot merge')
+
+    result = run_plan('puppetsync::merge_github_prs', plan_params)
+
+    expect(result.ok?).to be(false)
+    expect(result.value.msg).to match(/failures occured/)
+  end
+end

spec/plans/pipeline_stage_spec.rb

@@ -0,0 +1,49 @@
+require_relative 'plan_spec_helper'
+
+describe 'puppetsync::pipeline_stage' do
+  include_context 'puppetsync plan specs'
+
+  context 'stage gating and unchanged-target skipping (stage_smoke)' do
+    it 'runs gated stages only on changed targets and honors the stage list' do
+      expect_command('true').with_targets(['changed-repo'])
+      allow_out_message
+      expect_out_message.with_params('===== SKIPPING 1 UNCHANGED TARGET(S) FOR STAGE: gated_stage')
+
+      result = run_plan('puppetsync_test::stage_smoke', {})
+
+      expect(result.ok?).to be(true), result.value.to_s
+      expect(result.value['ran']).to eq(['changed-repo'])
+      expect(result.value['skipped_stage_result']).to eq([])
+    end
+  end
+
+  context 'failure holdback (stage_holdback)' do
+    it 'holds a failed target back from later stages while others proceed' do
+      allow_out_message
+      allow_command('stage-one').return do |targets:, command:, params:|
+        Bolt::ResultSet.new(targets.map do |target|
+          exit_code = target.name == 'bad-repo' ? 1 : 0
+          value = { 'stdout' => '', 'stderr' => '', 'merged_output' => '', 'exit_code' => exit_code }
+          Bolt::Result.for_command(target, value, 'command', command, [])
+        end)
+      end
+      expect_command('stage-two').with_targets(['good-repo'])
+
+      result = run_plan('puppetsync_test::stage_holdback', {})
+
+      expect(result.ok?).to be(true), result.value.to_s
+      expect(result.value).to eq(['good-repo'])
+    end
+  end
+
+  context 'unrecordable results (stage_bad_result)' do
+    it 'fails the plan when a stage block returns a non-Bolt-result' do
+      allow_out_message
+
+      result = run_plan('puppetsync_test::stage_bad_result', {})
+
+      expect(result.ok?).to be(false)
+      expect(result.value.msg).to match(/the stage block returned String/)
+    end
+  end
+end

spec/plans/plan_spec_helper.rb

@@ -0,0 +1,47 @@
+# Helper for BoltSpec plan specs (simp/puppetsync#76).
+#
+# These specs run the project's plans with stubbed tasks/commands — no
+# clones, no network. They require Bolt's Ruby (bolt_spec ships with the
+# openbolt package) and are excluded from the default plain-Ruby `rspec`
+# run via `.rspec`. Run them with:
+#
+#   GEM_HOME=.gems /opt/puppetlabs/bolt/bin/gem install rspec --no-document
+#   GEM_HOME=.gems /opt/puppetlabs/bolt/bin/ruby .gems/bin/rspec -O /dev/null spec/plans
+#
+# Why not `bundle exec`: Bundler's load-path isolation hides the packaged
+# openbolt gem, and declaring `gem 'openbolt'` makes bundler install a
+# SECOND bolt from rubygems (vendored, full native-extension dependency
+# closure) and test THAT copy instead of the OS package that runs real
+# syncs — reintroducing the dual-install the README warns against, with
+# silent version drift between Gemfile.lock and the package. The explicit
+# binstub invocation above is deterministic without any of that.
+#
+# Conventions learned the hard way:
+#   - Stub matching is last-defined-wins: declare catch-all
+#     `allow_out_message` BEFORE specific `expect_out_message` stubs
+#   - Plans that default parameters from `lookup()` must be given those
+#     parameters explicitly, or the spec depends on the project's Hiera data
+require 'bolt_spec/plans'
+
+PROJECT_ROOT = File.expand_path(File.join(__dir__, '..', '..')) unless defined?(PROJECT_ROOT)
+
+RSpec.shared_context 'puppetsync plan specs' do
+  include BoltSpec::Plans
+
+  def modulepath
+    [
+      File.join(PROJECT_ROOT, 'spec', 'fixtures', 'modules'),
+      File.join(PROJECT_ROOT, 'dist'),
+      File.join(PROJECT_ROOT, 'modules'),
+      File.join(PROJECT_ROOT, '.modules'),
+    ]
+  end
+
+  before(:each) do
+    BoltSpec::Plans.init
+    # Sensitive[String[1]] / String[1] params default from these
+    ENV['GITHUB_API_TOKEN'] ||= 'spec-dummy-token'
+    ENV['JIRA_USER'] ||= 'spec-dummy-user'
+    ENV['JIRA_API_TOKEN'] ||= 'spec-dummy-token'
+  end
+end