(#307) Always enable plan parsing #312
Conversation
danielparks
force-pushed
the
fix_plan_parsing
branch
from
74319ad to
7a08cb8
Compare
|
I think I like this version too. I can't immediately think of anything negative that would come out of it.. |
Previously, the tests for `PuppetStrings::Json` (in json_spec.rb) parsed a variety of code samples and then queried YARD to test the results. If a sample failed to parse, an error might be outputted (see [issue puppetlabs#307][puppetlabs#307]), but no failure was registered by the testing harness. This adds simple checks to verify that the parses actually succeed. Unfortunately, one of the samples currently fails to parse, so this commit introduces a testing error. The subbsequent commit will fix it. [puppetlabs#307]: puppetlabs#307
Previously, the parsing code activated task support in Puppet when it detected that it was parsing a file in a plans/ directory. This casued the JSON spec test for plans to fail because it parsed a string, which naturally had no file name to check. This turns on task support (and thus enables parsing plans) whenever the parser starts (assuming that task support is available). This helps avoid surprises to future users who need to parse plans from ruby code, such as in tests.
Previously, none of the examples in parser_spec.rb included a Puppet plan. This adds a plan and verifies that it parses correctly.
danielparks
force-pushed
the
fix_plan_parsing
branch
from
7a08cb8 to
f14c95e
Compare
| # The parsing code actually checks this and sets a global (Puppet[:tasks]). | ||
| # Plan parsing will fail if it hasn't yet encountered a file under plans/. | ||
| let(:file) { 'plans/test.pp' } | ||
| let(:source) { <<~'SOURCE' } |
There was a problem hiding this comment.
I’m using a slightly different style with this heredoc. It seems close to the recommended Ruby style (e.g. foo(<<EOF)), though the Ruby style guide doesn’t say anything about single line blocks.
I’m happy to switch over to wrapping it with the block more like above if that’s what’s preferred. Something like:
let(:source) do
<<~'SOURCE'
# . . .
SOURCE
endI figured the squiggly heredoc is better regardless (it’s been supported since Ruby 2.3). I can update all the other heredocs in the tests in a separate PR if you’re interested — I didn’t want to pollute this one.
There was a problem hiding this comment.
Sounds sensible. I think the work would be a good candidate for a fresh PR 👍
|
I’m going to go ahead and close #311 (thought I’ll keep I’ll keep the branch around until this is merged). |
This is an alternate to #311, which fixes this same problem in a different way. I like this solution better.
Catch parse failures in
PuppetStrings::JsontestsPreviously, the tests for
PuppetStrings::Json(in json_spec.rb) parsed a variety of code samples and then queried YARD to test the results. Ifa sample failed to parse, an error might be outputted (see issue #307), but no failure was registered by the testing harness.This adds simple checks to verify that the parses actually succeed.
Unfortunately, one of the samples currently fails to parse, so this commit introduces a testing error. The subbsequent commit will fix it.
(#307) Fix plan parsing in JSON spec tests
Previously, the parsing code activated task support in Puppet when it detected that it was parsing a file in a plans/ directory. This casued the JSON spec test for plans to fail because it parsed a string, which naturally had no file name to check.
This turns on task support (and thus enables parsing plans) whenever the parser starts (assuming that task support is available).
This helps avoid surprises to future users who need to parse plans from ruby code, such as in tests.
Add a plan test to parser_spec.rb
Previously, none of the examples in parser_spec.rb included a Puppet plan. This adds a plan and verifies that it parses correctly.