Parsing plans in unit tests fail without being caught

[danielparks]

Describe the Bug

It currently fails to parse the plan in spec/unit/puppet-strings/json_spec.rb and fails with error [error]: Failed to parse (stdin): Syntax error at 'param1' (line: 5, column: 19).

Adding a new unit test to parse a plan also fails.

However, it is able to parse plans in the wild, e.g. https://github.com/puppetlabs/puppetlabs-pam_tools/blob/main/REFERENCE.md#plans-1 . I suppose it’s possible that it has stopped working since that was generated; I haven’t yet checked.

This is an offshoot of #302; it seemed like this might be a bigger issue than just warnings during tests so I figured I should split it out.

Expected Behavior

It should parse a plan successfully without generating failures and generate documentation for it.

Steps to Reproduce

Steps to reproduce the behavior:

1. bundle exec rake spec

Environment

• ruby 3.1.2p20
• macOS 12.6.0
• Current main branch (dba16ed)

[danielparks]

I was sort of wrong about the second test. It was only failing because I was only running one file. Running all the tests made it work.

It turns out I can fix the original error by running this code at the top of the file (spec/unit/puppet-strings/json_spec.rb):

YARD::Parser::SourceParser.parse(File.expand_path("../../fixtures/plans/plan.pp", __dir__))
PuppetStrings::Markdown.generate

WTF.

[danielparks]

Oh, or alternatively,

YARD::Parser::SourceParser.parse(File.expand_path("../../fixtures/plans/plan.pp", __dir__))
YARD::Registry.clear

To be clear, both lines are required to make it work, though they don’t have to be in that order.

[danielparks]

Looks like this issue has something to do with the parser checking file paths to see if they match %r{^plans|/plans/}. If they do it might set Puppet[:tasks] = true.

puppet-strings/lib/puppet-strings/yard/parsers/puppet/parser.rb

Lines 25 to 31 in dba16ed

	if @file.to_s.match(/^plans|\/plans\//)
	if Puppet::Util::Package.versioncmp(Puppet.version, "5.0.0") < 0
	log.warn "Skipping #{@file}: Puppet Plans require Puppet 5 or greater."
	return
	end
	Puppet[:tasks] = true if Puppet.settings.include?(:tasks)
	end

If I add Puppet[:tasks] = true to the top of spec/unit/puppet-strings/json_spec.rb it fixes the issue.

[danielparks]

I submitted PR #311 for this, but I keep vacillating on whether or not it’s the right approach. It would be less likely to surprise people if we just moved the Puppet[:tasks] = true if Puppet.settings.include?(:tasks) line above outside of the if statement. In other words, always enable task support if it’s available.

I’m not sure why you would want tasks support off; I have a hard time imaging that it would make much difference since it gets turned on automatically by this code in pretty much all practical use cases aside from testing.