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-11022) Resolve order dependent test failures #8567

Merged
merged 4 commits into from
Apr 14, 2021

Conversation

joshcooper
Copy link
Contributor

This resolves order dependent test failures seen when running:

$ bundle exec rspec spec/integration/application/plugin_spec.rb spec/unit/gettext/config_spec.rb
$ bundle exec rspec spec/integration/application/plugin_spec.rb spec/unit/provider/service/init_spec.rb
$ bundle exec rspec spec/unit/util/selinux_spec.rb spec/unit/transaction_spec.rb
$ bundle exec rspec spec/unit/transaction/additional_resource_generator_spec.rb spec/unit/transaction_spec.rb

There will be merge conflicts when merging to main, I'll take care of that after this passes CI

@joshcooper joshcooper requested review from a team April 13, 2021 21:25
Copy link
Contributor

@Magisus Magisus left a comment

Choose a reason for hiding this comment

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

Confirmed that this fixed all of the failures for me!

Newer facter versions call Dir.entries during fact resolution/provider loading,
so call the original method by default. This fixes errors of the form:

    #<Dir (class)> received :entries with unexpected arguments
      expected: ("tmp")
      got: ("/Users/josh/.facter/facts.d")
The additional_resource_generator_spec.rb test removes the `generator` type
that both it and the transaction_spec.rb defined. Change the latter to register
a different name. This resolves errors like:

    Failure/Error: Puppet::Type.type(:generator).new :title => "generator" }
      NoMethodError:
        undefined method `new' for nil:NilClass
Fix order dependent test failures when running spec/unit/util/selinux_spec.rb
followed by spec/unit/transaction_spec.r:b

    Failure/Error: expect(Selinux).to receive(:matchpathcon_fini)
      Selinux does not implement: matchpathcon_fini

stub_const/hide_const automatically restore the previous value when the test
completes.
Fix order dependent test when running spec/integration/application/plugin_spec.rb
followed by spec/unit/gettext/config_spec.rb. The former disables i18n due to
the settings hook, and that breaks assumptions made by the latter.
@joshcooper joshcooper closed this Apr 14, 2021
@joshcooper joshcooper reopened this Apr 14, 2021
@joshcooper joshcooper merged commit bbb35f7 into puppetlabs:6.x Apr 14, 2021
@joshcooper joshcooper deleted the order_dependent_11022 branch April 14, 2021 23:16
@joshcooper joshcooper restored the order_dependent_11022 branch April 15, 2021 02:53
@@ -83,6 +83,7 @@
allow(provider_class).to receive(:defpath).and_return('tmp')

@services = ['one', 'two', 'three', 'four', 'umountfs']
allow(Dir).to receive(:entries).and_call_original
Copy link
Contributor

@gimmyxd gimmyxd Apr 15, 2021

Choose a reason for hiding this comment

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

I was a bit worried why this surfaced just know. Seems that is reproducible only if you have the default external facts dir created($HOME/.facter/facts.d). On Facter.value, Facter tries to load external facts so this means that a call to Facter.value goes to the system. I would expect this would also have failed with Facter 4.0.52.

The call that is not stubbed is coming from ./lib/puppet/provider/service/init.rb:57:in 'excludes' so i believe a better fix would be to stub Facter.value in https://github.com/puppetlabs/puppet/blob/main/spec/unit/provider/service/init_spec.rb#L81-L82

@@ -80,6 +80,7 @@

   describe "when getting all service instances" do
     before :each do
+      allow(Facter).to receive(:value).with(:osfamily).and_return('redhat')
       allow(provider_class).to receive(:defpath).and_return('tmp')

       @services = ['one', 'two', 'three', 'four', 'umountfs']

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 sounds good, I'll take a look today

@joshcooper joshcooper deleted the order_dependent_11022 branch April 15, 2021 14:42
@joshcooper joshcooper restored the order_dependent_11022 branch April 15, 2021 15:27
@joshcooper joshcooper deleted the order_dependent_11022 branch April 15, 2021 15:27
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

3 participants