voxpupuli / onceover

Your gateway drug to automated infrastructure testing with Puppet
Apache License 2.0
142 stars 45 forks source link

Fix and document :manifest opt #290

Closed op-ct closed 3 years ago

op-ct commented 3 years ago

Before this patch, running onceover run spec with --manifest PATH or when opts[:manifest] is set in the spec/onceover.yaml file would result in the Error #<TypeError: no implicit conversion of nil into String>.

This patch fixes that issue by correcting the initialization logic for configuring the manifests directory, and ensuring the path is used as the value for c.manifest setting in .onceover/spec/spec_helper.rb.

dylanratcliffe commented 3 years ago

As discussed in Slack, tests are failing due to the fact that the site.pp is now actually being honoured (yay!). My question is: Isn't the point of this that it defaults to nil, and doesn't this failing test mean that it's not successfully defaulting to nil?

op-ct commented 3 years ago

@dylanratcliffe Good catch!

The more I think about it, defaulting to nil is a bug; in the absence of any other setting, Puppet defaults to the environment's manifests directory. I've updated the patch to make this intention clear.

dylanratcliffe commented 3 years ago

I"m going to merge this then fix the broken test in master