MicrosoftPremier / VstsExtensions

Documentation and issue tracking for Microsoft Premier Services Visual Studio Team Services Extensions
MIT License
56 stars 14 forks source link

Build Quality Checks - Success when comparing to 0/0 elements for any absolute change #215

Closed gilaser closed 1 year ago

gilaser commented 1 year ago

Hi there. Really excited about this task.

When trying to add this task to our build pipeline, I found that doing a check on code coverage percentage will automatically pass when compared to a baseline build which has no code coverage, because it recognizes the 0% baseline comparison should be result in automatic success. But when trying to implement a code coverage policy that looks at absolute change of elements, it will always fail when compared to an uncovered baseline build (at least in the uncovered elements = true case).

Is it possible to add a check such that when the code coverage policy is looking at absolute change, if the current code coverage is 0/0, then it should always pass?

Thanks in advance!

ReneSchumacher commented 1 year ago

Hi @gilaser,

I believe there are a couple things mixed up here. There is an option to compare absolute or percentage values, and there is an option to look at covered or uncovered elements. Both should be independent of each other.

When comparing to the previous build, you can choose to compare percentage values or absolute values. Both behaves in the same way; i.e., if the baseline has zero coverage and the current build has zero coverage, the policy passes.

If you switch from covered to uncovered elements, we check (and I cannot remember the reason for doing this) if there are coverable elements. If there are none, we fail the policy by default. As mentioned, I don't remember why we thought this was a good idea. Maybe the reason was that you cannot improve if there's nothing to cover (you can never cover less than zero), but that would also be true for the other check (if there's nothing to cover, you could never cover more than zero). Maybe we should just remove that check? 🤔

gilaser commented 1 year ago

Hello!! Thank you for clarifying each of the pieces.

Could it be that the reason you have that check is because you want to fail when you go from no coverable elements to no coverable elements, with a legitimate uncovered element policy check? If that's the case, perhaps adding a check beforehand such that if you go from no coverable elements to some amount of coverable elements, then to automatically pass?

ReneSchumacher commented 1 year ago

I don't think so. Having nothing to cover in both the baseline and current build does not point to any decrease in quality. So, having zero coverable elements in both builds seems to be okay to me.

If the baseline build has no coverable elements and the current build has, the policy (imho) should always pass. Otherwise, we would force people to always cover 100% of their code as a single uncovered element would always be worse then zero uncovered elements. Thus, the first build with coverable elements should set the baseline. If we treat zero coverable elements as infinite uncovered elements, the logic would work again 😄 Because then both build would have infinite uncovered elements until the current one has coverable elements. And even if the current build has zero coverage (i.e., 100% uncovered elements), any number of elements would be better than infinite and, thus, set the baseline.

gilaser commented 1 year ago

That makes sense to me! Is it possible for you to add this change of treating zero coverable elements as infinite uncovered?

Thanks so much for your help and responsiveness here!

ReneSchumacher commented 1 year ago

I'm on it. I didn't have much time in the past to work on the tasks, but I must update them to Node 16 anyway. I'll make sure to include the change.

gilaser commented 1 year ago

Thank you!

ReneSchumacher commented 1 year ago

I'm closing this issue now as I have implemented the change in the next version (v9.0.0), which hopefully goes out to the marketplace by the end of the week. If the change doesn't meet your needs, let me know by either commenting on the closed issue or opening a new one.

Thanks for your feedback!