Skipping test from inside the It block

[michalporeba]

Resolves #1022

at the moment all but 5 unit tests pass, all five in "Describing Pester manifest and changelog". What do I need to do to make them pass?

[michalporeba]

OK, so the tests that failed locally, those relating to changlog and versioning passed in travis, but publish to GitHub failed in TeamCity. I cannot see why (no access to TC) so I will need some help.

Few things remaining potentially for discussion, that could be included in this PR quite easily are

1. Changing Set-TestInconclusive for it to internally use the new Set-ItResult
2. Adding a deprecation message to Set-TestInconclusive
3. Implementing Set-ItResult -Pending to explicitly set It result to pending from inside the block. I don't know if there is much value for it, there isn't for me, but I thought I'll bring it up as it would be easy to do it, and may make sense for 'completeness' of the solution

Any thought? Any recommended changes to the current implementation? I will welcome any reviews. Can anybody from the Pester crew comment on how do you see it? How likely is this PR to make it into Pester soonish? @nohwnd, anybody else? I'm working on big changes to dbachecks which I want to push out of the door this month, and if only possible, I'd like to use this new functionality already.

Thanks your your time and help (and I do appreciate there is PSConfEU coming up very soon, so many people are rather busy).

[nohwnd]

no access to TC

Just click login as guest, the build is public. It's not very obvious sorry.

1, 2. yeah that would be nice, Set-TestInconclusive should be deprecated in that case, and we need to offer a strategy for replacement.
3. Yes please, it only makes sense to add the function if it supports the complete set of states.

I am sorry but I cannot review your code at the moment, I am also preparing for the psconfeu and need to finish my talks that before comitting to someting else. In general the changes in this repo are merged rather slowly, especially if they concern public apis.

Are you attending the conf?:)

[michalporeba]

OK, I'll work something in for 1,2 and 3. I realise a lot of people are on the conf, but not me. I've just got back home. And anyway, I'm new to the whole powershell thing, so I only have just learnt about the conference 2 weeks ago ;) I'll see, if I learn some powershell this year, perhaps I'll come next year ;)

…

On 15 April 2018 at 14:06, Jakub Jareš ***@***.***> wrote: no access to TC Just click login as guest, the build is public. It's not very obvious sorry. 1, 2. yeah that would be nice, Set-TestInconclusive should be deprecated in that case, and we need to offer a strategy for replacement. 3. Yes please, it only makes sense to add the function if it supports the complete set of states. I am sorry but I cannot review your code at the moment, I am also preparing for the psconfeu and need to finish my talks that before comitting to someting else. In general the changes in this repo are merged rather slowly, especially if they concern public apis. Are you attending the conf?:) — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1026 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAHYMAAc6hziKqt5IVrPC5LYgaCL5FOcks5to0XQgaJpZM4TUcvK> .

[michalporeba]

The build error in TC appears to be to do with build configuration so somebody with the full access will have to investigate it.

[michalporeba]

There is nothing like a useful error message

turns out it wasn't build error after all. I used -in which is not supported in PowerShell v2

[michalporeba]

OK, so now I guess I will need some help with that v2. \Functions\Assertions\Set-TestInconclusive.Tests.ps1 is failing, complains the mock function was not called. Any ideas?

[michalporeba]

Can anybody help with making that powershell 2 test pass?
Also if somebody could review the code I'd be eager to make any adjustments over the weekend

[it-praktyk]

I tried to reproduce.

Tests under CI are run (as I understand) under Set-StrictcMode -Version Latest.

I've run tests for the branch https://github.com/michalporeba/Pester/tree/skip under
`
Name Value

---

CLRVersion 2.0.50727.5420
BuildVersion 6.1.7601.17514
PSVersion 2.0
WSManStackVersion 2.0
PSCompatibleVersions {1.0, 2.0}
SerializationVersion 1.1.0.1
PSRemotingProtocolVersion 2.1
`

The received results you can find below. It's too early/late for me to debug it deeply. @michalporeba can you look if can it be caused by your changes?

`
Set-ExecutionPolicy -Version Latest
Invoke-Pester
<OUTPUT_PARTIALLY_OMITTED>
Executing script C:\Users\Administrator\Pester\Functions\Mock.Tests.ps1

<OUTPUT_PARTIALLY_OMITTED>

Describing Mocking Cmdlets with dynamic parameters in a module
[-] Allows calls to be made with dynamic parameters (including parameter filters) 71ms
Expected Get-ChildItem in module TestModule to be called at least 1 times but was called 0 times
1260: Assert-MockCalled Get-ChildItem -ModuleName TestModule
at line: 1260 in

<OUTPUT_PARTIALLY_OMITTED>

Executing script C:\Users\Administrator\Pester\Pester.Tests.ps1

Describing Pester manifest and changelog
[+] has a valid manifest 80ms
[+] has a valid name in the manifest 13ms
[+] has a valid guid in the manifest 14ms
[!] is tagged with a valid version is skipped without an explanation 27ms
consider adding -Because parameter to the Set-ItResult call
[-] has valid release notes in the manifest 27ms
Expected strings to be the same, but they were different.
Expected length: 46
Actual length: 51
Strings differ at index 46.
Expected: 'https://github.com/pester/Pester/releases/tag/'
But was: 'https://github.com/pester/Pester/releases/tag/4.3.1'
---------------------------------------------------------^
50: $script:manifest.PrivateData.PSData.ReleaseNotes | Should -Be "https://github.com/pester/Pester/releas
es/tag/$script:tagVersion"
at line: 50 in C:\Users\Administrator\Pester\Pester.Tests.ps1
[+] has valid pre-release suffix in manifest (empty for stable version) 55ms
[-] tag and changelog versions are the same 19ms
Expected $null, but got '4.3.1'.
71: $script:changelogVersion | Should -Be $script:tagVersion
at line: 71 in C:\Users\Administrator\Pester\Pester.Tests.ps1
[-] tag and changelog versions are the same 36ms
Expected $null, but got '4.3.1'.
76: $script:changelogVersion | Should -Be $script:tagVersion
at line: 76 in C:\Users\Administrator\Pester\Pester.Tests.ps1
[!] all short versions are the same is skipped without an explanation 19ms
consider adding -Because parameter to the Set-ItResult call

<OUTPUT_PARTIALLY_OMITTED>

Tests completed in 40.96s
Tests Passed: 716, Failed: 4, Skipped: 2, Pending: 0, Inconclusive: 0
`

[michalporeba]

Thank you @it-praktyk. Those errors you see happen locally as, but they are only to do with the release process (CI) and they pass in TravicCI and TeamCity.

The failing test can be found in the TeamCity if you log in as guest

The problem is in only one of unit tests, not a very important one too. It would be easy to simply remove it, but I'd like to understand and fix it. I could probably remove it just to see if the build will pass without it.

[michalporeba]

Thank you @nohwnd. I didn't realise you can filter like that on Assert-MockCalled. That's cool.
Could you explain the changes? You moved the mock inside It and you moved the parameter filtering from the mock to assertion. Why would one (or both) of them be needed to fix it? I'm just trying to understand things better.

[nohwnd]

@michalporeba I don't think either was needed to fix it, I was just trying it out so I don't have to spin up a powershell 2 machine. :)

My reasoning is this:

• Mock should be defined as close to the usage as possible, the less shared things there are among tests the better.

• Catching all calls and then counting the calls you wanted is easier to comprehend (and safer) than only mocking the calls that you expect and then counting them.

I don't think skipping the deprecation test is way to go forward. There is something wrong with that test, so I will have to bite the bullet and install v2 somewhere :D

[michalporeba]

@nohwnd I don't know what has changed but suddenly both tests show as passing. Can this be merged, please?

[nohwnd]

@michalporeba thanks for the PR and sorry about the long silence I was taking a little break. I like the changes, but I don't like adding the explanation to the result 'is skipped without an explanation, consider adding -Because parameter to the Set-ItResult call', that will get old really fast, and sometimes you don't want to provide a reason (maybe because you are skipping the test programatically). The because parameter is optional so it should not be forcing the user to provide the reason.

[fourpastmidnight]

OK, so the tests that failed locally, those relating to changlog and versioning passed in travis, but publish to GitHub failed in TeamCity. I cannot see why (no access to TC) so I will need some help.

Few things remaining potentially for discussion, that could be included in this PR quite easily are

1. Changing Set-TestInconclusive for it to internally use the new Set-ItResult
2. Adding a deprecation message to Set-TestInconclusive
3. Implementing Set-ItResult -Pending to explicitly set It result to pending from inside the block. I don't know if there is much value for it, there isn't for me, but I thought I'll bring it up as it would be easy to do it, and may make sense for 'completeness' of the solution

Any thought? Any recommended changes to the current implementation? I will welcome any reviews. Can anybody from the Pester crew comment on how do you see it? How likely is this PR to make it into Pester soonish? @nohwnd, anybody else? I'm working on big changes to dbachecks which I want to push out of the door this month, and if only possible, I'd like to use this new functionality already.

Thanks your your time and help (and I do appreciate there is PSConfEU coming up very soon, so many people are rather busy).

I actually came across this today, and I submitted a PR #1141 to fix this.