Skip to content

Deprecate and unlist package version 3.0.0 and change release habits #1055

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

Closed
MarcoRossignoli opened this issue Jan 16, 2021 · 13 comments
Closed
Labels
discussion Generic discussion on something

Comments

@MarcoRossignoli
Copy link
Collaborator

MarcoRossignoli commented Jan 16, 2021

Last week we released new major version 3.0.0

Unfortunately the version has got an ugly bug has described by our amazing @daveMueller #1037 (comment)

We did a great improvement on coverage testing, today we have a way to check if we're able to recognize pattern and verify line numbers and branch numbers.
I'm very happy of the quality of our coverage.
With this version we're very close to OpenCover as reported by @JSkimming #1037 (comment) one of the best coverage solution around.

Anyway the bug is escaped from our UTs and integrations because it manifests only on multi-document project and we're testing 1 document at time(we check coverage for specific scenario and we generate 1 asm for 1 class and we tests only that class so only 1 cs file).
Also offering nightly build to our friends didn't help #1037 (comment) and I'm ok with it, it's a nightly not an "official preview".

In the spirit of "learning from our(my) errors" I propose to add one more step before official release. I think that when the new fix/improvements are "enough"(also if we have 1 big change) we should release an official preview to nuget and maybe ask to our friends of coverlet community to give it a try.

We could also open an coverlet Twitter account to ask some collaboration for preview testing.

Coverlet is becoming the choice for xplat .NET Coverage, we have a lot of download https://nugettrends.com/packages?months=12&ids=coverlet.collector&ids=coverlet.msbuild&ids=coverlet.console and users ~61K(github numbers) and we're on MS docs https://docs.microsoft.com/en-us/dotnet/core/testing/unit-testing-code-coverage?tabs=windows with the privilege of VSTest strict integration --collect "XPlat Code Coverage", for all these reason I think we need to do a step further also on this part of "chain" to improve the quality and move @tonerdo's baby to the adolescence😄

Please let me know what you think @tonerdo @petli

@MarcoRossignoli MarcoRossignoli added the discussion Generic discussion on something label Jan 16, 2021
@MarcoRossignoli MarcoRossignoli pinned this issue Jan 16, 2021
@Malivil
Copy link

Malivil commented Jan 16, 2021

You can add me to that list of people willing to test an official preview =)

I can't update to nightly builds but if you tag me in a preview test I can dedicate some time to run it through my solution.

@MarcoRossignoli
Copy link
Collaborator Author

MarcoRossignoli commented Jan 16, 2021

Sure thx @Malivil
I apologize wasn't my intention exclude/offend/forget nobody I wrote down first names came into my mind related to my past experience on coverlet with bugs/tests etc...that "etc" was not appropriate, I'm going to update immediately.

@MarcoRossignoli
Copy link
Collaborator Author

MarcoRossignoli commented Jan 16, 2021

Another idea could be add an announcement label and let coverlet users to subscribe to it when we release preview.
There we can invite with a cc:

@Malivil
Copy link

Malivil commented Jan 16, 2021

Sure thx @Malivil
I apologize wasn't my intention exclude/offend/forget nobody I wrote down first names came into my mind related to my past experience on coverlet with bugs/tests etc...that "etc" was not appropriate, I'm going to update immediately.

No worries at all. I didn't take it that way, I just wanted to put my name forward to help out as well =)

@JSkimming
Copy link

In the spirit of "learning from our(my) errors" I propose to add one more step before official release. I think that when the new fix/improvements are "enough"(also if we have 1 big change) we should release an official preview to nuget and maybe ask to our friends of coverlet community to give it a try.

Just adding my 2 cents. I'm not convinced such caution is necessary, though happy to be persuaded otherwise.

This is a test library, so not going to break runtime code. Also, by definition, coverlet's usage is 100% covered by tests. If someone receives a degraded result, like #1037, it's quite likely to be something consumers are monitoring, and therefore have the option to reject the upgrade, which is exactly what I did.

I have the same view as with libraries like xunit, Moq, Fluent Assertions, etc. When Dependabot pushes upgrades, I quickly check the build's output, and if all is good, accept.

@MarcoRossignoli
Copy link
Collaborator Author

Thanks for your opinion @JSkimming!
Understood your point on "test library" meaning.

@petli
Copy link
Collaborator

petli commented Jan 19, 2021

After thinking a bit about this I think I agree with @JSkimming. In general bugs will be found faster if they get pulled into PRs by dependabot across the ecosystem, rather than just by a subset of preview users. If people automatically also merge those PRs without reviewing them is up to them.

This particular bug, by affecting almost every single user, means that we got a bunch of duplicate issues reported which added some noise, but rather than that a bug not being found at all.

@MarcoRossignoli
Copy link
Collaborator Author

Thanks @petli

@pkrukp
Copy link

pkrukp commented Jan 20, 2021

Anyway the bug is escaped from our UTs and integrations because it manifests only on multi-document project and we're testing 1 document at time(we check coverage for specific scenario and we generate 1 asm for 1 class and we tests only that class so only 1 cs file).

I have not seen any mentions about improving automatic tests. "multi-document project" is the normal case, nobody has projects with only one document. Did you consider adding such test?

@MarcoRossignoli
Copy link
Collaborator Author

I have not seen any mentions about improving automatic tests. "multi-document project" is the normal case, nobody has projects with only one document. Did you consider adding such test?

We should try to add something on that side. Not super easy btw, at the moment we test all "patterns" not whole projects because every project is different so it's hard find a way to have a complete tests scenario. We'll improve for this specific case for sure.

Thanks for the comment.

@pkrukp
Copy link

pkrukp commented Jan 20, 2021

Maybe following could work and be easy to implement:

Collect coverage from some third party project. The project should be non trivial, should already have a good number of UT and a good coverage. Collect coverage from the UT before each release (or even after each commit if possible). If coverage numbers change and it was not expected, you would have to manually review why it changed.

This assumes normally coverage numbers do not change (given same version/commit of the third party project) and so it should have low maintenance cost.

As a bonus, you can track/notice performance changes.

@MarcoRossignoli
Copy link
Collaborator Author

Thanks @pkruk2 it's a good idea...we could try with some project like dotnet/roslyn, dotnet/runtime(they're using coverlet)

@MarcoRossignoli
Copy link
Collaborator Author

MarcoRossignoli commented Jan 23, 2021

Thanks for all comments, the outcome is:

  • Preview testers workflow seems overkill for a "test tool" where a user can do not accept an upgrade if it's not happy with that.
  • Anyway we can publish preview "silently" for who what to avoid nightly build feeds and too much infra but are able to use preview from official nuget feed.
  • We can choose a project that is using coverlet and before release compare numbers to catch macro regression.
  • Improve UTs/Integration

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

No branches or pull requests

5 participants