rtyler/puppet-jenkins

Tests need a mocked apt-module to work

Closed this issue · 8 comments

rtyler commented

Merging #8 predictably broke the rspec-puppet tests, should probably fix that

Duh

reidmv commented

Looks like the test failures are related to the following block of code in the apt module:

  if ! $release {
    fail("lsbdistcodename fact not available: release parameter required")
  }

The apt::source resource as defined in in the jenkins module does specify a release parameter, as an empty string:

  apt::source { "jenkins":
    location    => "http://pkg.jenkins-ci.org/debian",
    release     => "",
    repos       => "binary/",
    key         => "D50582E6",
    key_source  => "http://pkg.jenkins-ci.org/debian/jenkins-ci.org.key",
    include_src => false,
  }

This works just fine in practice. However, it looks like when rspec-puppet is running with it, that empty string gets interpreted to nil or some other False value. The three possible fixes for this are

  1. Figure out the how and where in puppet-rspec or rspec that lets this happen, and fix it.
  2. Change jenkins/manifets/repo/debian.pp to specify release => " " -- while an empty string is rspec-false it seems that whitespace is not.
  3. Change apt/manifets/source.pp to use the modified test if $release == undef {.

Brief perusal of open puppet issues on projects.puppetlabs.com relating to the undef type indicates that Option 1 might be "hard". Option 2 is ugly. I don't know enough about the internals of puppet to know if Option 3 is a good long-term fix, but I'm leaning towards it as my favorite. I'll look towards opening a pull request against the apt module to that effect and we'll see how well it flies.

See: puppetlabs/puppet-apt/pull/31

nanliu commented

In your rspec-puppet unit test, can you provide the appropriate lsbdistcodename fact?

let(:facts) { { :lsbdistcodename => 'some_val' } }

Would this resolve it?

reidmv commented

Hi @nanliu,

Unfortunately, it looks like no. The $release variable is specified explicitly in manifests/repo/debian.pp as the empty string, so adding lsbdistcodename to facts won't affect the test run. The puppet code itself is all perfectly functional; it's just the tests that are failing, seemingly because somewhere inside /rspec(-puppet)?/ the empty string is being interpreted as something that evaluates to false.

nanliu commented

Ah, in this case yes we need to change the manifests to allow ''.

reidmv commented

I keep forgetting that the puppet DSL doesn't behave quite the same as ruby. There's no problem with rspec, the issue is that to puppet itself the empty string is False. The following snippet demonstrates this when run.

$string = ""
if ! $string {
  fail("The test 'if ! \$string' results in True with string value: '$string'")
}

The apt module fix suggested at puppetlabs/puppet-apt/pull/31 then is still applicable, and imo the best way to address this issue.

reidmv commented

Hi @rtyler,

nanliu just merged the fix into puppetlabs/puppet-apt. The jenkins tests all pass now when used against apt (master).

Thanks @nanliu!

rtyler commented

That's part of the problem, the other part of the problem is that I (being a dirty rpm user) shouldn't need to pull puppet-apt into the environment to run these tests, thus my thread about mocking on the puppet-users list to which @jeffmccune replied.

reidmv commented

rtyler/puppet-jenkins/pull/10 is what I could come up with for this. Feels like a hack, but that's my best stab for now at how you might go about getting what you want.

P.S.
puppetlabs/apt has been released on the forge