Test fixes and code coverage automation fixes. by adityapatwardhan · Pull Request #5046 · PowerShell/PowerShell

adityapatwardhan · 2017-10-07T01:07:23Z

About help test needed Update-Help
Tab completion tests were too stringent. There can be more than 1 matches.
Get-Process test should use $PID.
Improvements to Start-CodeCoverage script with respect to clean up and progress preference.
Removed code for uploading data to coveralls.
Added testexe path to $env:PATH

SteveL-MSFT · 2017-10-07T02:27:21Z

This is almost exactly the same as above, perhaps a helper function instead?

SteveL-MSFT · 2017-10-07T02:30:41Z

Since you're modifying this, might as well fix the casing Remove-Item

And casing parameters too.

SteveL-MSFT · 2017-10-07T02:30:58Z

Search replace out-file with Out-File

And casing parameters too.

iSazonov

Leave a comment

iSazonov · 2017-10-07T06:47:27Z

Maybe Should BeGreaterThan 0 - else next line can throw with useless message.

@iSazonov Can you elaborate. I want to check it for being exactly 1. If the check fails, the next line will not be executed and the It will be marked as 'Failed'.

If $res.CompletionMatches.Count = 0 then $res.CompletionMatches[0] will throw with generic message.

But if $res.CompletionMatches.Count = 2 and we are using Should BeGreaterThan 0 then the test might pass but we are getting more matches than expected.

I wonder now - why are we removing this line with Should BeExactly 1 if we want just 1 ?

Correct, I confused this with the about_help test. This can be BeGreaterThan 0.

iSazonov · 2017-10-07T06:47:49Z

Maybe Should BeGreaterThan 0 - else next line can throw with useless message.

Correct, I confused this with the about_help test. This can be BeGreaterThan 0.

iSazonov · 2017-10-07T06:48:41Z

The same - maybe Should BeGreaterThan 0.

adityapatwardhan · 2017-10-09T17:02:06Z

@SteveL-MSFT your feedback has been answered. Please have a look.

daxian-dbw · 2017-10-10T01:04:27Z

Chatted with @adityapatwardhan offline and we found it strange that the Should complete about help topic test would fail, because we carry help content files in the package, and we pull it down from nuget package in build. So there shouldn't be a need to call update-help. @adityapatwardhan will investigate further to see what might be the root cause.

adityapatwardhan · 2017-10-10T21:57:34Z

@daxian-dbw The updateable help system tests blow away the help and do not restore them. https://github.com/PowerShell/PowerShell/blob/master/test/powershell/engine/Help/UpdatableHelpSystem.Tests.ps1#L180

daxian-dbw · 2017-10-10T22:08:13Z

@adityapatwardhan Thanks for finding the root cause! It looks to me that that test shouldn't remove all about*.txt files, as it's testing module help contents and it shouldn't be a problem for about*.txt files to be present. Right?

adityapatwardhan · 2017-10-10T23:11:01Z

@daxian-dbw your feedback has been answered. Please have a look.

daxian-dbw

LGTM

adityapatwardhan · 2017-10-10T23:25:12Z

@daxian-dbw I added another test fix. Please have a look.

adityapatwardhan · 2017-10-11T00:08:35Z

@daxian-dbw All tests have passed. Please have a look.

daxian-dbw · 2017-10-11T00:57:58Z

It will skip the tests on Unix plats. Is that intentional?

Maybe

$skipTest = (Get-Command 'ssh' -ErrorAction SilentlyContinue) -eq $null

Good point. Fixed.

adityapatwardhan · 2017-10-11T17:38:10Z

@daxian-dbw I have updated the tests and all tests pass.

daxian-dbw · 2017-10-12T18:31:43Z

@adityapatwardhan Can you please take a look at the test failures in Travis CI?

* The updatable help system tests had not need to remove about help files. * Fixed tab completion tests to better disambiguate root/interop namespace. * Moved back the about_splatting test and removed update-help.

adityapatwardhan · 2017-10-13T17:02:04Z

@daxian-dbw Thanks for the update.

adityapatwardhan · 2017-10-13T17:35:08Z

@daxian-dbw All tests passed. Ready for merge.

adityapatwardhan requested review from JamesWTruher, SteveL-MSFT and daxian-dbw October 7, 2017 01:07

adityapatwardhan requested a review from TravisEz13 as a code owner October 7, 2017 01:07

SteveL-MSFT requested changes Oct 7, 2017

View reviewed changes

iSazonov reviewed Oct 7, 2017

View reviewed changes

iSazonov assigned daxian-dbw Oct 9, 2017

SteveL-MSFT approved these changes Oct 9, 2017

View reviewed changes

daxian-dbw approved these changes Oct 10, 2017

View reviewed changes

daxian-dbw requested changes Oct 11, 2017

View reviewed changes

daxian-dbw approved these changes Oct 12, 2017

View reviewed changes

adityapatwardhan added 7 commits October 12, 2017 16:21

Test fixes and updates to code coverage automation

ed31099

Address CR comments

3bb19bf

[Feature] Address code review comments

8bd98e6

[Feature] Fixed UpdatableHelpSystem tests and tab completion tests

4f26945

* The updatable help system tests had not need to remove about help files. * Fixed tab completion tests to better disambiguate root/interop namespace. * Moved back the about_splatting test and removed update-help.

[Feature] Skip SSH test if ssh.exe is not present

f0a6693

[Feature] Added check for SSH

bc068e8

[Feature] Resetting the about_spla test

b5f23d9

adityapatwardhan force-pushed the testFixes branch from e548019 to b5f23d9 Compare October 12, 2017 23:21

daxian-dbw approved these changes Oct 13, 2017

View reviewed changes

Change 'ssh.exe' to 'ssh' in comment

080089f

daxian-dbw merged commit 486fb97 into PowerShell:master Oct 13, 2017

adityapatwardhan deleted the testFixes branch October 13, 2017 22:32

Conversation

adityapatwardhan commented Oct 7, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iSazonov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adityapatwardhan Oct 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adityapatwardhan commented Oct 9, 2017

Uh oh!

daxian-dbw commented Oct 10, 2017

Uh oh!

adityapatwardhan commented Oct 10, 2017

Uh oh!

daxian-dbw commented Oct 10, 2017

Uh oh!

adityapatwardhan commented Oct 10, 2017

Uh oh!

daxian-dbw left a comment

Choose a reason for hiding this comment

Uh oh!

adityapatwardhan commented Oct 10, 2017

Uh oh!

adityapatwardhan commented Oct 11, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adityapatwardhan commented Oct 11, 2017

Uh oh!

daxian-dbw commented Oct 12, 2017

Uh oh!

adityapatwardhan commented Oct 13, 2017

Uh oh!

adityapatwardhan commented Oct 13, 2017

Uh oh!

Reviewers

adityapatwardhan Oct 9, 2017 •

edited

Loading