Consider allowing property to add to file filters for code coverage

[heaths]

https://github.com/Azure/azure-sdk-for-net/blob/2cfa587f85f734fc7da3573a7349b5f642e73e44/eng/CodeCoverage.targets#L63C90-L63C90

We could modify the line above to also take filters from test projects that would ignore shared code, which a lot of libraries use. Ideally, this shared code is tested elsewhere e.g., Azure.Core.TestFramework does this, so every project that uses it doesn't have to.

[heaths]

We might consider an item group of file patterns to exclude - easier to get right - and then use item transforms to inject the proper syntax. That said, I worry that a broad filter might explode the command line to the point it's excessive and errs. /cc @hallipr

[heaths]

@annelo-msft is this still necessary when all of the shared code is moved to public types in Azure.Core?

[annelo-msft]

@heaths, I'm not sure I understand the question -- can you help me understand what the lines of code referenced in the issue are doing and why they're needed today? That will help me extrapolate to a later time when public types are in Azure.Core to say whether it would be needed in that world. Thanks!

[heaths]

Shared code - code we linked in .csproj files - wasn't getting covered if not tested by individual SDKs' test assemblies. But since you have been moving shared code to publicly exposed code in Azure.Core, there should be little - if any - shared code anymore, right?

[annelo-msft]

Ah, cool, thanks for the explanation!

I expect it will take a while to move everything to public types, but you are correct that this wouldn't be needed once we get to that point. It's also true that once we have #38304, we would expect to have good test coverage of any remaining shared types in Core, so that coverage shouldn't be needed in the service library projects themselves.

An alternative to consider to get higher code coverage numbers for shared source files in service libraries might be to include the test files for the shared source files in the projects that reference them, but that could get messy from a dependency perspective, so probably not something we'd want to pursue.

[heaths]

so that coverage shouldn't be needed in the service library projects themselves.
...
An alternative to consider to get higher code coverage numbers for shared source files in service libraries

That's exactly the problem: lack of actual tests for shared files, and we don't want to put [ExcludeFromCodeCoverage] on shared files' types because we want to make sure they are tested as part of Azure.Core. Pavel and I did consider including tests, but - as you noted - gets messy very quickly.

The biggest issue is that most of code coverage configuration is through files (XML, if that gives you some idea of age), so a "one size fits all" approach for any given track 2 SDK in a monorepo doesn't really work i.e., the configuration isn't scalable.

One thing that might help is just to exclude any types with a namespace prefix of Azure.Core, except for Azure.Core itself, but even that gets hard to populate. This is why the reports are filtered as much as we can instead of code coverage numbers that AzDO sees modified to exclude sources. /cc @pallavit who was asking about this as well.

[annelo-msft]

Is the top-level goal of this effort to improve code coverage numbers for all libraries, and this piece with shared source is one of the items standing in the way of that goal?

[heaths]

That's correct. A naive - or perhaps temporary (but maybe more work than it's worth) - is to [ExcludeFromCodeCoverage] all the shared source but test it in Azure.Core.Tests anyway. It wouldn't hurt CC numbers for any SDK including Azure.Core. But let's discuss it and how it relates to MQ offline. This naive approach could work but, again, may not be worth it.

[heaths]

We discussed this in our team today and because shared source is most of the legitimate reason to exclude files and because those should be migrated to publicly exported types in 6-12 months as needed, do not feel additional investment is necessary at this time. We also feel code coverage metrics can be misleading, as briefly described in the associated PR to update our CONTRIBUTING.md with our "soft policy" on this.