(SDK-296) Allow target selection for the metadata validator #124
Conversation
| @@ -70,4 +70,71 @@ | |||
| end | |||
| end | |||
| end | |||
|
|
|||
| context 'when validating a specific file' do | |||
| include_context 'in a new module', 'foo' | |||
There was a problem hiding this comment.
Due to SDK-294 (an intermittent bug when re-using the foo folder on Windows) it's preferable to give each feature its own test module name instead of always using 'foo'
lib/pdk/validators/base_validator.rb
Outdated
| c.context = :module | ||
| c.add_spinner(spinner_text) | ||
| end | ||
| targets = (self::INVOKE_STYLE == :per_target) ? targets.combination(1).to_a : Array[targets] |
There was a problem hiding this comment.
I needed to try this out in IRB for myself to understand what it's doing: the former case gives an array of targets; the latter case wraps the targets in an 'outer array' that means targets.each will return the targets array as a single value.
Is the the combination(1).to_a actually needed?
There was a problem hiding this comment.
Yeah, this is to try and avoid duplicating the logic below when handling the different invoke styles. If :per_target, we take the array of targets and using Array#combination, split it into a 2 dimensional array, where each element inside targets is itself an array of 1 target.
Given
targets = [
'target1',
'target2',
'target3',
]If the INVOKE_STYLE is :per_target, the targets array becomes [['target1'], ['target2'], ['target3']]. If the INVOKE_STYLE is :once, the targets array becomes [['target1', 'target2', 'target3']].
By doing it this way, we ensure that targets will always be an array of arrays and just loop through it and avoid having to put a conditional around the execution logic to determine if how many times the validated should be invoked and with which targets.
lib/pdk/validators/base_validator.rb
Outdated
| @@ -4,6 +4,8 @@ | |||
| module PDK | |||
| module Validate | |||
| class BaseValidator | |||
| INVOKE_STYLE = :once | |||
There was a problem hiding this comment.
I'm unsure if the intention of :once is immediately understandable - might :combined or :all_targets be more immediately understandable?
Or leave it as :once but add some commenting here on the possible values of INVOKE_STYLE
There was a problem hiding this comment.
👍 That constant definitely needs some documentation.
| @@ -4,6 +4,14 @@ | |||
| module PDK | |||
| module Validate | |||
| class BaseValidator | |||
| # Controls how many times the validator is invoked. | |||
In order to accommodate this functionality, I needed to refactor the base validator a bit so that we can optionally invoke the validator once per target (as metadata-json-lint only supports testing a single file).