Hey. So, I don't see the tests updated to verify the new assurance, which is
that we will load the fact if it later turns up in the path. It would be
great if you could add the high level behavior test that check we don't
regress on that in future.
Regards,
Daniel
--
Puppet Labs Developer –http://puppetlabs.com
Sent from a mobile device. Please forgive me if this is briefer than usual.
On Jul 26, 2011 2:16 PM, "Adrien Thebo" <[email protected]> wrote:
> If a fact file referenced an unresolvable fact, or a fact that would be
> later in the search path than the fact file, facter would attempt to
> load and resolve that fact first and recurse into the same file.
>
> The loader now stores the path of every file it has attempted to load,
> and before loading a new file it will ensure that the file has not been
> loaded before.
>
> Signed-off-by: Adrien Thebo <[email protected]>
> ---
> Local-branch: ticket/master/8491-remove_multiple_fact_loading
> lib/facter/util/loader.rb | 8 ++++++++
> spec/fixtures/unit/util/loader/nosuchfact.rb | 1 +
> spec/unit/util/loader_spec.rb | 8 ++++++++
> 3 files changed, 17 insertions(+), 0 deletions(-)
> create mode 100644 spec/fixtures/unit/util/loader/nosuchfact.rb
>
> diff --git a/lib/facter/util/loader.rb b/lib/facter/util/loader.rb
> index a52012c..8aaf3e3 100644
> --- a/lib/facter/util/loader.rb
> +++ b/lib/facter/util/loader.rb
> @@ -2,6 +2,11 @@ require 'facter'
>
> # Load facts on demand.
> class Facter::Util::Loader
> +
> + def initialize
> + @loaded = []
> + end
> +
> # Load all resolutions for a single fact.
> def load(fact)
> # Now load from the search path
> @@ -68,10 +73,13 @@ class Facter::Util::Loader
> end
>
> def load_file(file)
> + return if @loaded.include? file
> # We have to specify Kernel.load, because we have a load method.
> begin
> + @loaded << file
> Kernel.load(file)
> rescue ScriptError => detail
> + @loaded -= [file]
> warn "Error loading fact #{file} #{detail}"
> end
> end
> diff --git a/spec/fixtures/unit/util/loader/nosuchfact.rb
b/spec/fixtures/unit/util/loader/nosuchfact.rb
> new file mode 100644
> index 0000000..f0ba60a
> --- /dev/null
> +++ b/spec/fixtures/unit/util/loader/nosuchfact.rb
> @@ -0,0 +1 @@
> +Facter.value(:nosuchfact)
> diff --git a/spec/unit/util/loader_spec.rb b/spec/unit/util/loader_spec.rb
> index 1bc909f..3fb1f04 100755
> --- a/spec/unit/util/loader_spec.rb
> +++ b/spec/unit/util/loader_spec.rb
> @@ -280,4 +280,12 @@ describe Facter::Util::Loader do
> @loader.load_all
> end
> end
> +
> + it "should load facts on the facter search path only once" do
> + facterlibdir = File.expand_path(File.dirname(__FILE__) +
'../../../fixtures/unit/util/loader')
> + with_env 'FACTERLIB' => facterlibdir do
> + Facter::Util::Loader.new.load_all
> + Facter.value(:nosuchfact).should be_nil
> + end
> + end
> end
> --
> 1.7.6
>
> --
> You received this message because you are subscribed to the Google Groups
"Puppet Developers" group.
> To post to this group, send email to [email protected].
> To unsubscribe from this group, send email to
[email protected].
> For more options, visit this group at
http://groups.google.com/group/puppet-dev?hl=en.
>
--
You received this message because you are subscribed to the Google Groups
"Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/puppet-dev?hl=en.
