TypeError: no implicit conversion of nil into String when directories are missing

[LoganBarnett]

Preamble

I searched for this kind of issue in this GitHub repo. Additionally, per the README instructions, I tried to raise this ticket in the Jira tracker (https://tickets.puppetlabs.com/secure/Dashboard.jspa) but I can only create CVE tickets. If that's fine, I'm happy to move or clone the ticket there.

Describe the Bug

Missing module directories can cause require 'puppetlabs_spec_helper/module_spec_helper' to fail with TypeError: no implicit conversion of nil into String.

Expected Behavior

We shouldn't be seeing this error. Perhaps a different one?

Steps to Reproduce

1. Clone a fresh module that uses require 'puppetlabs_spec_helper/module_spec_helper' in its spec_helper.rb.
2. Install your gems (bundle install)
3. Run bundle exec rspec (I realize this is probably not supported, bear with me please).
4. Observe error.

Environment

Relevant parts of my Gemfile:

group :development, :unit_tests do
  gem 'guard-rspec', '~> 4.7',             :require => false
  gem 'guard-rake', '~> 1.0.0',            :require => false
  gem 'net-ldap', '~> 0.14',               :require => false
  gem 'rake',                              :require => false
  gem 'rspec', '~> 3.7',                   :require => false
  gem 'rspec-core', '~> 3.7',              :require => false
  gem 'rspec-puppet', '~> 3.0',            :require => false
  gem 'rspec-puppet-facts', '~> 1.9',      :require => false
  gem 'rspec-its', '~> 1.2.0',             :require => false
  gem 'rubocop',                           :require => false
  gem 'rubocop-performance',               :require => false
  gem 'rubocop-rspec',                     :require => false
  gem 'puppetlabs_spec_helper', '~> 6.0.1',:require => false
  gem 'puppet-blacksmith', '~> 3.4.0',     :require => false
  gem 'puppet-lint', '~> 4.0',             :require => false
  gem 'simplecov',                         :require => false
  gem 'simplecov-console',                 :require => false
  # An undeclared dependency for rspec-puppet.
  gem 'sync',                              :require => false
  gem 'parallel_tests',                    :require => false
  gem 'puppet_facts',                      :require => false
  gem 'pry',                               :require => false
  gem 'puppet', '~> 6.29',                 :require => false
  gem 'facter', '~> 2.5',                  :require => false
  gem 'puppet-retrospec', '~> 1.4',        :require => false
end

$ ruby -v
ruby 2.7.5p203 (2021-11-24 revision f69aeb8314) [x86_64-linux]

$ lsb_release -a
LSB Version:	:core-4.1-amd64:core-4.1-noarch
Distributor ID:	CentOS
Description:	CentOS Linux release 7.9.2009 (Core)
Release:	7.9.2009
Codename:	Core

Additional Context

I'm new to Puppet. I get the impression that running rspec directly is a no-no. Or perhaps rspec can be run directly after bundle exec rake spec has been run, where this package has had a chance to pull in the module files? I would hope one could run rspec for being able to run select tests. I don't know if that's remotely a responsibility of this package. I digress.

What I have found is that there seems to be a logic issue which just happens to work for the common case but my rspec runs did not allow for. I was able to trace the error here:

components = module_path.split(File::PATH_SEPARATOR).map do |dir|
  next unless Dir.exist? dir

  Dir.entries(dir).grep_v(/^\./).map { |f| File.join(dir, f, 'spec', 'lib') }
end
components.flatten.each do |d|
  $LOAD_PATH << d if FileTest.directory?(d) && !$LOAD_PATH.include?(d)
end

The issue manifests on the $LOADPATH << ... line. I found out this is because the prior #map loop includes a next, and my freshly cloned environment did not have the existing directories that were being checked for. If the components.flatten.each... becomes this:

components.compact.flatten.each ...

This fixes the issue. Another approach could be to remove the next line entirely, but that might be undesirable. Doing next in #map yields a nil for that iteration of the loop. For example, if we're mapping over an array of one element and the directory doesn't exist, components will be [ nil ]. I imagine FileTest.directory? is the one expecting a String but my debugging didn't get that far.

I looked around the repo and don't see an obvious place to create tests to produce this issue and then apply a fix as a clean PR. With some assistance I would be happy to do that legwork :)

I want to repeat that I realize my entry-point for this issue might have been flawed in the first place, but it seems like a logic issue regardless? At the very least, maybe the next person who makes the same mistake that I do will stumble across this and benefit from the findings.

Anyways, thanks so much for your work on this package! This is a critical component to several dozen Puppet modules we manage in our team internally at $WORK.

[jordanbreen28]

@LoganBarnett Thanks for raising this. And apologies for such a delay in response.
The reason you are seeing this issue is you havent run the spec_prep or spec rake tasks (spec will run spec_prep if not already).
Running bundle exec rake spec_prep is a prerequisite will prepare your fixtures directory (including a symlink to the current module) which is required before executing the tests. Running the rake task should fix this issue for you. Once the spec_prep rake task has been ran, you will be able to run bundle exec rspec /path/to/spec.rb as you would in any other instance. (Still not recommended however!)
Hope this helps!

[LoganBarnett]

That helps a lot! Thank you! Maybe if I get some cycles, I submit a PR for some error handling that would provide some direction to folks who missed that setup step. Would that be agreeable, at least a high level?

[jordanbreen28]

@LoganBarnett agreed, that makes the whole thing a lot easier for first time/unexperienced users! That would be absolutely fantastic!