8000 add a timeout to test-connection by jpsnover · Pull Request #2492 · PowerShell/PowerShell · GitHub
[go: up one dir, main page]
Skip to content

add a timeout to test-connection #2492

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Feb 23, 2017
Merged

add a timeout to test-connection #2492

merged 1 commit into from
Feb 23, 2017

Conversation

jpsnover
Copy link
Contributor

This replaces PR #2479

@msftclas
Copy link

Hi @jpsnover, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!

This seems like a small (but important) contribution, so no Contribution License Agreement is required at this point. Real humans will now evaluate your PR.

TTYL, MSBOT;

@jpsnover jpsnover mentioned this pull request Oct 17, 2016
Closed
@JamesWTruher
Copy link
Collaborator

jeffrey and I spoke about my comments - the current timeout is 1000 ms, so this does not change our current behavior and provides for more flexibility

JamesWTruher
JamesWTruher reviewed Oct 17, 2016
View reviewed changes
src/Microsoft.PowerShell.Commands.Management/commands/management/Computer.cs
@@ -128,6 +128,14 @@ public class TestConnectionCommand : PSCmdlet
public Int32 BufferSize { get; set; } = 32;

/// <summary>
/// The following is the definition of the input parameter "BufferSize".
/// Buffer size sent with the this command. The default value is 32.
/// </summary>
Copy link
Collaborator

Choose a reason for hiding this comment

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

this comment does not seem to apply

@JamesWTruher
Copy link
Collaborator

I'd love to see additional tests here -

Describe "test-connection" -Tags @('Feature','Slow' ) {
     It "test-connection respects timeout" {
        $start = [datetime]::now
        Test-Connection -Computer _bad_computer_name_ -Timeout 10000 -ea silentlycontinue
        $end = [datetime]::now
        ($end - $start).TotalSeconds | should BeGreaterThan 10.0
     }
}

something along those lines

@andyleejordan
Copy link
Member
andyleejordan commented Oct 17, 2016 •
edited
Loading

Your committer email (private@jsnover.com) appears to be connected to the @jsnover GitHub account, and not the @jpsnover GitHub account you opened the PR from (so the commit doesn't "belong" to @jpsnover as far as GitHub is concerned). Do you mean to use this email for you commits, and if so, is it possible to verify it with your current @jpsnover GitHub account?

@jpsnover
Copy link
Contributor Author

@andschwa I've made it so hat private@jsnover.com is the only account connected to my jpsnover github account.
let me know if I need to do anything else

lzybkr
lzybkr reviewed Oct 20, 2016
View reviewed changes
src/Microsoft.PowerShell.Commands.Management/commands/management/Computer.cs
/// Time-out value in milliseconds. If a response is not received in this time, no response is assumed. The default is 1000 milliseconds.
/// </summary>
[Parameter]
[ValidateRange((int)1, Int32.MaxValue)]
Copy link
Contributor

Choose a reason for hiding this comment

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

I've never liked this use of ValidateRange - it seems like we should have ValidatePositive and ValidateNonNegative attributes.

@iSazonov iSazonov mentioned this pull request Nov 5, 2016
Closed
@lzybkr lzybkr merged commit e80fe74 into PowerShell:master Feb 23, 2017
@lzybkr lzybkr mentioned this pull request Feb 23, 2017
Closed
@lzybkr lzybkr mentioned this pull request Mar 23, 2017
Closed
@iSazonov iSazonov removed the Stale label Mar 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lzybkr lzybkr lzybkr left review comments

@JamesWTruher JamesWTruher JamesWTruher left review comments

Assignees

@JamesWTruher JamesWTruher

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
0