Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Records show no coverage #1633

Open
tisonv opened this issue Feb 27, 2024 · 10 comments
Open

[BUG] Records show no coverage #1633

tisonv opened this issue Feb 27, 2024 · 10 comments
Labels
untriaged To be investigated with repro Issue with repro

Comments

@tisonv
Copy link

tisonv commented Feb 27, 2024

Describe the bug
With .Net 8, records show no coverage
Tested with the nightly build too
I read #1561 #1607 #1576

To Reproduce

public sealed record FolderInfoDto
{
    public bool IsHidden { get; init; }
    public long Creation { get; init; }
}
return new FolderInfoDto
{
    Creation = f.Creation,
    IsHidden = !string.IsNullOrEmpty(f.Mode) && 'h' == f.Mode[3]
};
internal abstract record BasicFolderDto
{
    public string? DfsMappedPath { get; init; }
    public string Path { get; init; } = null!;
}

internal sealed record CreateFolderDto : BasicFolderDto;

CreateFolderDto dto = new()
{
    Path = args != null && args.TryGetValue("path", out var path) ? (string)path : @"F:\rep1\rep2",
};
internal sealed record DeleteFolderDto : BasicFolderDto
{
    public bool Recurse { get; init; }
    public bool Force { get; init; }
}

DeleteFolderDto dto = new()
{
    Path = @"L:\path\",
};

Expected behavior
FolderInfoDto, CreateFolderDto, DeleteFolderDto, BasicFolderDto are marked as covered.

Actual behavior
They are marked as not covered.

Configuration (please complete the following information):
Please provide more information on your .NET configuration:
* Which coverlet package and version was used? 6.0.1 and nightly
* Which version of .NET is the code running on? 8.0.2
* What OS and version, and what distro if applicable? Windows 11
* What is the architecture (x64, x86, ARM, ARM64)? x64
* Do you know whether it is specific to that configuration? No

Additional context

  • Same result from Visual Studio and CLI
  • No difference with either <TargetFramework>net8.0</TargetFramework> or <TargetFramework>net8.0-windows10.0.17763.0</TargetFramework>
  • removing abstract or sealed makes no difference
  • SkipAutoProps makes no difference

test.runsettings:

<?xml version="1.0" encoding="utf-8"?>
<RunSettings>
	<RunConfiguration>
		<ResultsDirectory>.\TestResults</ResultsDirectory>
		<TargetPlatform>x64</TargetPlatform>
		<TargetFramework>net8.0</TargetFramework>
	</RunConfiguration>

	<DataCollectionRunSettings>
		<DataCollectors>
			<DataCollector friendlyName="XPlat code coverage">
				<Configuration>
					<Format>cobertura</Format>
					<Exclude>[coverlet.*.tests?]*,[*]Coverlet.Core*,[*]xunit*</Exclude>
				</Configuration>
			</DataCollector>
		</DataCollectors>
	</DataCollectionRunSettings>
</RunSettings>

❗ Please also read Known Issues

@github-actions github-actions bot added the untriaged To be investigated label Feb 27, 2024
@tisonv tisonv changed the title [BUG] ecords show no coverage [BUG] Records show no coverage Feb 27, 2024
@daveMueller
Copy link
Collaborator

Thanks for reporting and providing a repro. I see if I can get the discussion in #1576 started. I think we then would have less issues with records.

@daveMueller daveMueller added the with repro Issue with repro label Feb 28, 2024
@tisonv
Copy link
Author

tisonv commented Feb 28, 2024

Our SonarQube would not allow my PR since coverage was subpar.
I was just refactoring dtos .
I had to [ExcludeFromCodeCoverage] for the time being

@daveMueller
Copy link
Collaborator

Could you elaborate a bit on how to reproduce this? With my setup I can get coverage for those records.

grafik

@tisonv
Copy link
Author

tisonv commented Apr 2, 2024

Hello !

Latest SonarQube expects all the lines to be covered
https://community.sonarsource.com/t/sonarqube-not-showing-coverage-for-inherited-record/107476
I'm not sure whether they should be marked as covered or not

image

@Steve-OH
Copy link

A hopefully useful additional data point

  • Visual Studio 2022 17.10.0
  • FineCodeCoverage 1.1.215
  • NUnit 4.1.0
  • coverlet.collector 6.0.2
  • C# 12
  • .NET 8

I have an internal record:

internal record Foo
{
    public string Bar() => "baz";
}

And an associated test project with a test:

[TestFixture]
internal class FooTests
{
    [Test]
    public void FooBarShouldReturnBaz()
    {
        var foo = new Foo();
        Assert.That(foo.Bar(), Is.EqualTo("baz"));
    }
}

After running the test, the method call shows that it was covered, but the record itself does not:

image

If I modify the record declaration to include the redundant primary constructor parentheses:

internal record Foo()
{
    public string Bar() => "baz";
}

and repeat the test, this time I get green all around:

image

@tisonv
Copy link
Author

tisonv commented Jul 2, 2024

I confirm
Thanks @Steve-OH !

@Rast1234
Copy link

I have similar issue, it gets fixed if i remove my constructor and use the parenthesis in record declaration. Here's an example:

image

Note that constructor without coverage seems to accept an argument of the same type.

And here it's gone:

image

@daveMueller
Copy link
Collaborator

Hello !

Latest SonarQube expects all the lines to be covered https://community.sonarsource.com/t/sonarqube-not-showing-coverage-for-inherited-record/107476 I'm not sure whether they should be marked as covered or not

image

OK I see. It looks like SonarQube is expecting the record declaration as something that should be covered. This is something we are not doing. If you ignore all the generated IL of a record it is just the same as a simple class declaration. Is SonarQube expecting coverage for a class declaration? I don't think so because it doesn't really have any executable code.

image

@daveMueller
Copy link
Collaborator

@Steve-OH, @Rast1234 I think I now found one of the issues here. Starting from the sample code @Steve-OH provided. If I add the primary constructor parentheses, the generated report contains the constructor. This is an issue we need to tackle.

image

If it doesn't contain the parentheses the constructor is not part of the coverage report. Which should be the expected report in both cases because we want generated constructors to be completely excluded.

image

But in your sample, the record declaration seems to be expected to be covered also without the parentheses. This is different to what I'm currently observing. I wonder if this is part of the report? Are you able to provide me your coverage reports for both cases?

343599532-16bb60d4-c039-4dc7-914e-b82a0ada373c

@Steve-OH
Copy link

Here are the reports, with and without parentheses.

coverlet-test.zip

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
untriaged To be investigated with repro Issue with repro
Projects
None yet
Development

No branches or pull requests

4 participants