Jira (PUP-11435) Puppet 6.26 and 7.13 may cause specs to fail

[Nick Burgan (Jira)]

Nick Burgan updated an issue

Puppet / PUP-11435 Puppet 6.26 and 7.13 may cause specs to fail Change By: Nick Burgan Summary: Puppet 6.26 and 7.13 may cause specs to fail Add Comment

This message was sent by Atlassian Jira (v8.20.2#820002-sha1:829506d)

[Josh Cooper (Jira)]

Josh Cooper updated an issue

Puppet / PUP-11435 Puppet 6.26 and 7.13 may cause specs to fail

Change By: Josh Cooper Team: Phoenix

Add Comment

[Josh Cooper (Jira)]

Josh Cooper commented on PUP-11435

Re: Puppet 6.26 and 7.13 may cause specs to fail Interestingly, pdk has a command to generate a new custom fact and related unit test. The pdk automatically adds a Facter.clear in a "before each": # frozen_string_literal: true   require 'spec_helper' require 'facter' require 'facter/foo'   describe :foo, type: :fact do   subject(:fact) \{ Facter.fact(:foo) }     before :each do     # perform any action that should be run before every test     Facter.clear   end end where "clear" does even more than "reset". That said, I agree puppet should call "Facter.reset" (or perhaps "Facter.clear") in its spec_helper so it executes before each test, instead of in the settings code.

Add Comment

[Nirupama Mantha (Jira)]

Nirupama Mantha updated an issue

Puppet / PUP-11435

Puppet 6.26 and 7.13 may cause specs to fail

Change By: Nirupama Mantha Sprint: Phoenix 2022-02-16

Add Comment

[Josh Cooper (Jira)]

Josh Cooper updated an issue

Puppet / PUP-11435 Puppet 6.26 and 7.13 may cause specs to fail

Change By: Josh Cooper Story Points: 1

Add Comment

[Josh Cooper (Jira)]

Josh Cooper updated an issue

Puppet / PUP-11435 Puppet 6.26 and 7.13 may cause specs to fail

Change By: Josh Cooper Fix Version/s: PUP 7.15.0 Fix Version/s: PUP 6.27.0

Add Comment

[Josh Cooper (Jira)]

Josh Cooper updated an issue

Puppet / PUP-11435 Puppet 6.26 and 7.13 may cause specs to fail Change By: Josh Cooper

Commit 78246ca8d08dee770886f0940a6472c45483d627 first released in 6.26.0 and 7.13.0 speeds up rspec tests but breaks puppet-enteprise-modules due to the way {{Facter.reset}} breaks stubbing for a custom fact. [ https://github.com/puppetlabs/puppet/commit/78246ca8d08dee770886f0940a6472c45483d627 ] This ticket is to: 1. Delete the call to {{Facter.reset}} line in [https://github.com/puppetlabs/puppet/blob/4772afa194402a3876069785a178611797a8eb7d/lib/puppet/defaults.rb#L1998] 2. Optional (attempt 1), try adding {{Facter.reset}} to puppet's [spec_helper.rb|https://github.com/puppetlabs/puppet/blob/4772afa194402a3876069785a178611797a8eb7d/sec/spec_helper.rb]. That way the facter search path is reset across tests, but it's only triggered when running puppet's spec tests. But not when a module runs tests and calls puppet as a library. 3. Optional (attempt 2), if attempt 1 doesn't work, try adding {{Facter.reset}} to puppet's [test helper|https://github.com/puppetlabs/puppet/blob/4772afa194402a3876069785a178611797a8eb7d/lib/puppet/test/test_helper.rb#L145-L146] This likely has to be done after the call to {{Puppet.runtime[:facter]}}. Note this code will be invoked by modules that are using puppetlabs_spec_helper. 4. If neither work, just omit the call to Facter.reset entirely. 5. A more complete fix is to understand why this issue affects PE modules, but not a custom module generated via pdk (see comment below).

Add Comment

[Josh Cooper (Jira)]

To reproduce the issue: {code:shell}$ git clone g...@github.com:puppetlabs/puppet_enterprise_modules $ cd puppet_enterprise_modules $ git revert f8ec0e5d9234be99b3566034e921d797fb85ea43 $ PUPPET_VERSION="6.26.0" bundle install --without system_tests  $ cd modules/puppet_enterprise $ PUPPET_VERSION="6.26.0" bundle exec rake spec SPEC=spec/unit/facter/inventory_spec.rb ... Failures:  1) inventory facts the inventory and metadata facts when setting the inventory fact wil be populated when enabled      Failure/Error: expect(Facter.value('_puppet_inventory_1')).not_to be nil              expected not #<NilClass:8> => nil                 got #<NilClass:8> => nil              Compared using equal?, which compares object identity.      # ./spec/unit/facter/inventory_spec.rb:77:in `block (4 levels) in <top (required)>'  2) inventory facts the inventory and metadata facts when setting the metadata fact sets enabled to true if enabled      Failure/Error: expect(Facter.value('puppet_inventory_metadata')['packages']['collection_enabled']).to be true              expected true             got false      # ./spec/unit/facter/inventory_spec.rb:89:in `block (4 levels) in <top (required)>'Finished in 0.16165 seconds (files took 0.97491 seconds to load) 9 examples, 2 failures Failed examples: rspec ./spec/unit/facter/inventory_spec.rb:75 # inventory facts the inventory and metadata facts when setting the inventory fact wil be populated when enabled rspec ./spec/unit/facter/inventory_spec.rb:87 # inventory facts the inventory and metadata facts when setting the metadata fact sets enabled to true if enabled{code} But if then downgrade to 6.25.1 it works: {code} $ PUPPET_VERSION="6.25.1" bundle update ... Using puppet 6.25.1 (was 6.26.0) ... $ PUPPET_VERSION="6.25.1" bundle exec rake spec SPEC=spec/unit/facter/inventory_spec.rb ... Finished in 0.08799 seconds (files took 0.97687 seconds to load) 9 examples, 0 failures {code}

Add Comment

[Josh Cooper (Jira)]

But if then if you downgrade to 6.25.1 it works:

{code} $ PUPPET_VERSION="6.25.1" bundle update ... Using puppet 6.25.1 (was 6.26.0) ... $ PUPPET_VERSION="6.25.1" bundle exec rake spec SPEC=spec/unit/facter/inventory_spec.rb ... Finished in 0.08799 seconds (files took 0.97687 seconds to load) 9 examples, 0 failures {code}

If you edit {{defaults.rb}}

Add Comment

[Josh Cooper (Jira)]

Josh Cooper updated an issue

Puppet / PUP-11435 Puppet 6.26 and 7.13 may cause specs to fail Change By: Josh Cooper

Commit 78246ca8d08dee770886f0940a6472c45483d627 first released in 6.26.0 and 7.13.0 speeds up rspec tests but breaks puppet-enteprise-modules due to the way {{Facter.reset}} breaks stubbing for a custom fact. [https://github.com/puppetlabs/puppet/commit/78246ca8d08dee770886f0940a6472c45483d627] This ticket is to: 1. Delete the call to {{Facter.reset}} line in [https://github.com/puppetlabs/puppet/blob/4772afa194402a3876069785a178611797a8eb7d/lib/puppet/defaults.rb#L1998] 2. Optional (attempt 1), try adding {{Facter.reset}} to puppet's [spec_helper.rb|https://github.com/puppetlabs/puppet/blob/4772afa194402a3876069785a178611797a8eb7d/sec/spec_helper.rb]. That way the facter search path is reset across tests, but it's only triggered when running puppet's spec tests. But not when a module runs tests and calls puppet as a library. 3. Optional (attempt 2), if attempt 1 doesn't work, try adding {{Facter.reset}} to puppet's [test helper|https://github.com/puppetlabs/puppet/blob/4772afa194402a3876069785a178611797a8eb7d/lib/puppet/test/test_helper.rb#L145-L146] This likely has to be done after the call to {{{}Puppet.runtime[:facter]{}}}. Note this code will be invoked by modules that are using puppetlabs_spec_helper. 4. If neither work, just omit the call to Facter.reset entirely. 5. A more complete fix is to understand why this issue affects PE modules, but not a custom module generated via pdk (see comment below). To reproduce the issue: {code}

If you edit go back to 6.26.0 and comment out the {{ Facter.reset}} line in defaults.rb it works: {code } } $ PUPPET_VERSION="6.26.0" bundle update $ vi $(PUPPET_VERSION="6.26.0" bundle exec gem which puppet/defaults.rb)

Add Comment

[Josh Cooper (Jira)]

If you go back to 6.26.0 and comment out the {{Facter.reset}} line in defaults.rb it works:

{code} $ PUPPET_VERSION="6.26.0" bundle update $ vi $(PUPPET_VERSION="6.26.0" bundle exec gem which puppet/defaults.rb)

# comment out the call to "facter.reset" and it should now pass

$ PUPPET_VERSION="6.26.0" bundle exec rake spec SPEC=spec/unit/facter/inventory_spec.rb

{code}

Add Comment

[Josh Cooper (Jira)]

Josh Cooper commented on PUP-11435

Re: Puppet 6.26 and 7.13 may cause specs to fail In the case where things work, only one instance of Packages is created (early on when inventory.rb is loaded) and is shared across tests, which means there is the potential for state to leak across tests. In the case where things don't work, the call to Facter.reset unregisters the fact. So the next call to Facter.value('_puppet_inventory_1') will force the fact to be reloaded via Kernel.load. My theory is that rspec redefines a method on the class, but then the call to Kernel.load overwrites the rspec method, so rspec's hook is never called. I think we should just remove the call to Facter.reset like nick.burgan-illig suggested earlier.

Add Comment

[Josh Cooper (Jira)]

Josh Cooper updated an issue

Puppet / PUP-11435

Puppet 6.26 and 7.13 may cause specs to fail

Change By: Josh Cooper Acceptance Criteria: # Change is made to 6.x and main # The puppet_enterprise_module rspec tests should pass given the reproduction steps in the description

Add Comment

[Aria Li (Jira)]

Aria Li assigned an issue to Aria Li

Puppet / PUP-11435 Puppet 6.26 and 7.13 may cause specs to fail

Change By: Aria Li Assignee: Aria Li

Add Comment

[Christopher Thorn (Jira)]

Christopher Thorn assigned an issue to Christopher Thorn

Puppet / PUP-11435 Puppet 6.26 and 7.13 may cause specs to fail

Change By: Christopher Thorn Assignee: Aria Li Christopher Thorn

Add Comment

[Christopher Thorn (Jira)]

Christopher Thorn commented on PUP-11435

Re: Puppet 6.26 and 7.13 may cause specs to fail This looks good, with the change in puppet I tested the puppet-enterprise-modules failure. Locally in my puppet repo I pulled down the changes on the 6.x branch, then I updated puppet-enterprise-module's gemfile to have this line ` gem 'puppet', :path => '../puppet'` and ran bundle install in order to get bundle to use the puppet that has the fix. Then I re-ran the puppet_enterprise module's spec test and that is now passing.

Add Comment

[Christopher Thorn (Jira)]

Christopher Thorn assigned an issue to Unassigned

Puppet / PUP-11435

Puppet 6.26 and 7.13 may cause specs to fail

Change By: Christopher Thorn Assignee: Christopher Thorn

Add Comment

[Nirupama Mantha (Jira)]

Nirupama Mantha updated an issue

Puppet / PUP-11435 Puppet 6.26 and 7.13 may cause specs to fail

Change By: Nirupama Mantha Release Notes: Bug Fix

Add Comment

[Heston Hoffman (Jira)]

Heston Hoffman updated an issue

Puppet / PUP-11435 Puppet 6.26 and 7.13 may cause specs to fail

Change By: Heston Hoffman Release Notes Summary: Facter caused some module tests to fail.

Add Comment

[Heston Hoffman (Jira)]

Heston Hoffman updated an issue

Puppet / PUP-11435 Puppet 6.26 and 7.13 may cause specs to fail

Change By: Heston Hoffman Release Notes Summary: Facter Faster changes caused some module tests to fail This release fixes an issue where rspec tests would compile with the runner node’s facts instead of using the facts supplied by the test .

Add Comment

[Heston Hoffman (Jira)]

Heston Hoffman updated an issue

Puppet / PUP-11435 Puppet 6.26 and 7.13 may cause specs to fail

Change By: Heston Hoffman Release Notes Summary: Faster changes caused some module Rspec tests to fail with custom facts failed on some modules This release fixes an issue where rspec module tests would compile with the runner node’s facts instead of using the custom facts supplied by the test.

Add Comment

[Parker Leach (Jira)]

Parker Leach updated an issue

Puppet / PUP-11435 Puppet 6.26 and 7.13 may cause specs to fail

Change By: Parker Leach Labels: doc_reviewed

Add Comment

[Parker Leach (Jira)]

Parker Leach updated an issue

Puppet / PUP-11435 Puppet 6.26 and 7.13 may cause specs to fail

Change By: Parker Leach Labels: doc_reviewed docs_reviewed

Add Comment