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

Class name invalid for gcov parser #516

Closed
t-moe opened this issue May 3, 2022 · 12 comments
Closed

Class name invalid for gcov parser #516

t-moe opened this issue May 3, 2022 · 12 comments

Comments

@t-moe
Copy link

t-moe commented May 3, 2022

Describe the bug
When I use ReportGenerator to convert a gcov report to a cobertura report, the file path is not stripped from the class name.

To reproduce

  1. Generate a coverage file with lcov. test_coverage.info with contents like
...
TN:
SF:/home/agent/src/internal/MasterInternal.h
DA:10,0
DA:24,0
end_of_record
...
  1. Invoke ReportGenerator like this
    ReportGenerator -reports:./test_coverage.info -sourcedirs:/home/agent/src -targetdir:./Results/ -reporttypes:Cobertura

  2. Take a look at the generated ./Results/Cobertura.xml file:

<class name="/home/agent/src/internal/MasterInternal.h" filename="/home/agent/src/internal/MasterInternal.h" line-rate="1" branch-rate="1" complexity="NaN">
    ....
</class>
  1. The expected output should be:
<class name="MasterInternal.h" filename="/home/agent/src/internal/MasterInternal.h" line-rate="1" branch-rate="1" complexity="NaN">
    ....
</class>

Why is this a problem?
I later merge reports together using ReportGenerator with -reporttypes:HtmlInline_AzurePipelines.
Classnames containing a path like this, are not correctly rendered in the report.

Possible Fix
https://github.com/danielpalme/ReportGenerator/blob/main/src/ReportGenerator.Core/Parser/GCovParser.cs#L89
var @class = new Class(fileName, assembly);
var @class = new Class(className, assembly);

@t-moe
Copy link
Author

t-moe commented May 3, 2022

As a bonus: It would be nice if the assembly name could be derived from the input filename somehow.
Instead of the hardcoded Default assembly: https://github.com/danielpalme/ReportGenerator/blob/main/src/ReportGenerator.Core/Parser/GCovParser.cs#L56

@danielpalme
Copy link
Owner

I applied your proposed fix in dc395da, thanks for that!

Regarding the assembly name: I'm not sure if the file name is helpful. In your example it would be test_coverage.info.

@t-moe
Copy link
Author

t-moe commented May 4, 2022

Wow, thanks for this blazing fast implementation & reply.

Regarding the assembly name:
I actually invoke Report Generator with multiple gcov reports like this:
ReportGenerator -reports:./test_coverage.info;./test2_coverage.info <remaining parameters>.
Each file comes from a separate test binary. Currently, when I merge them together to a Cobertura/HtmlInline_AzurePipelines report, all Classes are shown under the same Default assembly. I would prefer it, if I can at least see that they come from separate test runs...

Some ideas, that would enable this:

  • Use the file name as assembly name :). test_coverage.info test_coverage2.info
  • Append a counter suffix to the default assembly name, so that they are shown as distinct assemblies. Default 0 Default 1
  • Allow specifying the default assembly-name as command line parameter. This default assembly-name would be used for all assemblies that don't have a name extracted by the parser. As a consequence, I would have to convert each GCOV report to a cobertura report separately, before merging them together.
    ReportGenerator -reports:./test_coverage.info -assemblyname:MyTest1 -targetdir:./IntermediateResults/ -reporttypes:Cobertura
    ReportGenerator -reports:./test_coverage2.info -assemblyname:MyTest2 -targetdir:./IntermediateResults/ -reporttypes:Cobertura
    ReportGenerator -reports::./IntermediateResults/*.xml -targetdir:./Results/ -reporttypes:Cobertura
  • Allow to somehow specify the default assembly-name via command line parameter for each passed report separately.

@danielpalme
Copy link
Owner

I like this approach:

Append a counter suffix to the default assembly name, so that they are shown as distinct assemblies. Default 0 Default 1

Reasons:

  • No additional command line parameters required
  • Works great for several coverage files with the same name, but in different directories

@danielpalme
Copy link
Owner

I thought about the assembly naming again.

If a suffix is added, it will only work if each file represents a different assembly.
There are most certainly situations where this is not the case and you want to merge all files into a single result.

@t-moe
Copy link
Author

t-moe commented May 9, 2022

Good point.
I like the approach with the new command line parameter that allows to specify the default assembly name. We actually merge reports also from Cppcoverage (windows tests) and they all have a proper assembly name. And then we have the gcov reports (linux tests) that are all under the 'Default'-Assembly, with no way of separating them per test-run or renaming them :(.

But I can see your points as well. It's a bit of an effort to introduce a new command line parameter. And in case I want to specify a different assembly name for multiple reports, I would have to invoke the tool multiple times...

@danielpalme
Copy link
Owner

danielpalme commented May 14, 2022

Release 5.1.7 contains a new setting/command line parameter which will allow to specify the default name.
You can use the following setting:

settings:defaultAssemblyName=MyTest1

See also:
https://github.com/danielpalme/ReportGenerator/wiki/Settings

@t-moe
Copy link
Author

t-moe commented May 16, 2022

Fantastic work. Thank you so much.

@t-moe t-moe closed this as completed May 16, 2022
@t-moe
Copy link
Author

t-moe commented May 16, 2022

I have just tried it in our production pipeline.
The default assembly name setting works 🎉 .

But the class names still contain the full path 😢.
Is it possible that only windows-style path-separators are supported?
The tool is invoked by the pipeline like this: /usr/bin/dotnet /home/agent/agent/_work/_tasks/reportgenerator_be803a55-9253-4895-a525-be570d86f161/5.1.7/tools/netcoreapp3.1/ReportGenerator.dll <....>
(See also: https://docs.microsoft.com/en-us/dotnet/api/system.io.path.directoryseparatorchar?view=net-6.0 )

@danielpalme
Copy link
Owner

Could you please try again with release 5.1.8?

@t-moe
Copy link
Author

t-moe commented May 18, 2022

Now it works ☺️ .
Thank you.

@danielpalme
Copy link
Owner

Sorry, had to revert the change in 5.1.9 because it caused #522.
If you have several classes with the same name in your code base, the name without the path is not unique.

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

No branches or pull requests

2 participants