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.

Sign up for GitHub

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

Jump to bottom

Tests are failing for Facter 4 #238

Closed
glennsarti opened this issue Apr 16, 2020 · 9 comments · Fixed by #239
Closed

Tests are failing for Facter 4 #238

glennsarti opened this issue Apr 16, 2020 · 9 comments · Fixed by #239
Labels
bug Something isn't working

Comments

@glennsarti
Copy link
Contributor

The Travis and Appveyor tests are failing now that Facter 4 is available.
@glennsarti glennsarti added the bug Something isn't working label Apr 16, 2020
@DavidS
Copy link
Contributor

DavidS commented Apr 16, 2020

See the failures at the bottom of #236

@glennsarti
Copy link
Contributor Author

Okay, so it's a bug in Facter 4.0.16. 4.0.15 is fine. I am 99% sure it's due to https://github.com/puppetlabs/facter-ng/blame/master/lib/facter.rb#L189

@DavidS
Copy link
Contributor

DavidS commented Apr 16, 2020

@glennsarti do you have a short repro? I can follow up with the team

@glennsarti
Copy link
Contributor Author

glennsarti commented Apr 16, 2020 •
edited
Loading 

bundle exec rspec --default .\spec\languageserver-sidecar\ -f d spec/languageserver-sidecar/integration/puppet-languageserver-sidecar/facter_helper_spec.rb:40

Apologies for Windows paths.

Will fail to find the custom fact in the cache dir fixture.

If you lock Facter to 4.0.15 e.g. in Gemfile add gem 'facter', '=4.0.15', redo the bundle install and run the test again it passes.

@glennsarti
Copy link
Contributor Author

bundle exec puppet facts --vardir C:/Source/puppet-editor-services/spec/languageserver-sidecar/fixtures/real_agent/cache --confdir C:/Source/puppet-editor-services/spec/languageserver-sidecar/fixtures/real_agent/confdir Still works so it's something I'm not initialising correctly, but that reset call I linked earlier seems a really bad idea.

@glennsarti
Copy link
Contributor Author

Interestingly... using Editor Services Facter.reset gets called twice, but puppet facts calls it three times!!

I guess it must REALLY be reset by then...

@glennsarti
Copy link
Contributor Author

glennsarti commented Apr 16, 2020 •
edited
Loading

The failing tests failed to find a custom fact in the puppet cache dir, or in a modulepath

glennsarti added a commit to glennsarti/puppet-editor-services that referenced this issue Apr 16, 2020
@glennsarti
(puppetlabsGH-238) Monkey patch Facter for minimal resets
f8e88a5 
It appears that Facter 4.0.16 is resetting the Facter search dirs
which then causes the sidecar to not find some external or custom facts.
This commit monkey patches the Facter module and makes it possible
to limit the number of resets to one.
@glennsarti glennsarti mentioned this issue Apr 16, 2020
Merged
@BogdanIrimie
Copy link

@glennsarti with the fix from https://github.com/puppetlabs/facter-ng/tree/FACT-2562-2 the tests pass.
To run the test I have used

export PUPPET_GEM_VERSION="~> 6.0"
bundle exec rake test_languageserver test_languageserver_sidecar test_debugserver

@glennsarti
Copy link
Contributor Author

I had a look at the branch. It does concern me there weren't tests for this though.

jpogran pushed a commit that referenced this issue Apr 18, 2020
Merge pull request #239 from glennsarti/gh238-facter4-patches
f86063e 
(GH-238) Monkey patch Facter for minimal resets
jpogran added a commit to jpogran/puppet-editor-services that referenced this issue Apr 22, 2020
@jpogran
Revert "(puppetlabsGH-238) Monkey patch Facter for minimal resets"
882fa21 
This reverts commit f8e88a5.
glennsarti added a commit that referenced this issue Apr 23, 2020
@glennsarti
Merge pull request #241 from jpogran/revert-gh-238
f2e758e 
Revert "(GH-238) Monkey patch Facter for minimal resets"
jpogran added a commit to jpogran/puppet-editor-services that referenced this issue Apr 28, 2020
@jpogran
Revert "(puppetlabsGH-238) Monkey patch Facter for minimal resets"
b34fd7b 
This reverts commit f8e88a5.
glennsarti added a commit to glennsarti/puppet-editor-services that referenced this issue May 16, 2020
@glennsarti
Merge branch 'master' of https://github.com/glennsarti/puppet-editor-…
447f61b 
…services into mergeup-master

* 'master' of https://github.com/glennsarti/puppet-editor-services:
  (maint) Release 0.26.0
  (puppetlabsGH-245) Remove facts_all sidecar request
  (puppetlabsGH-245) Use object cache for fact data
  (puppetlabsGH-245) Add acceptance tests for getFacts request
  (puppetlabsGH-242) Puppet Facts Endpoint
  Revert "(puppetlabsGH-238) Monkey patch Facter for minimal resets"
  (maint) Update branding urls
  (puppetlabsGH-238) Monkey patch Facter for minimal resets
  (maint) Update rubocop to 0.80.x
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

(GH-238) Monkey patch Facter for minimal resets glennsarti/puppet-editor-services
3 participants
@DavidS @BogdanIrimie @glennsarti