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.

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

New agent job history checks #582

Merged
merged 21 commits into from Jan 19, 2019

Conversation

Projects
None yet
2 participants
@ClaudioESSilva
Copy link
Collaborator

ClaudioESSilva commented Nov 30, 2018

A New PR

THANK YOU - We love to get PR's and really appreciate your time and help to improve this module

Accepting a PR

Before we accept the PR - please confirm that you have run the tests locally to avoid our automated build and release process failing. You can see how to do that in our wiki

https://github.com/sqlcollaborative/dbachecks/wiki

Please confirm you have 0 failing Pester Tests

[x] There are 0 failing Pester tests

Changes this PR brings

Checks for Maximum job history size (in rows) and Maximum job history rows per job

image

@SQLDBAWithABeard please take a look on the way Assert is done and let me know if it's ok or if we need to change.

@ClaudioESSilva ClaudioESSilva self-assigned this Nov 30, 2018

@ClaudioESSilva ClaudioESSilva requested a review from SQLDBAWithABeard Nov 30, 2018

@SQLDBAWithABeard
Copy link
Collaborator

SQLDBAWithABeard left a comment

Thank you @ClaudioESSilva That is awesome. there are a few minor changes required to descriptions and we need the unit tests for the assertions

Show resolved Hide resolved internal/configurations/DbcCheckDescriptions.json Outdated
Show resolved Hide resolved checks/Agent.Tests.ps1 Outdated
Show resolved Hide resolved checks/Agent.Tests.ps1 Outdated
Show resolved Hide resolved checks/Agent.Tests.ps1 Outdated
Show resolved Hide resolved internal/assertions/Agent.Assertions.ps1 Outdated
Show resolved Hide resolved internal/assertions/Agent.Assertions.ps1 Outdated
Show resolved Hide resolved internal/assertions/Agent.Assertions.ps1 Outdated
@@ -1,5 +1,5 @@
# load all of the assertion functions
. /../internal/assertions/Database.Assertions.ps1
(Get-ChildItem $PSScriptRoot/../../internal/assertions/).ForEach{. $Psitem.FullName}

This comment has been minimized.

@SQLDBAWithABeard

SQLDBAWithABeard Nov 30, 2018

Collaborator

Why do we need to change this?
We should only be loading the required functions for this file.

This comment has been minimized.

@ClaudioESSilva

ClaudioESSilva Nov 30, 2018

Author Collaborator

reverted. You are right. I have seen the other being done this way and because some error I had before I have changed it.
See https://github.com/sqlcollaborative/dbachecks/blob/development/tests/checks/InstanceChecks.Tests.ps1#L2

@ClaudioESSilva

This comment has been minimized.

Copy link
Collaborator Author

ClaudioESSilva commented Nov 30, 2018

let me know if the unit tests done this way (especially the Mocks) are ok.
thanks!

@SQLDBAWithABeard

This comment has been minimized.

Copy link
Collaborator

SQLDBAWithABeard commented Dec 26, 2018

image
please can you resolve the unexpected } @ClaudioESSilva

@ClaudioESSilva

This comment has been minimized.

Copy link
Collaborator Author

ClaudioESSilva commented Dec 26, 2018

Done.
I wonder how the tests have passed... 😞

Wrong comparison
Changed = to -eq

@SQLDBAWithABeard SQLDBAWithABeard merged commit 0655dc4 into development Jan 19, 2019

1 check passed

Development - Unit Testing #359 succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment