Skip to content

Conversation

@anmenaga
Copy link

Some tests for New-WinEvent that bring code coverage of the class to 80%.


Context "New-WinEvent tests" {

It 'Simple New-WinEvent' -Skip:(-not $IsWindows) {
Copy link
Member

Choose a reason for hiding this comment

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

You may want to use $PSDefaultParameterValues to set the value of skip.

Copy link
Author

Choose a reason for hiding this comment

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

Yup, I saw that approach; but don't really like it; I think having skip conditions explicitly on each test makes it easier to investigate test failures.

Copy link
Member

Choose a reason for hiding this comment

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

The recommendation according to the testing guidelines is to use PSDefaultParameterValues. Please see: https://github.com/PowerShell/PowerShell/blob/master/docs/testing-guidelines/WritingPesterTests.md

Copy link
Author

Choose a reason for hiding this comment

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

Updated.

It 'Simple New-WinEvent' -Skip:(-not $IsWindows) {
New-WinEvent -ProviderName Microsoft-Windows-PowerShell -Id 40962 -Version 1 # simple event without any payload
$filter = @{ ProviderName = 'Microsoft-Windows-PowerShell'; Id = 40962}
(Get-WinEvent -filterHashtable $filter).Count -gt 0 | Should be $true
Copy link
Member

Choose a reason for hiding this comment

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

Should be,

(Get-WinEvent -filterHashtable $filter).Count | Should BeGreaterThan 0

Copy link
Author

Choose a reason for hiding this comment

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

Updated.

}

It 'No provider found error' -Skip:(-not $IsWindows) {
{ New-WinEvent -ProviderName NonExistingProvider -Id 0 } | ShouldBeErrorID 'System.ArgumentException,Microsoft.PowerShell.Commands.NewWinEventCommand'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Typo. Please change "ShouldBeErrorID" -> "ShouldBeErrorId".

Copy link
Author

Choose a reason for hiding this comment

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

Updated.


It 'PayloadMismatch error' -Skip:(-not $IsWindows) {
$logPath = join-path $TestDrive 'testlog1.txt'
New-WinEvent -ProviderName Microsoft-Windows-PowerShell -Id 32868 *> $logPath # this will print the warning with expected event template to the file
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please move the comment on separate line.

Copy link
Author

Choose a reason for hiding this comment

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

Moved.

}

It 'Simple New-WinEvent' {
New-WinEvent -ProviderName Microsoft-Windows-PowerShell -Id 40962 -Version 1 # simple event without any payload
Copy link
Member

Choose a reason for hiding this comment

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

Move the comment to a separate line.

Copy link
Author

Choose a reason for hiding this comment

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

updated.

Copy link
Member

@SteveL-MSFT SteveL-MSFT left a comment

Choose a reason for hiding this comment

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

Which issue does this address if any?

}

It 'Simple New-WinEvent' {
New-WinEvent -ProviderName Microsoft-Windows-PowerShell -Id 40962 -Version 1 # simple event without any payload
Copy link
Member

Choose a reason for hiding this comment

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

use something like

$PSProviderId = 40962

Copy link
Author

Choose a reason for hiding this comment

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

Updated.

@anmenaga
Copy link
Author

anmenaga commented Aug 1, 2017

This goes toward #4156

@SteveL-MSFT
Copy link
Member

Based on the code coverage, you're missing some tests:
https://codecov.io/gh/PowerShell/PowerShell/src/9e41c647a9407e05d86f4971a6bd25719a7130d0/src/Microsoft.PowerShell.CoreCLR.Eventing/DotNetCode/Eventing/EventDescriptor.cs

The Reader code should probably be a different PR

@anmenaga
Copy link
Author

anmenaga commented Aug 2, 2017

@SteveL-MSFT All of the things that you've mentioned is ETW code and will go into separate PR.
This PR is specific to New-WinEvent cmdlet code.

@adityapatwardhan
Copy link
Member

@iSazonov Can you have a look again?

}

It 'Simple New-WinEvent' {
# simple event without any payload
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe we can remove the comment. And maybe move to the test name.

Copy link
Member

Choose a reason for hiding this comment

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

@anmenaga can you address this?

Copy link
Author

Choose a reason for hiding this comment

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

Updated.

@iSazonov
Copy link
Collaborator

iSazonov commented Aug 4, 2017

With one minor comment LGTM.

@adityapatwardhan adityapatwardhan merged commit 0b7451b into PowerShell:master Aug 7, 2017
@anmenaga anmenaga deleted the NewWinEventTests branch October 31, 2018 21:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants