Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(PUP-10524) Address spec failures when running with facter 4 #8165

Merged
merged 3 commits into from May 25, 2020

Conversation

melissa
Copy link
Contributor

@melissa melissa commented May 20, 2020

Fixes still required

  • $CHILD_STATUS returns nil
  • trace logging

@melissa melissa requested review from a team May 20, 2020 00:58
@melissa melissa force-pushed the maint/master/facter-4-and-friends branch from dc03ea9 to d625643 Compare May 20, 2020 01:00
@melissa melissa force-pushed the maint/master/facter-4-and-friends branch from d625643 to 51eb3a7 Compare May 20, 2020 01:03
@puppetcla
Copy link

CLA signed by all contributors.

@melissa melissa changed the title Maint/master/facter 4 and friends (PUP-10524) Address spec failures when running with facter 4 May 20, 2020
@BogdanIrimie
Copy link
Contributor

BogdanIrimie commented May 21, 2020

@melissa this is great!

The trace problem was fixed today in https://github.com/puppetlabs/facter-ng/pull/527/files

The test that fail with the $CHILD_STATUS error mock texecute or execute from execution.rb. Since the execution is mocked, a new process is never created and there is no exit status. It was just a coincidence that Facter 2 spawned a new process and it's exit status was interpreted as the exit status from texecute or execute

In order to fix the test we can add a before block to the test e.g

  before(:all) do
    `exit 0`
  end

With the changes from this branch, the $CHILD_STATUS fix and Facter 4 master branch, the unit test pass

Finished in 9 minutes 53 seconds (files took 8.71 seconds to load)
25800 examples, 0 failures, 39 pending

@BogdanIrimie
Copy link
Contributor

BogdanIrimie commented May 22, 2020

@melissa I pushed the changes for $CHILD_STATUS on your branch and released Facter 4.0.23 today (with the trace fix). Puppet test should pass with these changes.

@@ -4,6 +4,10 @@
if: Puppet.features.posix? && !Puppet::Util::Platform.jruby?do
let(:provider_class) { Puppet::Type.type(:service).provider(:redhat) }

before(:all) do
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @IrimieBogdan ! Could you add a comment to the code here explaining why we have to do this? I can see someone getting really confused a year from now as to why this is needed :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a comments describing the fix and why it is required.

@puppetcla
Copy link

CLA signed by all contributors.

@gimmyxd gimmyxd removed the WIP label May 25, 2020
@gimmyxd gimmyxd merged commit b4119c7 into puppetlabs:master May 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants