(SDK-277) Exit cleanly if pdk commands are run outside of a module #100
Conversation
lib/pdk/cli/validate.rb
Outdated
| if PDK::Util.module_root.nil? | ||
| PDK.logger.error(_('pdk validate must be run inside a module')) | ||
| exit 1 | ||
| end |
There was a problem hiding this comment.
- Please extract that to
PDK::CLI::Util.check_in_module!and apply liberally to all the commands that should run in a module:
- pdk new class
- pdk test unit
- pdk validate
- add
(no metadata.json found)to ensure users know what we're missing.
spec/acceptance/validate_spec.rb
Outdated
| require 'fileutils' | ||
|
|
||
| describe 'Running validation' do | ||
| context 'outside of a module' do |
There was a problem hiding this comment.
Once we have a uniform behaviour above, you can extract this (and https://github.com/puppetlabs/pdk/blob/master/spec/acceptance/new_class_spec.rb#L92) into a shared examples, so this becomes it_behaves_like 'requires running in a module directory'.
|
Good point, updated and deduped the tests. The shared example now get automagically included for all example groups with |
| @@ -1,6 +1,6 @@ | |||
| require 'spec_helper_acceptance' | |||
|
|
|||
| describe 'Running ruby validation' do | |||
| describe 'pdk validate ruby', module_command: true do | |||
There was a problem hiding this comment.
Is it generally understood by contributors how the shared_examples is being included here? I needed to read rspec-core source to understand what module_command does. It would be more readable to me if it used it_behaves_like 'a module command' to link to the shared example spec code; but I'm fairly new to rspec and shared_example metadata might be a generally understood concept?
There was a problem hiding this comment.
It is described in the main docs of rspec-core: https://relishapp.com/rspec/rspec-core/docs/example-groups/shared-context#declare-a-shared-context-and-include-it-with-metadata
There was a problem hiding this comment.
Apologies, I looked at that document and totally overlooked that section.
I still feel this method is a little bit 'magic', but if it's right there in the rspec docs then I think it's OK.
There was a problem hiding this comment.
Agreed on the magic thing. And I can't say I'm blown away by the rspec docs either :-( It's definitely something we can keep an eye on, and easy enough to change our minds afterwards.
The main advantage is in my view that it allows for external composeability, that is, a new shared context can add itself to module_commands without requiring changes across the entire test suite. Admittedly that is pretty weak sauce, and also YAGNI.
There was a problem hiding this comment.
Yeah, it's a very poorly documented but still powerful part of rspec
DavidS
changed the title
No description provided.