Modernize testing toolchain (voxpupuli-test 9.2.1, beaker ~> 6.9) - #455
Closed
justmedude wants to merge 3 commits into
Closed
justmedude wants to merge 3 commits into
justmedude wants to merge 3 commits into
Conversation
Bump voxpupuli-test to 14.0.0 and beaker to ~> 7.0 to pick up current facterdb fact data and beaker's Beaker::Platform-based package resolution. This surfaced several fixes needed to keep the suite green: - manifests/init.pp: change forward_x11_timeout, server_alive_count_max and server_alive_interval from Variant[Undef, ...] to Optional[Variant[...]] so puppet-lint's params_not_optional_with_undef check recognizes them as optional; regenerate REFERENCE.md to match. - Rakefile: drop the redundant require of the old puppet-syntax gem's rake tasks, now that puppetlabs_spec_helper loads the renamed puppetlabs-syntax gem itself; having both loaded caused duplicate syntax:manifests task registration and Puppet entity-redefinition errors. - .rubocop.yml: remove obsolete RSpec/Capybara/* and RSpec/FactoryBot/* entries (those cops moved to separate gems not in this project's bundle, and referencing them is now a hard config error); disable RSpec/NoExpectationExample for the beaker acceptance spec, which uses apply_manifest(..., catch_failures: true) rather than expect(...).to as its assertion mechanism. - spec/classes/init_spec.rb, spec/defines/config_entry_spec.rb: the bumped facterdb has no fact data for RedHat 7, so switch the OS-independent test fixtures to RedHat 9; add a missing coverage assertion for the 50-redhat include-dir file. - spec/fixtures/testing/Archlinux-_ssh(d)_config: rename to Archlinux-6_ssh(d)_config to match the release-qualified fixture naming the newer facterdb/rspec-puppet-facts now expects. Verified locally: rake parallel_spec (6270 examples, 0 failures, 100% coverage), rake syntax/lint/validate/rubocop/metadata_lint all clean, and a full rake beaker run against ubuntu-2604 and ubuntu-2204 to confirm no regressions from the beaker version jump.
justmedude
marked this pull request as ready for review
September 7, 2026 02:01
ghoneycutt
reviewed
Sep 11, 2026
|
|
||
| it { is_expected.to contain_package('openssh-clients') } | ||
| it { is_expected.to contain_file('ssh_config_include_dir').with_require(['Package[openssh-clients]']) } | ||
| # only needed for 100% resource coverage: redhat-9's module-default config_files entry |
Owner
There was a problem hiding this comment.
There is already a specific context for this under the following. Adding it here does not make sense because the context is about the manage_packages parameter. If you are just trying to get 100% coverage and not without this, it would be better to just reduce coverage in spec/spec_helper.rb than to make the tests harder to understand.
context "on #{os} with config_files set to a valid hash" do
Contributor
Author
There was a problem hiding this comment.
Good catch, thanks. Rather than shoehorning a coverage-only assertion into the manage_packages context, I moved the "OS independent" test fixture off RedHat 9 to RedHat 8 (matching the convention already used in server_spec.rb), which avoids the config_files-triggered coverage gap entirely — no extra assertion needed. Pushed in 6f2e6af.
ghoneycutt
reviewed
Sep 11, 2026
|
|
||
| ##### <a name="-ssh--forward_x11_timeout"></a>`forward_x11_timeout` | ||
|
|
||
| Data type: `Variant[Undef, String[1], Integer[0]]` |
Owner
There was a problem hiding this comment.
Please revert changes to this file as it is auto generated.
beaker ~> 7.0 requires Ruby >= 3.2, which broke every CI job still running on Ruby 2.7.8 (the legacy puppet7 unit leg and all acceptance jobs) with a hard Bundler dependency conflict. Cap beaker at ~> 6.9 instead: it already contains the Beaker::Platform-based package resolution (including an explicit Ubuntu 26.04/"resolute" codename mapping) that motivated the bump, without dropping Ruby 2.7 support. voxpupuli-test 14.0.0 has the same problem one level removed: its `puppet-syntax >= 6.0` requirement can only be satisfied by a puppetlabs_spec_helper >= 9.0.0, which itself requires Ruby >= 3.2. There's no pin that reconciles that under Ruby 2.7.8, so cap voxpupuli-test at 9.2.1, the newest version before it started pulling in puppet-syntax directly. That still resolves facterdb to 3.10.0 (up from the original 6.0.0 pin's facterdb < 2), which is enough to fix the original RedHat 7 fact-data gap. With this narrower toolchain, most of the earlier fallout fixes turned out to be unnecessary and are reverted: - manifests/init.pp's Optional[Variant[...]] change: puppet-lint 4.3.0 (resolved here) doesn't have the newer params_not_optional_with_undef check that flagged the original Variant[Undef, ...] form. - .rubocop.yml's obsolete-cop cleanup: rubocop-rspec 2.20.0 (resolved here) predates the Capybara/FactoryBot cop extraction, so the original config is still valid (just prints non-fatal namespace warnings). - REFERENCE.md: reverted to the upstream version per review feedback -- it's only meant to be regenerated by the maintainer's :release rake task, not hand-committed in PRs. What's still needed even under the older toolchain: - Rakefile: still drop the redundant require of puppet-syntax's rake tasks -- puppetlabs_spec_helper has auto-required them itself since at least 8.0.0, so the explicit require was always redundant and caused duplicate task registration. - spec/classes/init_spec.rb, spec/defines/config_entry_spec.rb: the "OS independent, only test one" fixtures still need to move off RedHat 7 (no fact data even at facterdb 3.10.0) to RedHat 8, matching the untouched convention in spec/classes/server_spec.rb. RedHat 8 has no config_files default, avoiding the coverage-gap-inducing interaction with the manage_packages tests that RedHat 9 introduced in the previous version of this PR. - spec/fixtures/testing/Archlinux-_ssh(d)_config: still renamed to Archlinux-6_ssh(d)_config -- rspec-puppet-facts resolves Archlinux's release to "6" independent of the facterdb version pinned here. Re-verified locally under both Ruby 3.2.9 and Ruby 2.7.8 (built via rbenv specifically to reproduce the CI legs that were failing): rake parallel_spec (6269 examples, 0 failures, 100% coverage under both), clean syntax/lint/rubocop/metadata_lint under both, and a real rake beaker run for ubuntu-2204 under Ruby 2.7.8 (4 examples, 0 failures) confirming the acceptance path works end-to-end on the Ruby version that was previously failing outright.
justmedude
changed the title
The bumped rubocop (1.50.2, pulled in via voxpupuli-test) depends on a real json gem (~> 2.3), which causes Bundler to install json 2.21.2 instead of relying on Ruby 2.7.8's bundled default (2.3.0). That newer json gem breaks Puppet 7.34's deprecated PSON compatibility shim during `rake syntax` with `NameError: constant PSON::Parser not defined` -- confirmed this doesn't happen with the original (pre-bump) gem pins, and that pinning json back down to ~> 2.3.0 fixes it without changing anything for Puppet 8 / Ruby 3.2, where json still resolves freely. Verified locally under both Ruby 3.2.9 and Ruby 2.7.8: rake parallel_spec (6269 examples, 0 failures, 100% coverage on both), and clean syntax/lint/rubocop/metadata_lint on both.
Owner
|
This was really helpful and the changes have been incorporated in #456 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Modernizes the pinned
voxpupuli-test(6.0.0 -> 9.2.1) andbeaker(~> 4.29 -> ~> 6.9) gems, picking up currentfacterdbfact data and beaker'sBeaker::Platform-based package resolution, while staying compatible with the Ruby 2.7.8 CI legs this module still tests against.Opened as a draft to get early feedback / CI signal before final review, and kept separate from other in-flight PRs since this is a general tooling/CI concern rather than an OS-support addition.
Changes
Gemfile:voxpupuli-testcapped at9.2.1andbeakerat~> 6.9, rather than jumping to their absolute latest. Both gems' newest versions require Ruby >= 3.2 transitively (viapuppetlabs_spec_helper >= 9.0.0andbeaker >= 7.0respectively), which breaks every CI job still running on Ruby 2.7.8 (the legacypuppet7unit leg and all acceptance jobs). These caps are the newest versions that stay Ruby-2.7-compatible:beaker 6.9.0already includes theBeaker::Platform-based package resolution (with an explicit Ubuntu 26.04/"resolute" mapping) that motivated the bump, andvoxpupuli-test 9.2.1still resolvesfacterdbto3.10.0, fixing the original RedHat 7 fact-data gap.Gemfile: also pinjsonto~> 2.3.0on Ruby 2.7 specifically. The bumpedrubocop(1.50.2, pulled in transitively) depends on a realjsongem, so Bundler installsjson 2.21.2instead of relying on Ruby 2.7.8's bundled default (2.3.0) -- and that newerjsonbreaks Puppet 7.34's deprecated PSON compatibility shim (NameError: constant PSON::Parser not definedduringrake syntax). Confirmed this is new (doesn't happen with the original pins) and that pinningjsonback down fixes it, with no effect on Puppet 8 / Ruby 3.2 wherejsonstill resolves freely.Rakefile: drop the redundantrequire 'puppet-syntax/tasks/puppet-syntax'—puppetlabs_spec_helperhas auto-required it itself since at least 8.0.0, so the explicit require was always redundant and caused duplicatesyntax:manifeststask registration.spec/classes/init_spec.rb,spec/defines/config_entry_spec.rb: the "OS independent, only test one" fixtures move off RedHat 7 (no fact data even atfacterdb 3.10.0) to RedHat 8, matching the existing convention already used inspec/classes/server_spec.rb.spec/fixtures/testing/Archlinux-_ssh(d)_config: renamed toArchlinux-6_ssh(d)_config—rspec-puppet-factsresolves Archlinux's release to"6"independent of the facterdb version pinned here.Gemfile.lockis gitignored in this repo, so it's not part of this diff; only theGemfileversion constraints are changed.REFERENCE.mdis untouched, since it's only meant to be regenerated by the:releaserake task.Test plan
Verified locally under both Ruby 3.2.9 and Ruby 2.7.8 (built specifically to reproduce the CI legs that were failing):
bundle exec rake parallel_spec— 6269 examples, 0 failures, 100% resource coverage (both Ruby versions)bundle exec rake syntax lint validate rubocop metadata_lint— all clean (both Ruby versions)bundle exec rake beakeragainstubuntu-2204under Ruby 2.7.8 — 4 examples, 0 failures, confirming the acceptance path works end-to-end on the Ruby version that was previously failing outrightKnown unrelated CI flake
The
debian-11acceptance jobs may fail with a 404 fetchinglibc-l10n_*.debfromdeb.debian.org's security archive. That's an external Debian 11 (oldstable) mirror availability issue, unrelated to this PR -- same category as Ubuntu 26.04's unpublishedpuppet8-release-resolute.debblocker on the other open PR.