SqlServerLogin: Addresses issue #1096 - doesn't check properties if login is not present on server

[jpomfret]

Pull Request (PR) description

Addresses issue #1096 where when a login doesn't exist there are null values in the verbose output from the test function by skipping that output if the login is absent and expected to be present.

This Pull Request (PR) fixes the following issues

Fixes #1096

Task list

• Added an entry under the Unreleased section of the change log in the CHANGELOG.md.
  Entry should say what was changed, and how that affects users (if applicable).
• Resource documentation added/updated in README.md.
• Resource parameter descriptions added/updated in README.md, schema.mof
  and comment-based help.
• Comment-based help added/updated.
• Localization strings added/updated in all localization files as appropriate.
• Examples appropriately added/updated.
• Unit tests added/updated. See DSC Resource Testing Guidelines.
• Integration tests added/updated (where possible). See DSC Resource Testing Guidelines.
• New/changed code adheres to DSC Resource Style Guidelines and Best Practices.

---

This change is 

[msftclas]

All CLA requirements met.

[codecov-io]

Codecov Report

Merging #1240 into dev will decrease coverage by <1%.
The diff coverage is 100%.

@@         Coverage Diff          @@
##            dev   #1240   +/-   ##
====================================
- Coverage    98%     98%   -1%     
====================================
  Files        34      34           
  Lines      4171    4171           
====================================
- Hits       4113    4111    -2     
- Misses       58      60    +2

[johlju]

@jpomfret Thank you for sending this in. Will be great to get this issue resolved!

Reviewed 2 of 2 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @jpomfret)

a discussion (no related file):
Could you please add a regression unit test for this? A unit test that make sure one of the New-VerboseMessage is asserted to be called 0 times (Assert-MockCalled).
This test should fail with the previous code, but pass with the new code.
https://github.com/PowerShell/SqlServerDsc/blob/dev/Tests/Unit/MSFT_SqlServerLogin.Tests.ps1

Let me know if you need any pointers for writing the unit test.

---

---

DSCResources/MSFT_SqlServerLogin/MSFT_SqlServerLogin.psm1, line 384 at r1 (raw file):

 -and $($loginInfo.Ensure) -ne 'Absent'

Could we make it -and $loginInfo.Ensure -eq 'Present' instead and have the same functionality? Easier to read?

[jpomfret]

Reviewable status: 1 of 3 files reviewed, 2 unresolved discussions (waiting on @johlju and @jpomfret)

a discussion (no related file):

Previously, johlju (Johan Ljunggren) wrote…

Could you please add a regression unit test for this? A unit test that make sure one of the New-VerboseMessage is asserted to be called 0 times (Assert-MockCalled).
This test should fail with the previous code, but pass with the new code.
https://github.com/PowerShell/SqlServerDsc/blob/dev/Tests/Unit/MSFT_SqlServerLogin.Tests.ps1

Let me know if you need any pointers for writing the unit test.

Done.

---

[jpomfret]

Reviewable status: 1 of 3 files reviewed, 2 unresolved discussions (waiting on @johlju)

---

DSCResources/MSFT_SqlServerLogin/MSFT_SqlServerLogin.psm1, line 384 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

 -and $($loginInfo.Ensure) -ne 'Absent'

Could we make it -and $loginInfo.Ensure -eq 'Present' instead and have the same functionality? Easier to read?

Done.

[johlju]

Reviewed 2 of 2 files at r2.
Reviewable status: complete! all files reviewed, all discussions resolved

[johlju]

After rebase and moved the change log entry (because of the latest release).

Reviewed 1 of 1 files at r4.
Reviewable status: complete! all files reviewed, all discussions resolved

[johlju]

Closing and reopening this PR to kick off the tests again.

[johlju]

After fixing the tests failing by another PR.

Reviewed 1 of 1 files at r6.
Reviewable status: complete! all files reviewed, all discussions resolved