GeezerVison

Note that I have not added unit tests yet for this PR. I looked to see where BeginInvoke and EndInvoke were tested, and they are included as part of a Pester test; however, PowerShell does not support async/await, so Pester does not appear to be an appropriate test framework for these changes. Please identify how these new methods should be automatically tested. I haven't added C# unit tests to this repo yet, so I'm not sure how I should proceed. Thank you.

Kirk Munro (@KirkMunro) Have you any gists with demo? It would help to document the enhancement.

I don't have examples showing how complicated this can be using BeginInvoke/EndInvoke, because I just went straight to defining these locally in projects where I have used async PowerShell invocation since they are just so much simpler, so I don't have a before/after example to show for comparison. Also, the code is so short, I'll just share it here instead of in a gist. Here's a snippet showing what use of this looks like when you want async invocation of PowerShell from a .NET assembly with these additional methods in place:

using (var ps = PowerShell.Create())
{
    ps.AddCommand("Connect-AzureRmAccount")
      .AddParameter("ServicePrincipal")
      .AddParameter("TenantId", tenantName)
      .AddParameter("Subscription", subscriptionId)
      .AddParameter("Credential", credential);

    var results = await ps.InvokeAsync();
    // Process results
}

Kirk Munro (@KirkMunro) You can add C# tests here: https://github.com/PowerShell/PowerShell/tree/master/test/csharp

@PowerShell/powershell-committee agree that we should expose a task model for async PowerShell, leaving implementation details to PR review

Bruce Payette (@BrucePay) Can you please review the signature of the proposed task-based async APIs?

In committee's discussion on this, there are proposals that we might want to re-implement the async invocation in the task-based model instead of wrapping the Begin/End Invoke. But as long as the API signatures are reviewed and approved, I guess we can go with wrapping the existing async APIs for now, and replace the implementation of those task-based APIs at later time, if we want to.

Regarding c# unit tests, I didn't get to them before going on vacation this week, so I'll come back to this PR next week and catch up.

@powershell-maintainers The test/csharp folder is targeted as Linux-only tests. Suggestion is to put new tests in test/xUnit/System.Management.Automation. Sample https://github.com/iSazonov/PowerShell/tree/add-unicode3/test/xUnit/System.Management.Automation

Ilya (@iSazonov) I'll move the tests over. Thanks for letting me know the correct path for those tests.

Wait. Ilya (@iSazonov), that path doesn't exist in master, so this is a new process then? How are tests under that path executed locally for verification?

Kirk Munro (@KirkMunro) We haven't such tests at all. And my question is for MSFT team where we should place these tests. I point my new unmerged code which have engine API xUnit tests too. I look how CoreFX places API tests and do the same.

Ilya (@iSazonov) Ok, thanks. I'll wait until the other maintainers discuss with you what should be done with these then, and follow the process accordingly.

This PR has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed if no further activity occurs within 10 days.
Thank you for your contributions.
Community members are welcome to grab these works.

Kirk Munro (@KirkMunro) Looks like Ilya (@iSazonov) has created a place for the xUnit tests in #8356 😄

Yes, binary tests was moved to new folder. Now all tests run parallel (excluding macOS).

You xUnit tests were failing in CIs. Please fix them. I'm also fine that you remove the xUnit tests for this PR.

I removed them since they were just duplicates of the Pester tests, and I'm not set up at the moment to easily test C# Unit tests (hence the issues with the CI not working).

Pending any further feedback, I think this PR is good to go with enough tests to cover the new Async APIs well enough.

Kirk Munro (@KirkMunro) Thanks for your contribution!