Skip to content
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

Bugfix: make registry_key resource case-insensitive #255

Merged
merged 4 commits into from Nov 24, 2015

Conversation

alexpop
Copy link
Contributor

@alexpop alexpop commented Nov 23, 2015

Add integration test for NtlmMinClientSec

Once #254 is fixed, we should add a test for NTLMMinServerSec

@chris-rock
Copy link
Contributor

Thanks for adding this @alexpop

@chris-rock chris-rock changed the title add NTLMMinServerSec test Bugfix: make registry_key resource case-insensitive Nov 23, 2015
@chris-rock
Copy link
Contributor

@arlimus added fix to this PR

# convert keys to lower case
return @registy_cache = Hash[@registy_cache.map do |key, value|
[key.downcase, value]
end]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we:

begin
  keys = JSON.parse(cmd.stdout)
  @registry_cache = ... blah ...
rescue JSON::ParseError => _
  @registry_cache = nil
end

@registry_cache

This also avoids a duplicate return statement.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@arlimus
Copy link
Contributor

arlimus commented Nov 24, 2015

Thank you @alexpop , this is an awesome addition! Thanks @chris-rock for extending our tests.

arlimus added a commit that referenced this pull request Nov 24, 2015
Bugfix: make registry_key resource case-insensitive
@arlimus arlimus merged commit 138c8ee into master Nov 24, 2015
@arlimus arlimus deleted the alexpop/more_registry_key_tests branch November 24, 2015 15:34
@chris-rock chris-rock added the Type: Bug Feature not working as expected label Nov 24, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Feature not working as expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants