Write-cupestertests: Take into considerations improvement remarks

[Stephanevg]

So I won't forget to do this:

Okay I am looking at this: powershelldistrict.com/write-cupester… I am gonna list all the stuff I'd do differently, hope it does not end up sounding overly critical :)

1. Use the new should syntax everywhere and make sure your casing is consistent. (so Should -Be instead of should Be)

2. The names of the tests contain a lot of extra characters that make them very hard to understand, I'd reduce the cognitive overhead by for example putting the class name into describe, and then only wrote what is being tested in the It name.

3. the test names don't say what is the expected result, I would prefer 'can be constructed'/'can be instantiated' over '.... - should not throw'

4. I'd consider putting the construction of the class in before all, and reused the instance in other places, that way if you have a constructor that needs parameters you don't have to specify them in every test

It will also fail the whole block if the class can't be constructed. Which might be a plus, especially in Pester v5 where correctly shows the amount of failed tests even if the whole block failed.

5. instead of comparing property type by the name of the type, you should use -HasType ([string]). if you don't go with the shared instance, I'd write those tests based on the type itself, and not based on $instance.GetType()

6. I am not fan of the extra comments at starts and ends of blocks, I am (usually) not confused about where blocks start and end, so I don't need that overhead. When I am confused I let the editor to show me via bracket colorizing plugin or something like that. You might be different, but it's a personal preference that I would not want a framework to force into my code :)

7. I'd work on the formatting, I like the # -- Assert annotations, but there is way too much whitespace around them.

Also add extra space in {} when you do Should -Throw, it makes the code easier to read.

8. Last but not least. Eradicate Should -Not -Throw, behind every such assertion there is an actual useful assertion waiting.

You know what parameters the method has, and which types it returns. So try to provide a default value to parameters, and a non-default value to assertions to force the test to fail. Templating this for a method that takes one string param and has a return type of string:

$c = [Class1]::New()
[String] $name = $null #TODO: DEFINE PARAMETER Name
$c.Method($name) | Should -Be "ABC" #TODO: DEFINE EXPECTED STRING

Seems way more useful than templating that the method Should -Not -Throw. Further more, the method does not throw by default, so your tests pass by default.

You might also consider leveraging powers of some other plugins, like a TODO plugin that highlights stuff that the user needs to specify.

---

Okay that is it, hope it won't discourage you from continuing on this project :) In case you need help figuring some of the stuff out let me know, I know reflection quite well. Also I am not sure if you know about it, but the title case / camel case methods in text info should be quite useful for taking more complicated names and changing them to uppercase/lowercase to follow powershell coding conventions dotnetperls.com/totitlecase