GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Test fixes and code coverage automation fixes. by adityapatwardhan · Pull Request #5046 · PowerShell/PowerShell

Conversation

@adityapatwardhan

@adityapatwardhan Aditya Patwardhan (adityapatwardhan) commented Oct 7, 2017

Copy link
Copy Markdown
Member
  • 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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@adityapatwardhan
Aditya Patwardhan (adityapatwardhan) requested review from James Truher (JamesWTruher), Steve Lee (SteveL-MSFT) and Dongbo Wang (daxian-dbw) October 7, 2017 01:07
@adityapatwardhan
Aditya Patwardhan (adityapatwardhan) requested a review from Travis Plunk (TravisEz13) as a code owner October 7, 2017 01:07
SteveL-MSFT
Steve Lee (SteveL-MSFT) requested changes Oct 7, 2017
View reviewed changes
Comment thread test/powershell/engine/Api/ProxyCommand.Tests.ps1 Outdated

@SteveL-MSFT Steve Lee (SteveL-MSFT) Oct 7, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@adityapatwardhan Aditya Patwardhan (adityapatwardhan) Oct 9, 2017

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Fixed.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread test/tools/CodeCoverageAutomation/Start-CodeCoverageRun.ps1 Outdated

@SteveL-MSFT Steve Lee (SteveL-MSFT) Oct 7, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@iSazonov Ilya (iSazonov) Oct 7, 2017

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

And casing parameters too.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@adityapatwardhan Aditya Patwardhan (adityapatwardhan) Oct 9, 2017

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Fixed.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread test/tools/CodeCoverageAutomation/Start-CodeCoverageRun.ps1 Outdated

@SteveL-MSFT Steve Lee (SteveL-MSFT) Oct 7, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Search replace out-file with Out-File

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@iSazonov Ilya (iSazonov) Oct 7, 2017

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

And casing parameters too.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@adityapatwardhan Aditya Patwardhan (adityapatwardhan) Oct 9, 2017

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Fixed.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
iSazonov
Ilya (iSazonov) reviewed Oct 7, 2017
View reviewed changes

@iSazonov Ilya (iSazonov) left a comment

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Leave a comment

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1 Outdated

@iSazonov Ilya (iSazonov) Oct 7, 2017

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@adityapatwardhan Aditya Patwardhan (adityapatwardhan) Oct 9, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Ilya (@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'.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@iSazonov Ilya (iSazonov) Oct 9, 2017

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@adityapatwardhan Aditya Patwardhan (adityapatwardhan) Oct 9, 2017

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@iSazonov Ilya (iSazonov) Oct 10, 2017

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@adityapatwardhan Aditya Patwardhan (adityapatwardhan) Oct 10, 2017

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@adityapatwardhan Aditya Patwardhan (adityapatwardhan) Oct 10, 2017

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Fixed.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1 Outdated

@iSazonov Ilya (iSazonov) Oct 7, 2017

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@adityapatwardhan Aditya Patwardhan (adityapatwardhan) Oct 10, 2017

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@adityapatwardhan Aditya Patwardhan (adityapatwardhan) Oct 10, 2017

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Fixed.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1 Outdated

@iSazonov Ilya (iSazonov) Oct 7, 2017

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

The same - maybe Should BeGreaterThan 0.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@adityapatwardhan Aditya Patwardhan (adityapatwardhan) Oct 10, 2017

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Fixed.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@iSazonov Ilya (iSazonov) assigned Dongbo Wang (daxian-dbw) Oct 9, 2017
@adityapatwardhan

Aditya Patwardhan (adityapatwardhan) commented Oct 9, 2017

Copy link
Copy Markdown
Member Author

Steve Lee (@SteveL-MSFT) your feedback has been answered. Please have a look.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
SteveL-MSFT
Steve Lee (SteveL-MSFT) approved these changes Oct 9, 2017
View reviewed changes
@daxian-dbw

Dongbo Wang (daxian-dbw) commented Oct 10, 2017

Copy link
Copy Markdown
Member

Chatted with Aditya Patwardhan (@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. Aditya Patwardhan (@adityapatwardhan) will investigate further to see what might be the root cause.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@adityapatwardhan

Aditya Patwardhan (adityapatwardhan) commented Oct 10, 2017

Copy link
Copy Markdown
Member Author

Dongbo Wang (@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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@daxian-dbw

Dongbo Wang (daxian-dbw) commented Oct 10, 2017

Copy link
Copy Markdown
Member

Aditya Patwardhan (@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?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@adityapatwardhan

Aditya Patwardhan (adityapatwardhan) commented Oct 10, 2017

Copy link
Copy Markdown
Member Author

Dongbo Wang (@daxian-dbw) your feedback has been answered. Please have a look.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
daxian-dbw
Dongbo Wang (daxian-dbw) approved these changes Oct 10, 2017
View reviewed changes

@daxian-dbw Dongbo Wang (daxian-dbw) left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@adityapatwardhan

Aditya Patwardhan (adityapatwardhan) commented Oct 10, 2017

Copy link
Copy Markdown
Member Author

Dongbo Wang (@daxian-dbw) I added another test fix. Please have a look.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@adityapatwardhan

Aditya Patwardhan (adityapatwardhan) commented Oct 11, 2017

Copy link
Copy Markdown
Member Author

Dongbo Wang (@daxian-dbw) All tests have passed. Please have a look.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
daxian-dbw
Dongbo Wang (daxian-dbw) requested changes Oct 11, 2017
View reviewed changes
Comment thread test/powershell/engine/Remoting/SSHRemotingAPI.Tests.ps1 Outdated

@daxian-dbw Dongbo Wang (daxian-dbw) Oct 11, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@iSazonov Ilya (iSazonov) Oct 11, 2017

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Maybe

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@adityapatwardhan Aditya Patwardhan (adityapatwardhan) Oct 11, 2017

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Good point. Fixed.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@adityapatwardhan

Aditya Patwardhan (adityapatwardhan) commented Oct 11, 2017

Copy link
Copy Markdown
Member Author

Dongbo Wang (@daxian-dbw) I have updated the tests and all tests pass.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
daxian-dbw
Dongbo Wang (daxian-dbw) approved these changes Oct 12, 2017
View reviewed changes
@daxian-dbw

Dongbo Wang (daxian-dbw) commented Oct 12, 2017

Copy link
Copy Markdown
Member

Aditya Patwardhan (@adityapatwardhan) Can you please take a look at the test failures in Travis CI?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Aditya Patwardhan (adityapatwardhan) added 7 commits October 12, 2017 16:21
@adityapatwardhan
Test fixes and updates to code coverage automation
ed31099
@adityapatwardhan
Address CR comments
3bb19bf
@adityapatwardhan
[Feature] Address code review comments
8bd98e6
@adityapatwardhan
[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.
@adityapatwardhan
[Feature] Skip SSH test if ssh.exe is not present
f0a6693
@adityapatwardhan
[Feature] Added check for SSH
bc068e8
@adityapatwardhan
[Feature] Resetting the about_spla test
b5f23d9
@adityapatwardhan
Aditya Patwardhan (adityapatwardhan) force-pushed the testFixes branch from e548019 to b5f23d9 Compare October 12, 2017 23:21
daxian-dbw
Dongbo Wang (daxian-dbw) approved these changes Oct 13, 2017
View reviewed changes
@daxian-dbw
Change 'ssh.exe' to 'ssh' in comment
080089f
@adityapatwardhan

Aditya Patwardhan (adityapatwardhan) commented Oct 13, 2017

Copy link
Copy Markdown
Member Author

Dongbo Wang (@daxian-dbw) Thanks for the update.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@adityapatwardhan

Aditya Patwardhan (adityapatwardhan) commented Oct 13, 2017

Copy link
Copy Markdown
Member Author

Dongbo Wang (@daxian-dbw) All tests passed. Ready for merge.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@daxian-dbw
Dongbo Wang (daxian-dbw) merged commit 486fb97 into PowerShell:master Oct 13, 2017
@adityapatwardhan
Aditya Patwardhan (adityapatwardhan) deleted the testFixes branch October 13, 2017 22:32
Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj-contribs/PowerShell that referenced this pull request Aug 6, 2026
@adityapatwardhan @daxian-dbw
Test fixes and code coverage automation fixes. (PowerShell#5046)
67d7d5b 
* Test fixes and updates to code coverage automation
* Fixed UpdatableHelpSystem tests and tab completion tests
* Fixed tab completion tests to better disambiguate root/interop namespace.
* Skip SSH test if ssh.exe is not present
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@iSazonov Ilya (iSazonov) iSazonov left review comments
@daxian-dbw Dongbo Wang (daxian-dbw) daxian-dbw approved these changes
@SteveL-MSFT Steve Lee (SteveL-MSFT) SteveL-MSFT approved these changes
@JamesWTruher James Truher (JamesWTruher) Awaiting requested review from JamesWTruher
@TravisEz13 Travis Plunk (TravisEz13) Awaiting requested review from TravisEz13

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@daxian-dbw Dongbo Wang (daxian-dbw)

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@adityapatwardhan @daxian-dbw @SteveL-MSFT @iSazonov
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.