(SDK-148) Add "test unit --list" #107
Conversation
lib/pdk/tests/unit.rb
Outdated
| rspec_json_output = JSON.parse(output[:stdout]) | ||
| if rspec_json_output['examples'].empty? | ||
| rspec_message = rspec_json_output['messages'][0] | ||
| raise PDK::CLI::FatalError, _('Unable to enumerate examples. rspec reported: %{message}' % { message: rspec_message }) unless rspec_message == 'No examples found.' |
There was a problem hiding this comment.
I'm not a fan of the 'guard clause' that rubocop pushes me towards here - the 'if' clause of this raise is off the edge of the screen :/
james-stocks
force-pushed
the
SDK-148
branch
3 times, most recently
from
d290aad to
89a1cc6
Compare
|
Appveyor is failing where the module spec tests require rspec-puppet-facts Repeating locally/manually; I get a bundle install failure for a generated module - I'm now trying to confirm if this is the same issue or something different |
|
@james-stocks the issue is that the windows puppet-module meta gems currently don't depend on |
lib/pdk/cli/test/unit.rb
Outdated
| @@ -20,20 +20,20 @@ module PDK::CLI | |||
| report = nil | |||
|
|
|||
| if opts[:list] | |||
| puts _('List of all available unit tests: (TODO)') | |||
| PDK::Test::Unit.list | |||
There was a problem hiding this comment.
I would recommend moving the output logic from PDK::Test::Unit.list and into here. If it just returns an array of test names and leaves it to the CLI to print them out for the user, it can be potentially be utilised in other places in the future (e.g. validating the contents of opts[:tests] before handing off to rspec).
lib/pdk/tests/unit.rb
Outdated
| @@ -1,6 +1,8 @@ | |||
| require 'pdk' | |||
| require 'pdk/cli/exec' | |||
| require 'pdk/util/bundler' | |||
| require 'rspec/core' | |||
There was a problem hiding this comment.
Does this need to be required?
lib/pdk/tests/unit.rb
Outdated
| def self.list | ||
| PDK::Util::Bundler.ensure_bundle! | ||
| PDK::Util::Bundler.ensure_binstubs!('rspec-core') | ||
| command_argv = [File.join(PDK::Util.module_root, 'bin', 'rspec'), '--dry-run', '-fjson'] |
There was a problem hiding this comment.
My preference is to use the more verbose forms of options in scripts (--format json instead of -fjson) as it makes the intentions clearer.
lib/pdk/tests/unit.rb
Outdated
| @@ -21,6 +23,32 @@ def self.invoke(tests, report = nil) | |||
| output = PDK::CLI::Exec.execute(cmd(tests)) | |||
| report.write(output) if report | |||
| end | |||
|
|
|||
| def self.list | |||
There was a problem hiding this comment.
A general stylistic suggestion for this method: Split the code up into logical paragraphs with a newline between them (e.g. paragraph 1 - bundler stuff, paragraph 2 - building the command array, paragraph 3 - executing the command, paragraph 4 - processing the output), makes it easier to grok a chunk at a time :)
spec/acceptance/test_spec.rb
Outdated
|
|
||
| describe command('pdk test unit --list') do | ||
| its(:exit_status) { is_expected.to eq 0 } | ||
| its(:stdout) { is_expected.to match(%r{Examples:.*demo_spec.rb\[1:1:1\].*demo_spec.rb\[1:2:1\].*demo_spec.rb\[1:3:1\]}m) } |
There was a problem hiding this comment.
Rather than using a big multiline regex, I'd suggest doing multiple matches on stdout
its(:stdout) { is_expected.to match(%r{^demo_spec\.rb\[1:1:1\]}) }
its(:stdout) { is_expected.to match(%r{^demo_spec\.rb\[1:2:1\]}) }
its(:stdout) { is_expected.to match(%r{^demo_spec\.rb\[1:3:1\]}) }There was a problem hiding this comment.
It's a single expectation that we match all three at the same time; though I guess multiple matches like you suggest is more readable
|
Rekicking appveyor after the new puppet-module-gems helped. |
james-stocks
force-pushed
the
SDK-148
branch
3 times, most recently
from
116e6e5 to
9bd0d71
Compare
|
@rodjek Have addressed your comments, thanks |
lib/pdk/tests/unit.rb
Outdated
| rspec_json_output = JSON.parse(output[:stdout]) | ||
| if rspec_json_output['examples'].empty? | ||
| rspec_message = rspec_json_output['messages'][0] | ||
| if rspec_message == 'No examples found.' # rubocop:disable Style/GuardClause |
There was a problem hiding this comment.
You can get rid of this rubocop disable by rewriting this conditional as a suffix to the return statement and letting it fall through to the raise if it doesn't match
return [] if rspec_message == 'No examples found.'
raise PDK::CLI::FatalError, _(...)
|
Rebased, and merged the test suite into the now existing |
|
@james-stocks feel free to merge, if you're satisfied with my changes. |
DavidS
changed the title
Implements the pdk test unit --list option that outputs a list of the module's spec tests