coverlet icon indicating copy to clipboard operation
coverlet copied to clipboard

Closing brace is missed when writing PowerShell cmdlets and calling WriteError

Open fgimian opened this issue 3 years ago • 12 comments

Hey there, firstly thank you so much for this incredible tool! 😄

When writing PowerShell cmdlets in C#, it is common to run the inherited WriteError to write non-terminating errors. Sadly it seems that Coverlet does not detect the closing brace in this scenario.

Here's a simply demo example:

using System;
using System.Management.Automation;

namespace Lunette.Cmdlets.Utilities
{
    [Cmdlet(VerbsCommon.Get, "SpecialFolderPath")]
    [OutputType(typeof(string))]
    public class GetSpecialFolderPathCommand : Cmdlet
    {
        [Parameter(Position = 0, Mandatory = true)]
        public string Name { get; set; }

        protected override void ProcessRecord()
        {
            if (Name == "BAD")
            {
                WriteError(new ErrorRecord(
                    new ArgumentException("oh no"),
                    "ItemNotFoundException",
                    ErrorCategory.InvalidArgument,
                    "Hello"));
            }
            else
            {
                WriteObject("hello");
            }
        }
    }
}

And here are the tests:

using System;
using System.Linq;
using Lunette.Cmdlets.Utilities;
using Xunit;

namespace Lunette.Tests.Cmdlets.Utilities
{
    public class GetSpecialFolderPathCommandTests
    {
        [Fact]
        public void Invoke_ValidSpecialFolder_ShouldReturnPath()
        {
            // Arrange
            var cmdlet = new GetSpecialFolderPathCommand()
            {
                Name = "GOOD"
            };

            // Act
            var results = cmdlet.Invoke().OfType<string>().ToList();

            // Assert
            Assert.Equal("hello", results[0]);
        }

        [Fact]
        public void Invoke_InvalidSpecialFolder_ShouldError()
        {
            // Arrange
            var cmdlet = new GetSpecialFolderPathCommand()
            {
                Name = "BAD"
            };

            // Act & Assert
            var exception = Assert.Throws<ArgumentException>(
                () => cmdlet.Invoke().GetEnumerator().MoveNext());
            Assert.Equal("oh no", exception.Message);
        }
    }
}

And as per the coverage report, the closing brace on line 22 is not covered:

image

Any ideas if there's a way to work through this?

Huge thanks! Fotis

fgimian avatar Jun 25 '21 13:06 fgimian

P.S.: I am using a Debug build and have also tried to execute cmdlet.Invoke().OfType<string>().ToList() in the second test to be sure that the enumerator is fully exhausted, but sadly I see the same result.

fgimian avatar Jun 25 '21 13:06 fgimian

Which version are you using?Can you try with nightly? https://github.com/coverlet-coverage/coverlet/blob/master/Documentation/ConsumeNightlyBuild.md

MarcoRossignoli avatar Jul 05 '21 17:07 MarcoRossignoli

Which version are you using?Can you try with nightly? https://github.com/coverlet-coverage/coverlet/blob/master/Documentation/ConsumeNightlyBuild.md

Thanks so much for the reply Marco. I'm using the latest release available on nuget which is 3.0.3. I'll try the nightly as you recommended and let you know how I go. 😄

fgimian avatar Jul 05 '21 22:07 fgimian

I just attempted this with the latest nightly 3.0.4-preview.32.gdc6edb1dd7 but sadly the problem persists. Would it assist you if I produced a complete minimal project / solution that demonstrated the problem so you can reproduce it?

P.S.: I'm not certain if this is relevant, but I am developing on .NET Core 3.1 as I am targeting PowerShell 7.0.x right now.

fgimian avatar Jul 05 '21 22:07 fgimian

Here's a fully setup solution with related library and test projects.

CoverletProblem.zip

I then invoke tests and Coverlet using:

dotnet test /p:CollectCoverage=true /p:CoverletOutputFormat=lcov /p:CoverletOutput='../lcov.info'

You will see that line 22 is not covered in the lcov output:

sls ",0$" .\lcov.info

I used the latest nightly build in this solution along with .NET 5.0 to rule out a specific issue with .NET Core 3.1.

Please let me know if I can provide any further info to help 😄

Thanks so much! Fotis

fgimian avatar Jul 05 '21 22:07 fgimian

Thanks for the repro!

MarcoRossignoli avatar Jul 06 '21 06:07 MarcoRossignoli

Hey there Marco, I've done a lot more digging and believe that I can explain what's going on a lot better. I also think I have a better solution for testing cmdlets which I'll post soon.

Ultimately, this is relatively vague territory as Microsoft don't provide a good guide for testing cmdlets written in C# using XUnit so one is left to dig deeper into the PowerShell source to figure out what's going on.

The summary is as follows:

  • Invoking a Cmdlet seems actually call the respective underlying method in an implementation of ICommandRuntime, where the default implementation is DefaultCommandRuntime.

  • As you'll see, the code that gets executed when calling WriteError is as follows:

    public void WriteError(ErrorRecord errorRecord)
    {
        if (errorRecord.Exception != null)
            throw errorRecord.Exception;
        else
            throw new InvalidOperationException(errorRecord.ToString());
    }
    
  • A good solution seems to be to mock this and replace the CommandRuntime before invoking the cmdlet in tests, similar to what has been done by Andrew Theken here. Another example is seen in the azure-powershell repo here.

  • I do still wonder why Coverlet didn't pick up the closing brace after the exception was thrown by DefaultCommandRuntime though; I assume this is because it is thrown indirectly and Coverlet has no way to know this. Based on this knowledge, we should be able to produce a more minimal non-PowerShell specific repro of this issue to see if you have any ideas whether or not it's possible to cater for.

I'll provide further information soon, but just thought I'd share an update with you in the meantime.

Huge thanks Fotis

fgimian avatar Jul 09 '21 23:07 fgimian

Good news, as I suspected, I can reproduce the problem with a more succinct example which I've attached below:

CoverletProblem-v2.zip

The summary of the problem is that Coverlet will detect a closing brace if the method being called throws an exception. However, it will not detect the closing brace if the method calls another method, and the other method throws the exception.

e.g.

if (name == "BAD")
{
    // PROBLEM HERE: If WriteError throws an exception, the closing brace below 
    // won't be counted by Coverlet.
    _handler.WriteError();
}
else
{
    // OK: Coverlet will cover the closing brace if the method directly throws the exception.
    throw new ArgumentException("oh no");
}

I'm assuming this would be extremely hard to cater for, but perhaps you have an idea how to solve it?

Thanks again for all your help and time! Fotis

fgimian avatar Jul 10 '21 00:07 fgimian

And one final update. I also tested this with OpenCover and it had the same problem as Coverlet.

Instructions for doing this via OpenCover (for your reference) are as follows:

# The following instructions assume you've installed OpenCover using the MSI installer
# provided at https://github.com/OpenCover/opencover/releases

# Create a directory for coverage reports
mkdir .coverage

# Run tests using OpenCover
~\AppData\Local\Apps\OpenCover\OpenCover.Console.exe `
    -target:"dotnet.exe" `
    -targetargs:"test" `
    -output:".coverage\coverage.xml" `
    -register:user -filter:"+[CoverletProblem]*"

# Install ReportGenerator globally
dotnet tool install -g dotnet-reportgenerator-globaltool

# Generate the coverage HTML report
ReportGenerator.exe -reports:".coverage\coverage.xml" -targetdir:".coverage"

# View in your browser
ii .coverage\index.html

Cheers Fotis

fgimian avatar Jul 10 '21 00:07 fgimian

Thanks for all informations and for the repro!

MarcoRossignoli avatar Jul 10 '21 11:07 MarcoRossignoli

In case it's useful to anyone, I've written a more detailed blog post on the approach I've used to overcome this while developing PowerShell cmdlets in C#.

fgimian avatar Jul 11 '21 07:07 fgimian

This issue is stale because it has been open for 3 months with no activity.

github-actions[bot] avatar Sep 10 '23 01:09 github-actions[bot]