dotnet-affected icon indicating copy to clipboard operation
dotnet-affected copied to clipboard

Code Coverage for Affected Projects only

Open leonardochaia opened this issue 3 years ago • 10 comments

When testing only what's affected, the coverage report will show incorrect results.

For example, given this project structure:

  1. Project.Shared
  2. Project (.1)
  3. Project.Tests (depends on .2)
  4. Shared.Tests (depends on .1)

If .2 has changes, .3 will be affected so we will build and test .2 and .3, but there's no need to test .4 since .1 has not changed. Hence, the coverage report will show a low coverage percentage for .1 since its test have not run

We should be able to filter down the coverage report to only include the resulting projects of dotnet-affected.

leonardochaia avatar Jul 10 '21 17:07 leonardochaia

Hi Leonardo. We are setting up a CI pipeline in our monorepo project and we would like to know if you will take care of this issue soon. We like to use this tool but without this feature we will be getting incorrect data and that compromises the test coverage report.

nvsoares avatar Feb 15 '23 14:02 nvsoares

Hi @nvsoares , we are quite close of having this implemented.

  1. We have an abstraction, the IOutputFormatter which can be used to format the affected projects into an output file
  2. The TextOutputFormatter can be used as inspiration for writing new ones.

The plan is to be able to output Corvelet --include filter expression

I can't confirm when I will be able to implement this feature, so would you guys considering contributing an IOutputFormatter implementation for Corvelet or your testing processor of use?

leonardochaia avatar Feb 15 '23 15:02 leonardochaia

Hi @leonardochaia.

We are implementing an OutputFormatter for Coverlet when used with MSBuild. Since we are not yet very comfortable with the project, we would like you to say if this is what is intended.

First, we have changed the ProjectInfo class to include the AssemblyName.

public ProjectInfo(ProjectGraphNode node)
{
    this.Name = node.GetProjectName();
    this.AssemblyName = node.GetProjectAssemblyName();
    this.FilePath = node.ProjectInstance.FullPath;
}

Then, we have created the CoverletIncludeOutputFormatter like this:

internal class CoverletIncludeOutputFormatter: IOutputFormatter
{
    public string Type => "text";
    public string NewFileExtension => ".txt";
    public Task<string> Format(IEnumerable<IProjectInfo> projects)
    {
        List<string> assemblies = new List<string>();
        foreach (var project in projects)
        {
            assemblies.Add($"[{project.AssemblyName}]*");
        }
        return Task.FromResult($"{string.Join(",", assemblies)}");
    }
}

If I understand, the idea is to pass the content of the created file to /p:Include=.

nvsoares avatar Feb 16 '23 14:02 nvsoares

Hey @nvsoares , looking good :+1: I didn't notice we needed the assembly name, but adding to ProjectInfo sounds ok.

I think the output should be the filter expression (like [<Assembly>]*) ready to pass to corvelet. I.e, usage would be

dotnet affected <....> -f corvelet
dotnet test affected.proj /p:CollectCoverage=true /p:Include=(cat corvelet.txt)

I haven't tested this to check the filter expression works but we want all types from all affected assemblies so I think it should be something like according to filter docs

[<assembly 1>]*, [<assembly2]*, <... etc>

leonardochaia avatar Feb 18 '23 11:02 leonardochaia

Hi @leonardochaia. I'm a co-worker of @nvsoares and I'm also working on this issue. I have a case here that fails with this strategy. For example we have the following projects:

  1. Project
  2. Project.Tests

If the changes are only to .2, then this is the only affected project. However, it is not this one that we should put in the include filter, but .1.

Do you have any idea what we should do?

JoaoEdu93 avatar Mar 02 '23 12:03 JoaoEdu93

Hmm, interesting but yes makes sense. Basically it expects the "test covered project" as the filter target, instead of the "project that has the tests". Makes sense.

So, haven't really thought this trough but it looks like we need to compose a filter that includes all projects that .2 references

In which case it looks like the formatter needs to know the ReferencingProjects in order to compose the filter. I think we may need to expand ProjectInfo to include the flat list of ReferencingProjects. I may be able to take a look in the upcoming days.

leonardochaia avatar Mar 02 '23 15:03 leonardochaia

I think that include all references does not solve all the cases. Let's look at the following scenario:

  1. Shared (no dependencies)
  2. Shared.Tests (depends on .1)
  3. Project (no dependencies)
  4. Project.Tests (depends on .3 and .1)

The only affected project is the .4. So, we will run the tests only for project .4. Since project .4 references .1 and .3, these will be set in the include filter. The results are as follows:

  1. Shared -> 0% (Tests didn't run)
  2. Project -> 100%

The total coverage will be 50%, since the testing of the .1 project has not been run.

The only solution I have in mind is to use some convention in the naming of the test projects or have some sort of configuration file.

JoaoEdu93 avatar Mar 03 '23 16:03 JoaoEdu93

Hmmm I see.. Again, without thinking this through too much but:

The only affected project is the .4. So, we will run the tests only for project .4. Since project .4 references .1 and .3, these will be set in the include filter.

What if the formatter filters out the 4. ReferencingProjects to only include the ReferencingProjects that were built?

Sorry for the vague responses but I think I need to sit down a bit and think this through with some dummy repo to see how it behaves.

EDIT: right they were not built so this does not work

leonardochaia avatar Mar 03 '23 16:03 leonardochaia

Hello Leonardo. Have you been able to take the time to look into this question? It is very important to us. Thank you.

nvsoares avatar Mar 24 '23 10:03 nvsoares

Hi @nvsoares @JoaoEdu93 , sorry I have not been able to review this yet. Been having some busy weeks at work / life. Will try to get back to it as soon as possible.

Regards, Leo.

leonardochaia avatar Mar 30 '23 10:03 leonardochaia