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

Guidance or suggestions needed for when .proto files are generating classes #1141

Closed
StingyJack opened this issue Apr 11, 2021 · 9 comments
Closed
Labels
waiting for customer Waiting for customer action

Comments

@StingyJack
Copy link
Contributor

StingyJack commented Apr 11, 2021

I have projects that have .proto files in them and that are using the Grpc.Tools nuget package. That package will "compile" the contents of the proto file into c# classes whenever the proto is changed and saved or when the project is built.

This generated code does not carry the [ExcludedFromCodeCoverage] attribute, nor is it in a .g.cs file, but it does have an //<AutoGenerated> on the first line, and all of the code is inside of a #region Designer generated code region. Coverlet is currently including this code as "not covered", but as its code that is generated by a 3rd party tool, I think it should be excluded from the coverage evaluation.

What should or can I do here to avoid having this code be included in the coverage evaluation?

EDIT For completeness, the things that are commonly used to denote code that is generated and that should be excluded are

  • [ExcludeFromCodeCoverage]
  • [DebuggerNonUserCode]
  • [GeneratedCode]
  • files ending in .g.cs, .g.vb, or similar
  • files ending in .designer.cs, .designer.vb, or similar
  • code inside a region #region Windows Form Designer generated code
  • There are one or two more that I cant remember at the moment.

protobuf compiler uses none of these conventions and instead uses

  • code inside a region #region Designer generated code
  • a comment preceding code in the file the form of an xml element that looks like // <AutoGenerated>

An example proto file contents

syntax = "proto3";
option csharp_namespace = "Core.Schema";
package example_data_schema;

message ExampleData {
	bytes id = 1;
	string name  = 3;
	int32 code = 5;
	string description = 7;
}

generates a code file that starts like this

// <auto-generated>
//     Generated by the protocol buffer compiler.  DO NOT EDIT!
//     source: Protos/example_data_payloads.proto
// </auto-generated>
#pragma warning disable 1591, 0612, 3021
#region Designer generated code

using pb = global::Google.Protobuf;
using pbc = global::Google.Protobuf.Collections;
using pbr = global::Google.Protobuf.Reflection;
using scg = global::System.Collections.Generic;
namespace Core.Payloads {

  /// <summary>Holder for reflection information generated from Protos/example_data_payloads.proto</summary>
  public static partial class ExampleDataPayloadsReflection {

    #region Descriptor
    /// <summary>File descriptor for Protos/example_data_payloads.proto</summary>
    public static pbr::FileDescriptor Descriptor {
      get { return descriptor; }
    }
    private static pbr::FileDescriptor descriptor;

    static ExampleDataPayloadsReflection() {
      byte[] descriptorData = global::System.Convert.FromBase64String(
//400 more lines of protocol buffer support

Also linking grpc/grpc#25950

@MarcoRossignoli
Copy link
Collaborator

MarcoRossignoli commented Apr 19, 2021

Have you tried with "type filtering"? Do you know the name of types right?

https://github.com/coverlet-coverage/coverlet/blob/master/Documentation/MSBuildIntegration.md#filters

@MarcoRossignoli MarcoRossignoli added waiting for customer Waiting for customer action and removed untriaged To be investigated labels Apr 19, 2021
@StingyJack
Copy link
Contributor Author

How does that work for Visual Studio's Test Explorer?

@StingyJack
Copy link
Contributor Author

@MarcoRossignoli - these c# types are generated at compile time by protoc.exe from a .proto file. Sure I can guess but there wont be a pattern from project to project.

@StingyJack
Copy link
Contributor Author

@MarcoRossignoli the protoc maintainers are going to add [GeneratedCode] to the types they are generating. Will coverlet skip those automatically?

@petli
Copy link
Collaborator

petli commented May 10, 2021

@StingyJack They will not be skipped automatically, you have to include the attribute in the ExcludeByAttribute setting. Documentation on that setting:
https://github.com/coverlet-coverage/coverlet/blob/master/Documentation/MSBuildIntegration.md#attributes

Example of how to set it with the VSTest integration:
https://github.com/coverlet-coverage/coverlet/blob/master/Documentation/VSTestIntegration.md#advanced-options-supported-via-runsettings

@StingyJack
Copy link
Contributor Author

StingyJack commented May 10, 2021

in the case of the proto generated classes, we are getting by with writing a small "code coverage patch" file that has additional parts for all the partial classes that protoc generates and adding the Exclude attribute to it. It looks like this...

namespace YourProtosNamesapce
{
    using System.Diagnostics.CodeAnalysis;

    [ExcludeFromCodeCoverage]
    public static partial class ExampleDataProtoService     {    }

    [ExcludeFromCodeCoverage]
    public static partial class ExampleDataStackReflection    {    }
//etc...
}

... which is not hard to create but still takes time to make.

Are there any reasons why [GeneratedCode] is not automatically skipped as part of the analysis?

_excludedAttributes = PrepareAttributes(parameters.ExcludeAttributes, nameof(ExcludeFromCoverageAttribute), nameof(ExcludeFromCodeCoverageAttribute));

@petli
Copy link
Collaborator

petli commented May 11, 2021

I think it is because there is no IncludeByAttribute, so users who would want code coverage on [GeneratedCode] would have no way of getting that. [ExcludeFromCoverage]/[ExcludeFromCodeCoverage] are the only two attributes that clearly states that code should be excluded.

Adding [GeneratedCode] to the list of standard exclusion would be a backward-incompatible change, so it would require a new major version in addition to a mechanism to revert to the old behaviour.

@StingyJack
Copy link
Contributor Author

@petli - once the GrpcTools version that marks the generated code as [GeneratedCode] is released I think the runsettings is going to be the way to go since we can just push a single file for every repo, or alter the build defs to create/modify to include one. I didnt consider the runsettings since we use a mix of mstest, nunit, and xunit, and had mentally associated it as being an mstest thing.

@StingyJack
Copy link
Contributor Author

Also here is a link that was shared with me to what the Roslyn compiler thinks is generated code. dotnet/docs#24657 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting for customer Waiting for customer action
Projects
None yet
Development

No branches or pull requests

4 participants