Add additional tests for Clear-Content to improve coverage#3157
Conversation
iSazonov
left a comment
There was a problem hiding this comment.
We now have mixed capitalization for cmdlet names, parameter names, "should be". It is difficult to read. We should fix this (use "Should Be", "Set-Content"...) and parameters cuts.
There was a problem hiding this comment.
In our tests, we have too many deviations from our guide for throw "No Exception!"
I suggest using the excellent function ShouldBeErrorId
(I have already prepared new PR #3161 to move existing tests to this pattern)
The new look of this test:
{
try {
Push-Location function:
Get-Command clear-content -stream $streamName
}
finally {
Pop-Location
}
} | ShouldBeErrorId "NamedParameterNotFound,Microsoft.PowerShell.Commands.GetCommandCommand"Only insert:
Import-Module $PSScriptRoot\..\..\Common\Test.Helpers.psm1There was a problem hiding this comment.
I have a bit of a problem with this. The functionality of checking the FullyQualifiedErrorId rather than the exception message should be in Pester rather than our environment. Putting this code in our own environment does not help the community, but only increases the burden on our code maintenance.
There was a problem hiding this comment.
@JamesWTruher @iSazonov Let's create an issue for this discussion. I don't think this issue should block this PR. Please reply back with the issue and let's have any further discussion on the issue thread.
There was a problem hiding this comment.
It would be nice to have at least a small synopsis of the function
Coverage has improved to more than 85% for Clear-Content
0b82783 to
fdf00b2
Compare
|
restarted Linux in TravisCI |
Coverage is reported to be more than 85% for Clear-Content
actual coverage for this is much higher (it looks like closing braces are not being counted as covered). I am following up with OpenCover folks.