-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
resolves #66 #67
resolves #66 #67
Conversation
Signed-off-by: Will <[email protected]>
@@ -40,7 +40,7 @@ | |||
end | |||
end | |||
else |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doing some more research, it's expecting that if something is passed in that's not a hash, then it's a file path.
the ini one basically is just building off of the json one: https://github.com/inspec/inspec/blob/main/lib/inspec/resources/ini.rb#L5
the constructor for the json one calls out to a wrapper parse function: https://github.com/inspec/inspec/blob/main/lib/inspec/resources/json.rb#L37
the check to see if it's a hash otherwise it treats it like a path which is the codepath we're currently following: https://github.com/inspec/inspec/blob/main/lib/inspec/resources/json.rb#L37
the codepath we actually want where we treat the input as just a raw file: https://github.com/inspec/inspec/blob/main/lib/inspec/resources/json.rb#L89
…tructor Signed-off-by: Amndeep Singh Mann <[email protected]>
Signed-off-by: Amndeep Singh Mann <[email protected]>
…cement - just returns an empty string that obv isn't in the array of expected values Signed-off-by: Amndeep Singh Mann <[email protected]>
@@ -40,8 +40,7 @@ | |||
end | |||
end | |||
else | |||
describe ini(network_manager) do | |||
its('main.dns') { should exist } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wdower you're probably gonna wanna review my changes before you merge this in. imo it looks good now. |
Resolves #66