Custom facts with multiple setcode points have inaccurate coverage reports

[rnelson0]

The coverage report generated for the test of a custom fact with multiple setcode points is dependent on the order of the tests; only the last test seems to count as covered. Given this custom fact:

# lib/facter/puppet_role.rb
# ([a-z]+)[0-9]+, i.e. www01 or logger22 have a puppet_role of www or logger
if Facter.value(:hostname) =~ /^([a-z]+)[0-9]+$/
  Facter.add('puppet_role') do
    setcode do
      $1
    end
  end

# ([a-z]+), i.e. www or logger have a puppet_role of www or logger
elsif Facter.value(:hostname) =~ /^([a-z]+)$/
  Facter.add('puppet_role') do
    setcode do
      $1
    end
  end

# Set to hostname if no patterns match
else
  Facter.add('puppet_role') do
    setcode do
      'default'
    end
  end
end

There are three setcode points that need tested. Running rspec without any tests for this fact results in:

$ be rake spec_standalone
<lots of unrelated stuff>

Coverage report generated for RSpec to /home/rnelson0/puppet/controlrepo/dist/profile/coverage. 3 / 10 LOC (30.0%) covered.

COVERAGE:  30.00% -- 3/10 lines in 1 files

+----------+----------------------------------------+-------+--------+-----------------+
| coverage | file                                   | lines | missed | missing         |
+----------+----------------------------------------+-------+--------+-----------------+
|  30.00%  | dist/profile/lib/facter/puppet_role.rb | 10    | 7      | 5, 11-13, 19-21 |
+----------+----------------------------------------+-------+--------+-----------------+

With this spec code:

# spec/unit/facter/puppet_role_spec.rb
require 'spec_helper'
require 'facter/puppet_role'

describe 'puppet_role', :type => :fact do
  before { Facter.clear }
  after { Facter.clear }

  context "test72" do
    before do
     Facter.fact(:hostname).stubs(:value).returns('test72')
    end

    it "should return test" do
      expect(Facter.fact(:puppet_role).value).to eq('test')
    end
  end

  context "role" do
    before do
     Facter.fact(:hostname).stubs(:value).returns('role')
    end

    it "should return role" do
      expect(Facter.fact(:puppet_role).value).to eq('role')
    end
  end

  context "default" do
    before do
     Facter.fact(:hostname).stubs(:value).returns('99luftballoons')
    end

    it "should return default" do
      expect(Facter.fact(:puppet_role).value).to eq('default')
    end
  end
end

This is the result:

+----------+----------------------------------------+-------+--------+------------+
| coverage | file                                   | lines | missed | missing    |
+----------+----------------------------------------+-------+--------+------------+
|  40.00%  | dist/profile/lib/facter/puppet_role.rb | 10    | 6      | 3-5, 11-13 |
+----------+----------------------------------------+-------+--------+------------+

By re-ordering the tests, I will receive different missing values, where all but the last test count as missing:

# Moving the first test to the end...
require 'spec_helper'
require 'facter/puppet_role'

describe 'puppet_role', :type => :fact do
  before { Facter.clear }
  after { Facter.clear }

  context "role" do
    before do
     Facter.fact(:hostname).stubs(:value).returns('role')
    end

    it "should return role" do
      expect(Facter.fact(:puppet_role).value).to eq('role')
    end
  end

  context "default" do
    before do
     Facter.fact(:hostname).stubs(:value).returns('99luftballoons')
    end

    it "should return default" do
      expect(Facter.fact(:puppet_role).value).to eq('default')
    end
  end

  context "test72" do
    before do
     Facter.fact(:hostname).stubs(:value).returns('test72')
    end

    it "should return test" do
      expect(Facter.fact(:puppet_role).value).to eq('test')
    end
  end
end

# Results in...
+----------+----------------------------------------+-------+--------+--------------+
| coverage | file                                   | lines | missed | missing      |
+----------+----------------------------------------+-------+--------+--------------+
|  40.00%  | dist/profile/lib/facter/puppet_role.rb | 10    | 6      | 11-13, 19-21 |
+----------+----------------------------------------+-------+--------+--------------+

Here is my Rakefile, spec_helper.rb, and bundle gem versions:

$ cat Rakefile
require 'rubygems'
require 'bundler/setup'

require 'puppetlabs_spec_helper/rake_tasks'
require 'puppet/version'
require 'puppet/vendor/semantic/lib/semantic' unless Puppet.version.to_f < 3.6
require 'puppet-lint/tasks/puppet-lint'
require 'puppet-syntax/tasks/puppet-syntax'
require 'metadata-json-lint/rake_task'
require 'rubocop/rake_task'

# These gems aren't always present, for instance
# on Travis with --without development
begin
  require 'puppet_blacksmith/rake_tasks'
rescue LoadError # rubocop:disable Lint/HandleExceptions
end

RuboCop::RakeTask.new

exclude_paths = [
  "bundle/**/*",
  "pkg/**/*",
  "vendor/**/*",
  "spec/**/*",
]

# Coverage from puppetlabs-spec-helper requires rcov which
# doesn't work in anything since 1.8.7
Rake::Task[:coverage].clear

Rake::Task[:lint].clear

PuppetLint.configuration.relative = true
PuppetLint.configuration.disable_80chars
PuppetLint.configuration.disable_arrow_alignment
PuppetLint.configuration.disable_class_inherits_from_params_class
PuppetLint.configuration.disable_class_parameter_defaults
PuppetLint.configuration.fail_on_warnings = true

PuppetLint::RakeTask.new :lint do |config|
  config.ignore_paths = exclude_paths
end

PuppetSyntax.exclude_paths = exclude_paths

desc "Run acceptance tests"
RSpec::Core::RakeTask.new(:acceptance) do |t|
  t.pattern = 'spec/acceptance'
end

desc "Populate CONTRIBUTORS file"
task :contributors do
  system("git log --format='%aN' | sort -u > CONTRIBUTORS")
end

desc "Run syntax, lint, and spec tests."
task :test => [
  :metadata_lint,
  :syntax,
  :lint,
  :rubocop,
  :spec,
]

$ cat spec/spec_helper.rb
require 'puppetlabs_spec_helper/module_spec_helper'
require 'rspec-puppet-facts'

include RspecPuppetFacts

require 'simplecov'
require 'simplecov-console'

SimpleCov.start do
  add_filter '/spec'
  add_filter '/vendor'
  formatter SimpleCov::Formatter::MultiFormatter[
    SimpleCov::Formatter::HTMLFormatter,
    SimpleCov::Formatter::Console
  ]
end

RSpec.configure do |c|
  c.hiera_config = File.expand_path(File.join(__FILE__, '../fixtures/hiera.yaml'))
end

$ be gem list

*** LOCAL GEMS ***

addressable (2.4.0)
ansi (1.5.0)
ast (2.2.0)
astrolabe (1.3.1)
backports (3.6.7)
bundler (1.10.6)
coderay (1.1.0)
diff-lcs (1.2.5)
docile (1.1.5)
domain_name (0.5.25)
ethon (0.8.1)
facter (2.4.5)
facterdb (0.3.1)
faraday (0.9.2)
faraday_middleware (0.10.0)
ffi (1.9.10)
formatador (0.2.5)
generate-puppetfile (0.9.6)
gh (0.14.0)
guard (2.13.0)
guard-rake (1.0.0)
hiera (3.0.5)
highline (1.7.8)
hirb (0.7.3)
http-cookie (1.0.2)
jgrep (1.4.0)
json (1.8.3)
json_pure (1.8.3)
launchy (2.4.3)
listen (3.0.5)
lumberjack (1.0.10)
metaclass (0.0.4)
metadata-json-lint (0.0.11)
method_source (0.8.2)
mime-types (2.99)
mocha (1.1.0)
multi_json (1.11.2)
multipart-post (2.0.0)
nenv (0.2.0)
net-http-persistent (2.9.4)
net-http-pipeline (1.0.1)
netrc (0.11.0)
notiffany (0.0.8)
parser (2.2.3.0)
powerpack (0.1.1)
pry (0.10.3)
puppet (4.3.1)
puppet-blacksmith (3.3.1)
puppet-lint (1.1.0)
puppet-lint-absolute_classname-check (0.1.3)
puppet-lint-classes_and_types_beginning_with_digits-check (0.1.0)
puppet-lint-leading_zero-check (0.1.0)
puppet-lint-trailing_comma-check (0.3.1)
puppet-lint-unquoted_string-check (0.2.5)
puppet-lint-version_comparison-check (0.1.2)
puppet-syntax (2.0.0)
puppetlabs_spec_helper (1.0.1)
pusher-client (0.6.2)
rainbow (2.0.0)
rake (10.5.0)
rb-fsevent (0.9.7)
rb-inotify (0.9.5)
rest-client (1.8.0)
rspec (3.1.0)
rspec-core (3.1.7)
rspec-expectations (3.1.2)
rspec-mocks (3.1.3)
rspec-puppet (2.3.0)
rspec-puppet-facts (1.3.0)
rspec-support (3.1.2)
rubocop (0.33.0)
ruby-progressbar (1.7.5)
shellany (0.0.1)
simplecov (0.11.1)
simplecov-console (0.3.0)
simplecov-html (0.10.0)
slop (3.6.0)
spdx-licenses (1.0.0)
thor (0.19.1)
travis (1.8.2)
travis-lint (2.0.0)
typhoeus (0.8.0)
unf (0.1.4)
unf_ext (0.0.7.1)
websocket (1.2.2)

[DavidS]

This seems to be related to, if not a duplicate of #316 . There seems to be a fundamental issue in how ruby code coverage is setup up by rspec-puppet, but I haven't had any luck in uncovering what's happening, when I looked the last time :-(

[rodjek]

Unfortunately, this is not something that can be easily worked around. SimpleCov uses the builtin Coverage library in Ruby which is reset every time the file is loaded. Because of the way that this fact is written (if statement around multiple calls to Facter.add so that only one resolution is ever loaded), it has to be reloaded in order to change the value after the hostname value changes.

For example, if this fact were rewritten like

Facter.add(:puppet_role) do
  confine :hostname do |value|
    value =~ /^[a-z]+[0-9]+$/
  end

  setcode { Facter.value(:hostname)[/^([a-z]+)[0-9]$/, 1] }
end

Facter.add(:puppet_role) do
  confine :hostname do |value|
    value =~ /^[a-z]+$/
  end

  setcode { Facter.value(:hostname) }
end

Facter.add(:puppet_role) do
  setcode { 'default' }
end

Then the spec can be rewritten to not require the use of Facter.clear, which will prevent the facts from being reloaded from disk and you'll get a complete coverage report.

[rnelson0]

@rodjek I finally got back to this and am still seeing issues. Here's the new fact file and tests:

#dist/profile/lib/facter/roles.rb
# ([a-z]+)[0-9]+, i.e. www01 or logger22 have a puppet_role of www or logger
Facter.add(:puppet_role) do
  confine :hostname do |value|
    value =~ /^[a-z]+[0-9]+$/
  end
  setcode { Facter.value(:hostname)[/^([a-z]+)[0-9]+$/, 1] }
end

# ([a-z]+), i.e. www or logger have a puppet_role of www or logger
Facter.add(:puppet_role) do
  confine :hostname do |value|
    value =~ /^[a-z]+$/
  end
  setcode { Facter.value(:hostname) }
end

# Set to 'default' if no patterns match
Facter.add(:puppet_role) do
  setcode { 'default' }
end

# spec/unit/facter/puppet_role_spec.rb
require 'spec_helper'
require './dist/profile/lib/facter/roles.rb'

describe 'puppet_role', :type => :fact do
  context "test72" do
    before do
      Facter.fact(:hostname).stubs(:value).returns('test72')
    end

    it "should return test" do
      expect(Facter.fact(:puppet_role).value).to eq('test')
    end
  end

  context "role" do
    before do
      Facter.fact(:hostname).stubs(:value).returns('role')
    end

    it "should return role" do
      expect(Facter.fact(:puppet_role).value).to eq('role')
    end
  end

  context "default" do
    before do
      Facter.fact(:hostname).stubs(:value).returns('99luftballoons')
    end

    it "should return default" do
      expect(Facter.fact(:puppet_role).value).to eq('default')
    end
  end
end

Here's the output:

puppet_role
  test72
    should return test
  role
    should return role (FAILED - 1)
  default
    should return default (FAILED - 2)

Failures:

  1) puppet_role role should return role
     Failure/Error: expect(Facter.fact(:puppet_role).value).to eq('role')

       expected: "role"
            got: "test"

       (compared using ==)
     # ./spec/unit/facter/puppet_role_spec.rb:22:in `block (3 levels) in <top (required)>'

  2) puppet_role default should return default
     Failure/Error: expect(Facter.fact(:puppet_role).value).to eq('default')

       expected: "default"
            got: "test"

       (compared using ==)
     # ./spec/unit/facter/puppet_role_spec.rb:32:in `block (3 levels) in <top (required)>'

Finished in 0.01986 seconds (files took 1.08 seconds to load)
3 examples, 2 failures

Failed examples:

rspec ./spec/unit/facter/puppet_role_spec.rb:21 # puppet_role role should return role
rspec ./spec/unit/facter/puppet_role_spec.rb:31 # puppet_role default should return default

Coverage report generated for RSpec to /home/rnelson0/puppet/controlrepo/coverage. 10 / 10 LOC (100.0%) covered.

COVERAGE: 100.00% -- 10/10 lines in 1 files

The coverage is up, but removing the clear appears to not reset the fact value between runs. If I add Facter.clear back in, then I get nil for all the values. Not sure what is going on here or how to fix it.

[rodjek]

@rnelson0 Sorry, I missed a bit. You'll need to call Facter.flush after stubbing the hostname fact. This will flush the cached value but not reload any ruby files.

asmodean :0: tmp/rnelson0 » cat test.rb 
require 'facter'
require 'mocha'
require 'simplecov'
require 'simplecov-console'

SimpleCov.start do
  add_filter '/spec'
  add_filter '/vendor'

  formatter SimpleCov::Formatter::Console
end

# Configure rspec to use mocha to match setup done by puppetlabs_spec_helper
RSpec.configure do |c|
  c.mock_with :mocha
end

require './lib/facter/puppet_role.rb'

describe 'puppet_role' do
  let(:hostname_fact) { self.class.description }
  before(:each) do
    Facter.fact(:hostname).stubs(:value).returns(hostname_fact)
    Facter.flush
  end
  subject { Facter.fact(:puppet_role).value }

  context 'test72' do
    it { is_expected.to eq('test') }
  end

  context 'role' do
    it { is_expected.to eq('role') }
  end

  context '99luftballoons' do
    it { is_expected.to eq('default') }
  end
end

asmodean :0: tmp/rnelson0 » bundle exec rspec test.rb --format documentation

puppet_role
  test72
    should eq "test"
  role
    should eq "role"
  99luftballoons
    should eq "default"

Finished in 0.00365 seconds (files took 0.14032 seconds to load)
3 examples, 0 failures

COVERAGE: 100.00% -- 10/10 lines in 1 files

[rnelson0]

@rodjek Thanks so much, I finally found time to get back to this and implement in production. Awesomesauce!

[rodjek]

Woot, glad to hear it worked!