Skip to content

Remove errors in counter tests on non-Windows#2914

Closed
iSazonov wants to merge 2 commits into
PowerShell:masterfrom
iSazonov:countertests
Closed

Remove errors in counter tests on non-Windows#2914
iSazonov wants to merge 2 commits into
PowerShell:masterfrom
iSazonov:countertests

Conversation

@iSazonov
Copy link
Copy Markdown
Collaborator

@iSazonov iSazonov commented Dec 21, 2016

Errors in Travis CI log is Get-ItemProperty : Cannot find a provider with the name 'Registry'.
(Get-Counter, Import-Counter and Export-Counter is only Windows cmdlets.)

@iSazonov iSazonov force-pushed the countertests branch 5 times, most recently from 984c536 to 30df1a8 Compare December 21, 2016 11:41
Errors is Get-ItemProperty : Cannot find a provider with the name
'Registry'.
@iSazonov iSazonov changed the title Skip counter tests on non-Windows Remove errors in counter tests on non-Windows Dec 21, 2016
@PowerShellTeam PowerShellTeam added the Review - Needed The PR is being reviewed label Dec 21, 2016
@lzybkr
Copy link
Copy Markdown
Contributor

lzybkr commented Dec 21, 2016

The changes look fine.

I thought I would make more general comments on these tests (I think @Francisco-Gamino is the original author).

I think the tests would be a bit cleaner if the test used BeforeAll and set the default parameter for It -Skip in BeforeAll. The test for skipping can be simpler as well, in most cases, it's just !$IsWindows which is much faster than calling a function.

The last thing I noticed - the ValidateParameters function shouldn't exist, the -TestsCases parameter to It should be used instead. This will also be faster to skip on non-Windows platforms as It will be called once instead of once per testcase.

@iSazonov
Copy link
Copy Markdown
Collaborator Author

@lzybkr Thanks for comments.
I made some performance optimizations (using $isWindows and set -MaxSamples 2). The savings amounted to about 4 seconds.
With regard to the "ValidateParameters function shouldn't exist" I believe that this is not acceptable because there is a request from @JamesWTruher #2887 - we must explicitly skip every test to retain the correct stats.

@iSazonov
Copy link
Copy Markdown
Collaborator Author

iSazonov commented Jan 6, 2017

I close the PR since others (#2958 and #2952) have already addressed these changes.

@iSazonov iSazonov closed this Jan 6, 2017
@iSazonov iSazonov deleted the countertests branch January 17, 2017 11:09
@iSazonov iSazonov removed the Review - Needed The PR is being reviewed label Mar 30, 2017
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.

4 participants