(MODULES-10384) - Registry value check tightened

[david22swan]

No description provided.

[david22swan]

[codecov-io]

Codecov Report

Merging #343 into master will not change coverage.
The diff coverage is 0%.

@@          Coverage Diff           @@
##           master    #343   +/-   ##
======================================
  Coverage    56.2%   56.2%           
======================================
  Files          20      20           
  Lines         758     758           
======================================
  Hits          426     426           
  Misses        332     332

Impacted Files	Coverage Δ
lib/puppet_x/sqlserver/features.rb	30.68% <0%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d371737...d3fd861. Read the comment docs.

[sanfrancrisko]

I can't see any unit tests for this method or any other methods that calls it in the module, so I guess the question comes down to whether we want to add one?

Given it's essentially just reporting the values from the registry, I'm not sure I'd see much additional benefit, however I am a big proponent of adding tests for scenarios that caused bugs. Might be worth looking at how much time it would take to write a test for get_instance_info or get_instance_features? If it's going to take a lot of mocking / stubbing, then probably not worth it.

Code change LGTM

[david22swan]

I can't see any unit tests for this method or any other methods that calls it in the module, so I guess the question comes down to whether we want to add one?

Given it's essentially just reporting the values from the registry, I'm not sure I'd see much additional benefit, however I am a big proponent of adding tests for scenarios that caused bugs. Might be worth looking at how much time it would take to write a test for get_instance_info or get_instance_features? If it's going to take a lot of mocking / stubbing, then probably not worth it.

Code change LGTM

@cmccrisken-puppet Talked to Schmidt and come to the decision that no test is needed right now.