Open alx9r opened 6 years ago
The reasoning here is that people rarely care about the type of the collection they pass in, rather they care about the data, and I want to keep the assertions consistent and useful, even though there are some tradeoffs to be made. With Should
we learned that for this assertion:
1, 2 | Assert-Equal 1
People favor seeing Expected 1 but got collection 1, 2.
, instead of Expected 1 but got 2.
. To do that I need to collect the complete input and assert on it.
With types it is a bit more difficult, because there you are asserting on the internal details of how I collect the input. To get correct result you would need to circumvent the pipeline and do:
Assert-Type -Actual 1,2 -Expected ([Array])
Or is there a way to get the original collection after the pipeline without anything special being done by the user?
Another reason is that assertions over collections can have multiple requirements, sometimes you look for Any item to pass, sometimes you look for all items to pass, sometimes you want the items to be ordered but you don't really care about the data etc. That's why I am trying to make one set of assertions that will deal with "values" and another one that will deal with collections.
tl;dr I think that most new PowerShell users are surprised by how the pipeline works. The reason for that seems to be deficient courseware. Should
already caters to those users. I thought Assert
was for advanced users.
With Should we learned that for this assertion: 1, 2 | Assert-Equal 1 People favor seeing Expected 1 but got collection 1, 2.
It seems to me that what we learned from Should
is that users unfamiliar with PowerShell's pipeline are surprised by PowerShell's pipeline behavior. I certainly was surprised when I was starting out. That surprise occurred in many varied cases aside from Should
. In retrospect for me, it seems like this was caused by PowerShell training materials that were prepare by instructors who had a great deal of C# experience and very little PowerShell experience. C# does not have pipeline. PowerShell is not C#.
When I was starting out I expected
1,2 | Assert-Equal 1
to fail. "Of course 1,2
isn't the same as 1
," is what I thought. But that was because I had neither learned what |
does nor how PowerShell's pipeline works. So I was basing my reading of most PowerShell statements on an incorrect understanding. PowerShell was extremely painful during that time. At one point I re-watched all 11 courses offered through my video training subscription thinking I had missed something. I hadn't. There was (and is to this day) no part of that learning path (or any other PowerShell learning path I have found) that provides even a remotely adequate primer of PowerShell's pipeline. Yet most PowerShell statements involve the pipeline in one form or another. After much language-introspective testing and many stackoverflow questions I eventually figured out how the pipeline works for myself. Now when I read
1,2 | Assert-Equal 1
I expect it to pass for the first record, and fail for the second record. It's apparent that 1,2
is an array which is enumerable so the individual elements 1
and 2
are piped as separate records into Assert-Equal
. Of course Assert-Equal
could accumulate 1
and 2
but that would be weird given its name. If that's what it's doing, I would expect it to be called Assert-ElementsEqual
or something more elaborate. The simplicity of the name Assert-Equal
evokes simplicity of behavior.
I get that there is popular demand for a testing framework that works in the the way expected by people who haven't learned how PowerShell's pipeline works. I would have preferred that when I was starting out. Thankfully for me though, Should
at the time behaved in the customary PowerShell way. So I was forced even by Pester to learn how the pipeline works. Since Pester 4.x Should
introduced some unusual pipeline behavior. I'm fine with that. Pester is ubiquitous so Should
should probably behave the way most people expect it to.
I suppose I interpreted the words "advanced" and "simplify" in "A set of advanced assertions for Pester to simplify how you write tests" to mean that Assert
would embrace the PowerShell pipeline. It seems to me that building unusual pipeline behavior into assertion primitives like Assert-Equal
is contrary to both "advanced" and "simplify."
I feel like we might be miscommunicating about the matter of collections and enumerations and assertions. Here's how I'm thinking about this:
[XmlElement]
enumerates nodes, [ConfigInstructions]
enumerates steps that depend on the outcome of previous steps).[string]
,[hashtable]
,[SortedList]
).I never meant this module to be for advanced users only, the opposite actually. The purpose is to simplify the assertions to behave consistently and predictably. Which is something the current Should assertions in Pester don't do.
I think you are reading too much in the collections and enumerators, most people are dealing with arrays and lists 90% of the time, and they expect it to show the whole collection in the assertion output even thought they pass it through pipeline. So the a collection in this sense is something that will be enumerated when passed through pipleline, or won't be wrapped when you do @(
That people don't understand pipeline might be true, but there is not much I can do about it. And I still like writing 1,2,3 | Assert-Equal "a"
better than Assert-Equal 1,2,3 "a"
.
I was looking at the code the whole day, and I can't really decide how to progress. I would like to merge some of the features with Pester, (especially the formatting), but I am not sure what it will break, and also not sure how to keep it modularized and still use the functions in Pester. :/
Or is there a way to get the original collection after the pipeline without anything special being done by the user?
Per this comment the answer seems to be "no". 😢
For posterity, Pester/Pester#386 is one real-world example of the confusion caused by the pipeline's conditional enumeration while unit testing.
@alx9r yeah you are right, now the question is what would be the best way forward. Would getting rid of pipeline completely help to eradicate those problems and make the syntax coherent and less surprising? At what cost?
...now the question is what would be the best way forward.
Since this debate came up I have been experimenting with an assertion library that assuages this issue (and a number of other pathologies). I expect I'll publish the library at some point. That might be a way forward.
Would getting rid of pipeline completely help to eradicate those problems...
AFAICT avoiding all kinds of unchecked conditional enumeration is critical to meaningful unit tests in PowerShell. It turns out there are a number of conditions where that occurs. |
is the obvious case. Operators like -eq
(see also Pester/Pester#984) and the .
member access operator (see PowerShell/PowerShell#5832) are others.
... and make the syntax coherent and less surprising?
Here's how I see the lay of the land:
$obj.Misspelt | Should -beNullOrEmpty
) and misleading test failure messages (eg. $arrayYouThoughtWasScalar.a | Should -be 1
) abound.Misspelt
and a
members)Should
. I expect this is true even for readers unfamiliar with the library. This is mainly because the verbosity of the equivalent tests written using Should
explodes around the numerous explicit checks mentioned in (2).Should
. This is mainly because there is much less repetition than the equivalent tests written using Should
.Should
.In other words, authors able to learn can benefit from an improved assertion library that leads to coherent test code with fewer surprises.
Authors unable to learn are probably best served by Pester's Should
despite its attendant surprises. I see no practical way to improve Should
to avert surprises. I also see no way to implement less surprising general-purpose assertions that are as easy to use for the uninitiated as Should
.
FWIW, here is an example of test code written against my experimental library:
Describe Example {
$arrayYouThoughWasScalar = @(
[pscustomobject]@{a=1}
[pscustomobject]@{a=1}
)
It 'Verbose style' {
New-TestSubject $arrayYouThoughWasScalar |
Select-InnerObject (Property a) {
$_ | Assert-That -ItIs -EqualTo 1
}
}
It 'Terse style' {
subject $arrayYouThoughWasScalar |
pick (prop a) {
$_ | assert -eq 1
}
}
}
The output is as follows:
This is a lot of very useful information. To me it seems that the most strict solution to avoid the problems would be to reject using the pipeline and differentiate between scalar and "enumerable" assertions. This way input parameters can be rejected (as I do in some cases already) so that:
for scalar assertion providing enumerable to Expected will throw argument exception suggesting you should use some of the enumerable assertions
for scalar assertion providing enumerable to Actual will fail the assertion, because enumerable cannot be equal to scalar
enumerable assertions will implement operations like All, Any etc. and provide a predicate. the predicate can either return $false or throw so we can reuse our assertions without providing another set of Test-* functions
One big annoyance is that negative numbers become strings when passed as parameters:
function a ($a) {
$a.gettype().Name
}
a 1 # int
a -1 # string
a (-1)
One way to solve this is trying to cast the value to decimal, but then we should have an option to disable that casting. Or possibly a parameter set can be implemented that defines a well typed parameter (-ExpectedNumber
The thing is that I don't want to get rid of the pipeline input because it is just so convenient. And without it I doubt anyone will ever use this libary, no matter how safe and awesome it is :)
So aditionally we could:
In your code example, you are doing pretty much what I envisioned this library to do, except the names are different, and I don't get what the subject
does differently than if we just gave the value directly to Select-InnerObject via parameter.
Coming from: https://github.com/pester/Pester/issues/386#issuecomment-364197081
I agree that it is difficult to detect what the user meant if you only look to the input, but it gets more clear if you also look to what the user expects it Should -Be
.
e.g.: if I test:
$Something | Should -BeOfType [System.Data.DataTable]
I apparently expect something to be a DataTable Type and if it is not, it would be valuable if I get a hint that I e.g. should add an unary comma operator in front of the input object.
In other words, I should (only) get a hint (to add unary comma operator) if the following conditions are true:
-BeOfType
is any of the types that gets automatically unrolled, e.g. [ArrayList],
[DataTable], [XmlElement], [ConfigInstructions], etc. (which might be programmically recognizable somehow). The hint will then only be unnecessarily displayed if there are (multiple) objects in the pipeline tested against any of the specific "automatically unrolling" types:
In examples like below, I will probably get an unnecessarily hint:
$XmlElement | Should -BeOfType [System.Data.DataTable]
$Object1, $Object2 | Should -BeOfType [System.Data.DataTable] #(Where $Object1 -isNot [System.Data.DataTable])
The thing is that I don't want to get rid of the pipeline input because it is just so convenient.
Putting |
between the test subject and the assertion means (categorically, it seems) that the assertion cannot know exactly what the original object was. With that as a starting point there will always be surprising results. I think you can probably mitigate some of the most common surprises using some of the tactics suggested above, but each such mitigation I experimented with created another different, perhaps less likely, usually more surprising, sometimes more pernicious, behavior. Usually this boils down to replacing one problem with another.
And without it I doubt anyone will ever use this libary, no matter how safe and awesome it is :)
It seems you and I have different priorities. My goal is to build an assertion library that is robust to PowerShell's rich behavior. Some of us desperately need such a library, and one currently does not exist. Of course I think it would be good if others can benefit from such a library, but I've decided not to compromise that goal in pursuit of popularity. I found that to be an easy decision because Should
already strikes the compromise between the ease of use and robustness necessary to achieve popularity.
I don't get what the subject does differently than if we just gave the value directly to Select-InnerObject via parameter.
In practice it's unusual to use subject
because the test subject is usually the results of invoking the command under test. A test fixture is necessary to gather results of invoking the command under test and the fixture's output is piped straight into assert
or pick
without ever involving subject
. Without such a test fixture you end up repeating boilerplate in your test code to test for common things like the following:
This is just a small sampling; there are many other benefits to using a test fixture for command invocation.
Even when no command invocations are involved, the use of subject
results from parameter binding tradeoffs. Once you account for all the ways that assert
and Select-InnerObject
ought to be composable, their parameters become a very crowded place. Moving the test subject to the pipeline reduces that crowding in a manner that has a hope of being intuitive. There are a lot of tradeoffs at play there.
By "enumerating types" I mean any type for which
[LanguagePrimitive]::GetEnumerator()
exists orIsUnrolledByPipeline
is true.Consider the following tests:
Many of these results are surprising to me:
Assert-Type
were customary PowerShell, it would be checking the type of each element of the array. I get that users who haven't yet learned what|
really does might be surprised if this test failed. But such users are going to experience a lot of pain until they learn that anyway. It seems to me that promoting tools that hide PowerShell's default pipeline behavior just prolongs that painful learning period.[int]
.It strikes me that there are two distinct intentions a user might have for such enumerating types:
And there are two corresponding things the assertions could do:
a. Test the enumerating object. b. Test the elements the enumerator emits.
Customary PowerShell involving
|
would lead to (a) regardless of the user's intentions. It seems like that would be good behavior for all users for the following reasons:Assert-
is what you'd expect from any other customary advanced function.It seems like (b) should only be done when
|
is involved when the user explicitly expresses their intentions as such. I see the following reasons for this explicitness requirement:Assert
more maintainable because intention (1) is clearly distinguished from intention (2).I can imagine a few ways to implement
Assert
's commands in a manner that clearly couples (1) to (a) and (2) to (b). But I'm wondering if there is a different line of thinking I'm missing for the wayAssert
currently handles enumerating types. How are tests involving enumerating types meant to be written?