Skip to content

Empty lines in compiler generated methods #1023

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

Conversation

daveMueller
Copy link
Collaborator

Partially closes #799

@daveMueller
Copy link
Collaborator Author

daveMueller commented Dec 20, 2020

Moved the code to skip nested ranges from Coverage (#1006) to the Instrumenter. Thus I could
omit the empty line from the Document.
-> see latest comments

@daveMueller
Copy link
Collaborator Author

/azp run

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 1023 in repo coverlet-coverage/coverlet

@daveMueller
Copy link
Collaborator Author

Somebody has an idea what went wrong on the macOS Debug build with the integration test Msbuild_SourceLink?

@MarcoRossignoli
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@daveMueller
Copy link
Collaborator Author

Mhh OK this is strange. I just disabled 2 integration tests and enabled them back again and now all tests passed. I also don't have any problems when executing the integration tests locally. I guess we should discuss this when someone finds time to review this. Maybe it has something to do with serialization/deserialization speed because I extended the InstrumenterResult by a new DataMember.

@MarcoRossignoli
Copy link
Collaborator

MarcoRossignoli commented Dec 23, 2020

@daveMueller I need to deeply check...that new prop serialized could be very big, have you compared before and after size to find out some sort of ratio?
Can we avoid to persist it?
I mean if you use it during instrumentation to skip some sequences and after we reload it for other reason(nested accounting) should work, right?
Pay attention in tests, when you debug inprocess you need to clean that list https://github.com/coverlet-coverage/coverlet/blob/master/test/coverlet.core.tests/Coverage/InstrumenterHelper.cs#L120 the prepared result is in memory so we need to simulate the serialization(on file) and deserialization(from disk).

@MarcoRossignoli MarcoRossignoli added the tenet-coverage Issue related to possible incorrect coverage label Dec 23, 2020
@daveMueller
Copy link
Collaborator Author

@MarcoRossignoli I now thought of a better solution. I always assumed the documents list is immutable in Coverage and that's why I made the changes in the Instrumenter. Just to answer your questions real quick.

  1. No changes regarding serialization anymore
  2. Avoided to persist it because everything is done in Coverage
  3. Yes this was my intention to reuse the data after reloading but this also is obsolete now.

Also thanks for tip with the inprocess debugging.

@MarcoRossignoli
Copy link
Collaborator

MarcoRossignoli commented Jan 20, 2021

@daveMueller is this PR still needed?

@daveMueller
Copy link
Collaborator Author

@MarcoRossignoli I will check this again and get back to you.

@daveMueller daveMueller marked this pull request as draft May 3, 2021 14:40
@daveMueller
Copy link
Collaborator Author

I converted this to draft. I look into it again when I have some more free time.

@daveMueller daveMueller deleted the 799_EmptyLine_New branch September 12, 2023 22:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tenet-coverage Issue related to possible incorrect coverage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve coverage for edge cases
2 participants