pester / Pester

Pester is the ubiquitous test and mock framework for PowerShell.
https://pester.dev/
Other
3.11k stars 473 forks source link

Should we add tests that validate help? #648

Closed nohwnd closed 6 years ago

nohwnd commented 7 years ago

At the moment I am torn between liking the idea of being sure the help is there, and getting test failures for not having help. At the moment we have a group of tests for these non essential requirements. The whitespace tests.

When these are failing it has no impact on how Pester is working in PowerShell. It's only stylistic rules that should be kept in the repository. And the help falls in pretty much the same category, a non-functional requirement. It would be much better to have it as a some kind of warning that is raised on demand.

You can see the situation with the versioning tests, those should be excluded from the test base because they will fail for anyone who is not currently releasing the Pester version.

Imho we need something like test traits (XUnit) where we can specify a category of tests (or more generally a trait a test has) and build scenarios around that. (or you know just do it in place with IFs :D )

it "Should run during version release" -Traits @{Category = 'Version release'} {} 
it "Should validate help" -Traits @{Category='Version release'; Environment = 'Production'} {}
it "should find correct temp on linux" -Traits @{ Os = "Linux", Version = "6+" } {}

@it-praktyk let's discuss this here.

it-praktyk commented 7 years ago

The summary of current state of helps - for functions exported by module

The functions without 'Description'

The functions without described parameters (at least one missed)

The functions without any example

Results calculated based on the tests what you can find here - code equal with the pull request #647 .

dlwyatt commented 7 years ago

Many of those commands are not intended for public use, but they have to be exposed because of how Pester tests jump in and out of the Pester module's scope. (Examples: Invoke-Mock, Set-DynamicParameterValues, SafeGetCommand, Get-MockDynamicParameters.) For each of these, I added a very small comment-based help block that just says "pay no attention to the man behind the curtain."

Others are covered in about* topics and/or in the wiki, as they're not really traditional powershell commands, but more of a pseudo language element. (BeforeEach / AfterEach / BeforeAll / AfterAll)

We should definitely make sure we have coverage of all the parameters on Invoke-Pester, Setup, Set-TestInconclusive, and examples on New-PesterOption.

Should is a weird command that gets its own about_Should topic; so long as the comment-based help refers people to that, it's okay.

I didn't even realize Get-TestDriveItem existed, have never used it or updated its code. :)

it-praktyk commented 7 years ago

I would like update proposed tests based on your comments.

My initial proposal is like below.

# (Get-Command -Module pester ) -join ','

# AfterAll,AfterEach,Assert-MockCalled,Assert-VerifiableMocks,BeforeAll,BeforeEach,Context,
# Describe,Get-MockDynamicParameters,Get-TestDriveItem,In,InModuleScope,Invoke-Mock,
# Invoke-Pester,It,Mock,New-Fixture,New-PesterOption,SafeGetCommand, Set-DynamicParameterVariables,
# Set-TestInconclusive,Setup,Should

[String[]]$AcceptMissedHelpSynopsis = @('AfterAll','AfterEach', 'BeforeAll','BeforeEach' )

[String[]]$AccepteMissedHelpDyscription = @('AfterAll','AfterEach', 'BeforeAll','BeforeEach' )

[String[]]$AcceptMissedHelpExamples = @('Invoke-Mock', 'Set-DynamicParameterValues', 'SafeGetCommand', 'Get-MockDynamicParameters')

[String[]]$RequiredFilledHelpLink = @()

$FormulaLinksMapping = @{'AfterAll'='about_BeforeEach_AfterEach'}

Could you categorize functions based on your knowledge?

it-praktyk commented 7 years ago

The results for tests what were added as the pull request #652

Due to my mistake - Pester versions mismatch - described here #651 - the graphical report what is attached below is incorrect also.

pester-help-tests_-_2016-12-02_13 51 04

it-praktyk commented 7 years ago

I've pulled updated comment based help to Invoke-Pester #654

Updates for the next function do you prefare in the same pull request or in separated?

dlwyatt commented 7 years ago

Doesn't really matter, but separate PRs might allow us to merge some changes while others are waiting for edits.

it-praktyk commented 7 years ago

Another one question. If changes made in master branch will be marged/ported/pushed - I don't know what is the best word - to the branch DevelopmentV4 ?

I see e.g. in the DevelopmentV4 branch a description of parameter OutputXml (Invoke-Pester.psm1) but that parameter was removed.

it-praktyk commented 7 years ago

@dlwyatt

I didn't even realize Get-TestDriveItem existed, have never used it or updated its code. :)

The function was introduced in the pull request #70 .

I used Google to check if it is used and I didn't find nothing. I think that the Get-TestDriveItem function should be marked as deprecated in Pester v.4. I propose to create separate issue for that.

The similar function - but not exported - Get-TestDriveChildItem exists also. It is used in the Context function only.

CC: @jole78

jole78 commented 7 years ago

Haven't really used Pester in a while and if you think it can be removed that's fine for me. If it comes up again, the code is there to add it later on I guess.

it-praktyk commented 7 years ago

The related issue #555

it-praktyk commented 6 years ago

The current status except Gherkin-related - what is in separate issue #808.

Context The function [Set-DynamicParameterVariable] - Help
      [-] Description for the function is filled up 10ms
        Expected: value to not be empty
        67:                     $FunctionDescription | Should not BeNullOrEmpty
        at Invoke-LegacyAssertion, <FOLDER_PATH>\Pester\Functions\Assertions\Should.ps1: line 190
        at <ScriptBlock>, <FOLDER_PATH>\Pester\Help.Tests.ps1: line 67
      [-] The parameter [SessionState] contains description 21ms
        Expected: value to not be empty
        89:                         $ParameterDescription | Should not BeNullOrEmpty
        at Invoke-LegacyAssertion, <FOLDER_PATH>\Pester\Functions\Assertions\Should.ps1: line 190
        at <ScriptBlock>, <FOLDER_PATH>\Pester\Help.Tests.ps1: line 89
      [-] The parameter [Parameters] contains description 30ms
        Expected: value to not be empty
        89:                         $ParameterDescription | Should not BeNullOrEmpty
        at Invoke-LegacyAssertion, <FOLDER_PATH>\Pester\Functions\Assertions\Should.ps1: line 190
        at <ScriptBlock>, <FOLDER_PATH>\Pester\Help.Tests.ps1: line 89
      [-] The parameter [Metadata] contains description 32ms
        Expected: value to not be empty
        89:                         $ParameterDescription | Should not BeNullOrEmpty
        at Invoke-LegacyAssertion, <FOLDER_PATH>\Pester\Functions\Assertions\Should.ps1: line 190
        at <ScriptBlock>, <FOLDER_PATH>\Pester\Help.Tests.ps1: line 89
      [-] Example - At least one example exist 33ms
        Expected {0} to be greater than {0}
        103:                     $ExamplesCount | Should BeGreaterthan 0
        at Invoke-LegacyAssertion, <FOLDER_PATH>\Pester\Functions\Assertions\Should.ps1: line 190
        at <ScriptBlock>, <FOLDER_PATH>\Pester\Help.Tests.ps1: line 103
it-praktyk commented 6 years ago

The function Set-DynamicParameterVariable was named previously Set-DynamicParameterVariables and was excluded because is generally internal (as is stated in the function synopsis).

it-praktyk commented 6 years ago

Completed, #997