Comments (19)
@csandfeld Yes, please work on adding Inconclusive count back. But leave out Pending.
from pester.
Thanks @fflaten, appreciate the tip. That is also the (temporary?) workaround I had in mind. But as you mention it is not very elegant, and I was not 100% sure if it would be a reliable way of doing it.
I also use the "because" message in my reporting, and was surprised to see that additional data is injected in that message. It is a little counter-intuitive that the test author no longer has full control of what is in that message.
As a user of the product I would prefer the "because" message to remain unaltered, and Pending
and Inconclusive
- or an alternative custom author specified reasoning to be surfaced in another attribute.
from pester.
Not sure if this address your concerns @fflaten, but here is an example with the changes implemented in PR #2401.
I do notice missing is skipped, because ... output for 'c' and 'd', I'll take a look at that as well.
$r = invoke-pester -container ( new-pestercontainer -ScriptBlock {
Describe "a" {
It 'a' {
$true | Should -BeTrue
}
It 'b' {
$true | Should -BeFalse
}
It 'c' {
Set-ItResult -Inconclusive -Because 'demo inconclusive'
}
It 'd' {
Set-ItResult -Pending -Because 'demo pending'
}
It 'e' {
Set-ItResult -Skipped -Because 'demo skipped'
}
}
}) -passThru -Output Detailed
# output
Starting discovery in 1 files.
Discovery found 5 tests in 16ms.
Running tests.
Describing a
[+] a 18ms (3ms|15ms)
[-] b 9ms (7ms|2ms)
Expected $false, but got $true.
at $true | Should -BeFalse, :9
at <ScriptBlock>, <No file>:9
[!] c 7ms (4ms|2ms)
[!] d 6ms (4ms|2ms)
[!] e is skipped, because demo skipped 12ms (9ms|2ms)
Tests completed in 229ms
Tests Passed: 1, Failed: 1, Skipped: 3 NotRun: 0
$r | Select-Object -Property Result, *Count
# output
Result : Failed
FailedCount : 1
FailedBlocksCount : 0
FailedContainersCount : 0
PassedCount : 1
SkippedCount : 3
NotRunCount : 0
TotalCount : 5
$r.Tests | ForEach-Object {
[pscustomobject]@{
Name = $_.Name
Result = $_.Result
ShouldRun = $_.ShouldRun
ErrorId = $_.ErrorRecord.FullyQualifiedErrorId
}
}
# output
Name : a
Result : Passed
ShouldRun : True
ErrorId :
Name : b
Result : Failed
ShouldRun : True
ErrorId : PesterAssertionFailed
Name : c
Result : Skipped
ShouldRun : True
ErrorId : PesterTestInconclusive
Name : d
Result : Skipped
ShouldRun : True
ErrorId : PesterTestPending
Name : e
Result : Skipped
ShouldRun : True
ErrorId : PesterTestSkipped
from pester.
I never used Pending in either Pester 4 or tried to use it in Pester 5. I do use Skip a lot, for example when having PowerShell modules that is used cross-platform. Some code does not work on (mostly) Linux and macOS so tests testing that code are skipped when on incompatible platform. But I thought about if there were a scenario when Pending could be used and I could only think of one, if some test code are not finished then Pending would be better than Skip, but then why merge an unfinished test, what would the purpose be for that? I would not trust an unfinished test so I wouldn't merge it. I have not needed Pending in the work I do in the years I written Pester tests, not yet anyway. Would be curious in what scenarios Pending have been used.
from pester.
I would keep it as the -Pending parameter on Set-ItResult, and remove it in v6.
👍 Maybe add a deprecation notice at the end of the run if we detect it's been used.
from pester.
You can still identify the tests skipped using Set-ItResult -Inconclusive
by checking the errorrecord message stored in the test-object.
Either by using a reason as keyword or by searching for PENDING
or INCONCLUSIVE
which is added to the message by Set-ItResult
when not using -Skipped
. Not saying it's elegant, so just suggesting it as a workaround 🙂
$conf = New-PesterConfiguration
$conf.Output.Verbosity = 'Detailed'
$conf.Run.ScriptBlock = {
Describe 'a1' {
Context 'b1' {
It 'i1' {
Set-ItResult -Inconclusive -Because 'demo'
}
It 'i2' -Skip {
1 | Should -Be 1
}
}
}
}
$conf.Run.PassThru = $true
$run = Invoke-Pester -Configuration $conf
$run.Skipped | ? { $_.ErrorRecord.FullyQualifiedErrorId -eq 'PesterTestSkipped' -and $_.ErrorRecord -cmatch 'INCONCLUSIVE' } | ft ExpandedPath, Result, ErrorRecord
# output
ExpandedPath Result ErrorRecord
------------ ------ -----------
a1.b1.i1 Skipped {is skipped, because INCONCLUSIVE: demo}
from pester.
I looked at the implementation of Set-ItResult and I see that we set 'PesterTestSkipped' as the error id for any of the skip reasons.
throw [Pester.Factory]::CreateErrorRecord(
'PesterTestSkipped', #string errorId
$Message, #string message
$File, #string file
$Line, #string line
$LineText, #string lineText
$false #bool terminating
)
There are few handlers that check for this in the upper scopes and set the test results to skipped, rather than failed.
e.g. the one that still has the remnants of inconclusive support:
Pester/src/functions/Output.ps1
Line 268 in a50354e
I think if you added the switch that used to be there in Set-ItResult, so it throws different error id for inconclusive, and then handled both error ids in thosre remaining handlersr to translate to skipped (literally just adding -or or -contains), it would be a very easy change to make (hopefully :) ), and then the info also flows all the way up to your result, so we can handle the summaries at our own pace, and you still get a non-brittle way to get this info.
import-module s:/p/pester/bin/pester.psd1 -force
$r = invoke-pester -container ( new-pestercontainer -ScriptBlock {
Describe "a" {
It "b" {
Set-ItResult -Inconclusive -Because 'demo'
}
}
}) -passThru
$r.Tests[0].ErrorRecord[0].FullyQualifiedErrorId
# output
Starting discovery in 1 files.
Discovery found 1 tests in 6ms.
Running tests.
[+]
Describe "a" {
It "b" {
Set-ItResult -Inconclusive -Because 'demo'
}
}
61ms (2ms|54ms)
Tests completed in 67ms
Tests Passed: 0, Failed: 0, Skipped: 1 NotRun: 0
PesterTestSkipped
from pester.
Thanks @nohwnd - I can try to tackle that :)
from pester.
I think if you added the switch that used to be there in Set-ItResult, so it throws different error id for inconclusive, and then handled both error ids in thosre remaining handlersr to translate to skipped (literally just adding -or or -contains), it would be a very easy change to make (hopefully :) ), and then the info also flows all the way up to your result, so we can handle the summaries at our own pace, and you still get a non-brittle way to get this info.
We'd likely have to update Add-RSpecTestObjectProperties
also to mark them as Skipped for now. If not they'll probably get NotRun as Result in the PassThru/run object.
I'm unable to verify until the weekend.
from pester.
Skipped message output fixed in latest commit (2e73e47) added to PR #2401.
$r = invoke-pester -container ( new-pestercontainer -ScriptBlock {
Describe "a" {
It 'a' {
$true | Should -BeTrue
}
It 'b' {
$true | Should -BeFalse
}
It 'c' {
Set-ItResult -Inconclusive -Because 'demo inconclusive'
}
It 'd' {
Set-ItResult -Pending -Because 'demo pending'
}
It 'e' {
Set-ItResult -Skipped -Because 'demo skipped'
}
}
}) -passThru -Output Detailed
# output
Pester v5.5.0
Starting discovery in 1 files.
Discovery found 5 tests in 147ms.
Running tests.
Describing a
[+] a 67ms (38ms|29ms)
[-] b 41ms (37ms|4ms)
Expected $false, but got $true.
at $true | Should -BeFalse, :9
at <ScriptBlock>, <No file>:9
[!] c is skipped, because INCONCLUSIVE: demo inconclusive 18ms (15ms|3ms)
[!] d is skipped, because PENDING: demo pending 5ms (3ms|2ms)
[!] e is skipped, because demo skipped 14ms (7ms|7ms)
Tests completed in 701ms
Tests Passed: 1, Failed: 1, Skipped: 3 NotRun: 0
from pester.
I also use the "because" message in my reporting, and was surprised to see that additional data is injected in that message. It is a little counter-intuitive that the test author no longer has full control of what is in that message.
💯
from pester.
@nohwnd, @fflaten - thank you for merging #2401 👍
I have looked into what changes are needed to bring back PendingCount and InconclusiveCount (plus Pending and Inconclusive generic lists) properties, and think I have a decent understanding of it by now.
Would you approve of me working on adding that?
If yes, are there any issues I should be aware of that kept them out of v5?
from pester.
Since it's listed here I assume it was just pending some discussion. @nohwnd ?
Inclusive makes sense imo.
As for Pending, do we need it (yet)? Especially Set-ItResult -Pending
is confusing to me. When would we use it?
Pending feels like a initial result for tests with ShouldRun=$true Executed=$False
. If so, is it necessary at all in the summary and Set-ItResult
? Or is it mostly useful once we start exposing the test state during runtime.
from pester.
I'm wondering if we really need Pending. Isn't it the same as
Executed=$False
? Any obvious use cases it would solve?
The use case for Pending is likely the same as the -Pending switch on It:
Line 30 in acc66a9
-Pending [<SwitchParameter>]
Use this parameter to explicitly mark the test as work-in-progress/not implemented/pending when you
need to distinguish a test that fails because it is not finished yet from a tests
that fail as a result of changes being made in the code base. An empty test, that is a
test that contains nothing except whitespace or comments is marked as Pending by default.
You could argue that if It already has a -Pending switch, there would be no need for Set-ItResult
to support it. But the Set-ItResult
has the switch, it is just currently not hooked up to anything but the ErrorId
change implemented in #2401. And if we opt to reimplement Inconclusive, it looks simple to add Pending as well while we are at it.
Another argument for reimplementing both, is a degree of increasedcbackwards compatibility.
from pester.
Fair point. That's what I get from using mobile app to comment (didn't check the code) 😄
Still, the result name is ambiguous and the same function could easily be achieved using Skip
with a reason or comment. So does it warrant being a first-class result?
Just want to raise the discussion now considering it's easier to leave it out than removing later if it feels like clutter.
from pester.
Just want to raise the discussion now considering it's easier to leave it out than removing later if it feels like clutter.
I agree
from pester.
from pester.
Pending was an idea I had, and briefly found useful only to never use it. So imho killing it off is the best. I would keep it as the -Pending parameter on Set-ItResult, and remove it in v6. It is supposed to be a temporary marker for tests in progress, so removing it should not be a big problem.
from pester.
@csandfeld Yes, please work on adding Inconclusive count back. But leave out Pending.
@nohwnd, my proposed implementation is in #2405. At your convenience please have a looked at it, and let me know if you see a need for additional changes etc.
from pester.
Related Issues (20)
- Export Should-Invoke and Should-HaveParameter
- Output should result to Assert assertions
- Add soft assertions to new assertions
- Expose GlobalPluginData to all plugin steps
- Infinite loop for `FileInfo` and `DirectoryInfo` due to `Root` properties in `DirectoryInfo`. Should we restrict depth as a general fix in addition to specialcasing `DirectoryInfo` in `Get-DisplayProperty2`? HOT 1
- Measure Code Coverage for Pester
- `Get-Help Should-Be` will still show `Assert-Be`. We'll also need to modify `generate-command-reference.ps1` in docs-repo to rename files and title to match alias. HOT 1
- replace `Get-EquivalencyOption` with parameters HOT 2
- Inconsistent behavior mocking external command between output verbosity 'Detailed' and 'Diagnostic' HOT 3
- Calling mock of Get-ChildItem with -File parameter fails on Linux HOT 1
- Does Pester support to view the code coverage report in browser? HOT 1
- Ability to specify the path to the location of the actual problem in the test result HOT 2
- Missing Description in new Assert assertions HOT 1
- Remove or update included about_ help files HOT 2
- Update Invoke-Pester help
- Cleanup legacy parameters in Invoke-Pester with Pester 6?
- Update SECURITY.md and SUPPORT.md? HOT 1
- Option to disable old assertions HOT 7
- Add trimWhitespace to Should-beString
- Add description to all should-* assertions and remove this todo HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from pester.