(PDK-917) Validates provider.get values against Type schema #88
Conversation
| Style/TrailingCommaInLiteral: | ||
| Description: Prefer always trailing comma on multiline literals. This makes diffs, | ||
| and re-ordering nicer. | ||
| EnforcedStyleForMultiline: comma |
| @@ -4,7 +4,7 @@ | |||
| require 'tempfile' | |||
|
|
|||
| RSpec.describe 'exercising simple_get_filter' do | |||
| let(:common_args) { '--verbose --debug --trace --modulepath spec/fixtures' } | |||
| let(:common_args) { '--verbose --debug --trace --strict=error --modulepath spec/fixtures' } | |||
There was a problem hiding this comment.
There is a space too much here (and everywhere you copy&pasted this)
lib/puppet/resource_api.rb
Outdated
| @@ -304,6 +290,8 @@ def call_provider(value); end | |||
| my_provider.get(context).find { |h| namevar_match?(h) } | |||
| end | |||
|
|
|||
| @warning_count = 0 # always reset when called | |||
There was a problem hiding this comment.
Don't reset here. After 100 warnings for a type, there is no additional info that devs can get from this, and we don't want to kill production if someone slips in hitting this.
| let(:prefix) { 'Warning:' } | ||
|
|
||
| it 'is raises the expected warnings' do | ||
| stdout_str, status = Open3.capture2e("puppet resource #{common_args} provider_validation") |
There was a problem hiding this comment.
You can re-use the expectations here by wrapping the describes in a loop like this:
[ ['warning', 'Warning: '], ['error', 'Error: Could not run:'], ...].each do |strict_value, prefix_value|
describe "when strict is set to #{strict_value}" do
let(:common_args) { super() + " --strict=#{strict_value}" }
let(:prefix) { prefix_value }
...
| end | ||
|
|
||
| describe '#check_schema_values' do | ||
| let(:definition) do |
There was a problem hiding this comment.
Any reason not to update the main :definition, instead of this and duplicating it in the next context?
| message = "Provider returned data that does not match the Type Schema for `#{name}[#{resource[namevars.first]}]`" | ||
|
|
||
| rejected_keys = check_schema_keys(resource) # removes bad keys | ||
| bad_values = check_schema_values(resource.dup) |
There was a problem hiding this comment.
Is check_schema allowed to modify resource or not? If yes, do you need to dup here? If not, shouldn't this dup before check_schema_keys?
Also, now that the question has come up, you might want to add a unit test making sure that resource doesn't get modified when checking.
No description provided.