Hi @McAndersDK, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla.microsoft.com.

TTYL, MSBOT;

@McAndersDK, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, MSBOT;

Your code does not compile. Please fix before we review this PR.

it build on my machine?
I cant see in the build details what is wrong? @lzybkr

@McAndersDK you can see logs from the build server by clicking on the build status in your PR (red cross)

For example, here
https://ci.appveyor.com/project/PowerShell/powershell/build/6.0.0-alpha.9-36#L276

The compilation error message

C:\projects\powershell\src\Microsoft.PowerShell.Security\security\CredentialCommands.cs(122,83): error CS1061: 'GetCredentialCommand' does not contain a definition for 'Username' and no extension method 'Username' accepting a 
first argument of type 'GetCre
dentialCommand' could be found (are you missing a using directive or an assembly reference?)
C:\projects\powershell\src\Microsoft.PowerShell.Security\security\CredentialCommands.cs(124,40): error CS1061: 'GetCredentialCommand' does not contain a definition for 'Username' and no extension method 'Username' accepting a 
first argument of type 'GetCre
dentialCommand' could be found (are you missing a using directive or an assembly reference?)
C:\projects\powershell\src\Microsoft.PowerShell.Security\security\CredentialCommands.cs(129,67): error CS0103: The name 'caption' does not exist in the current context

Thanks @vors for some reason appveyor only showed the first 200 lines here.
I had to download the log to see the complete output.

@McAndersDK it should not be the case in general, but could be a one-off problem with AppVeyor.

Thanks for fixing the build errors.

Before accepting this PR, we'll need some tests. I was going to accept this PR without tests because Get-Credential is a bit harder to test than some cmdlets, but...

@JamesWTruher has ported our Get-Credential to Pester, hopefully we'll see a PR from him today.

After that PR is accepted, I'll ask you to add some tests for your new functionality.

@daxian-dbw Thanks for input, I will make sure this will go into the PR with the test for the changes.

We should have done this a long time ago. Thanks!

Can you add a test to test\powershell\Modules\Microsoft.PowerShell.Security\GetCredential.Tests.ps1 and fix the formatting as @rkeithhill suggested (though we'd fix that with a tool later anyway).

yeah I will. Should I create a new PR with both file in it or just create a new one with the test separate?

I'm fine with opening another issue to make the host ensure the prompt is unambiguously coming from PowerShell.

@McAndersDK - I think this is ready merge unless you're adding a test to verify the title is passed correctly to the host.

Such a test would be nice but not required as it's still insufficient in validating the title gets displayed, the only way to validate that would require an expect like test.

Yeah i need to add some more tests with the streams.

Please give me some days to add them in.

Awesome - thanks for being responsive to our feedback.

Travis just timed out?
How do we rerun travis? except to do a push?

@lzybkr That did not work. we need to test the invoke for credential object?, if none throw?
but why change this? it worked before? atleast the test did not fail, I have not tryed the other way around

The problem with not using a script block is that if it doesn't throw, the test passes but should fail. Adding the change I suggested exposed the fact that no exception was thrown, so failing the test is correct.

I think this test might not make sense. Looking at the console host - we prompt for the username if it's null or empty. Other hosts might do that or throw, but we don't really control other hosts.

I'd say just remove this test.

Set "message" to optional is not best practice, from https://msdn.microsoft.com/en-us/library/dd878348%28v=vs.85%29.aspx :
•Each parameter set must have at least one unique parameter. If possible, make this parameter a mandatory parameter.

Set both "message" and "title" to optional open a way to "anonymous" request of credential. That is bad. This provokes writing bad code.

Before this change, you could call Get-Credential with no parameters, so I thought the change was necessary to keep that behavior. Maybe it wasn't.

The current experience (after this PR) with Get-Credential and no additional parameters is broken - it prompts twice (see #2306). So maybe we need to revert the change on the Message parameter.

Before this change, you could call Get-Credential with no parameters

Get-Credential with no parameters set message to default "Enter you credentials". Message is in another parameter set and that is another experience.
So I think we should follow the msdn recommendations above mentioned.