Stephanevg / PSClassUtils

A set of utilities to work with Powershell Classes
http://powershelldistrict.com/how-to-generate-a-uml-diagram-using-powershell/
91 stars 23 forks source link

Fixes #122 #126

Open vikingtiger opened 4 years ago

vikingtiger commented 4 years ago

Methods with undefined return type should not be tested for type. They should rather be treated like void methods.

This was already attempted by comparing the return type with "[void]" or $null. With an undefined return type, however, the ReturnType property (from CUClassMethod) turns out to be an empty string ("").

By making a small adjustment to two if sentences, methods with an undefined return type are no longer type tested.

Stephanevg commented 4 years ago

Hi @vikingtiger Thank you SO much for your PR. For some reason, I missed the notification of this one.

I have looked at your PR. It looks like a few tests fail now. (But I don't think it is related to your change. (See here https://ci.appveyor.com/project/Stephanevg/psclassutils/builds/28245167 ).

Before I merge this, do you think you could add a simple test with your PR so that we can ensure this 'bug'? is catched in the futur.

(Plz let me know if you are willing to do this, otherwise I'll do it my self when I find myself a moment).

Cheers and thanks again!!

vikingtiger commented 4 years ago

As far as I can tell from the AppVeyor project history, all CI tests have failed on dev branch since commit 23fb8ac4.

Regarding your request for a simple Pester test to go with this PR, I could try to add a specific test in Tests/ClassUtils.Write-CUPesterTest.Tests.ps1 to verify that Write-CUPesterTest properly handles methods of unspecified type. I'm thinking something along the lines of

It '[Write-CUPesterTest] Methods of unspecified type should be handled like void methods' {
    # (the test)
}

Is something like that what you had in mind?

Stephanevg commented 4 years ago

Yes, that would be fine. Thanks for pointing the failed commit out. I'll have a look at what broke, and fix it.