dataplat / dbachecks

✔ SQL Server Environmental Validation
https://dbachecks.readthedocs.io/en/latest/
MIT License
463 stars 143 forks source link

Network Latency Test Seems Innacurate #33

Closed gbargsley closed 6 years ago

gbargsley commented 6 years ago

I am not sure how the test is connecting, but we do not have the latency output by the test.

`Executing script C:\GitHub\dbachecks\checks\latency.Tests.ps1

Describing Testing network latency [-] network latency for PRDPRSDBS11A,5510 should be less than 40 ms 299ms Expected: {True} But was: {False} 7: $results.Average.TotalMilliseconds -lt $max | Should be $true at Invoke-LegacyAssertion, C:\Program Files\WindowsPowerShell\Modules\Pester\4.1.1\Functions\Assertions\Should.ps1: line 190 at , C:\GitHub\dbachecks\checks\latency.Tests.ps1: line 7`

SQLDBAWithABeard commented 6 years ago

What does (Test-DbaNetworkLatency -SqlInstance PRDPRSDBS11A,5510).Average.TotalMilliseconds return ?

ClaudioESSilva commented 6 years ago

This test just says if it is lower than the configured value or not. This can be another thing to discuss.

When we are comparing numbers/strings, should we check for boolean states? Or compare the values directly and in that case, we will have the output of the expected value (already known) VS the output?

SQLDBAWithABeard commented 6 years ago

We should not check for boolean - It is better to have

Actual value | Should Be Expected Value

than

(Actual Value -eq|-gt|-lt Expected Value) | Should Be $True

gbargsley commented 6 years ago

image

ClaudioESSilva commented 6 years ago

@gbargsley I have just change the test. Can you pick the latest version and test it again? This shows the before and the after image

gbargsley commented 6 years ago

I see the update.

image

SQLDBAWithABeard commented 6 years ago

Thats cool :-)

SQLDBAWithABeard commented 6 years ago

We should be using PRs now though please ;-)

potatoqualitee commented 6 years ago

yeah it should have prevented commits w/o a PR :( Thank you!

potatoqualitee commented 6 years ago

k please sync master and lemme know