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

Improve Encoding Fallback Tests (34.9%, 6.2%) #20285

Open
benaadams opened this issue Feb 21, 2017 · 10 comments
Open

Improve Encoding Fallback Tests (34.9%, 6.2%) #20285

benaadams opened this issue Feb 21, 2017 · 10 comments
Labels
area-System.Text.Encoding good first issue Issue should be easy to implement, good for first-time contributors increase-code-coverage Tracking need to add more test and increase code coverage of a component test-enhancement Improvements of test source code
Milestone

Comments

@benaadams
Copy link
Member

Tracking issue

Encoding Fallback issue was detected via CI failure in XML tests rather than via Encoding tests

see: dotnet/corefx#16252 (comment)

/cc @danmosemsft

@benaadams
Copy link
Member Author

Code coverage for
DecoderFallbackBufferHelper 29.8% (16.6% branch)
EncoderFallbackBufferHelper 34.9% (6.2% branch)

@benaadams benaadams changed the title Improve Encoding Fallback Tests Improve Encoding Fallback Tests (34.9%) Feb 21, 2017
@benaadams benaadams changed the title Improve Encoding Fallback Tests (34.9%) Improve Encoding Fallback Tests (34.9%/6.2%) Feb 21, 2017
@benaadams benaadams changed the title Improve Encoding Fallback Tests (34.9%/6.2%) Improve Encoding Fallback Tests (34.9%, 6.2%) Feb 21, 2017
@tarekgh
Copy link
Member

tarekgh commented Feb 21, 2017

@benaadams are you willing to help with this one too?

@benaadams
Copy link
Member Author

Encoding has burnt me twice; might have to lick my wounds for a little while... Though I do still want it to be faster; but may be a little while.

@benaadams
Copy link
Member Author

3rd time I'd have to get it perfect 😉

@tarekgh
Copy link
Member

tarekgh commented Feb 21, 2017

when you look at it again we need to consider pick up the optimization from the corefxlab with it. also I am wondering why you didn't see Xml failure in your tests? didn't these tests run in your case?

@benaadams
Copy link
Member Author

I normally end up with quite a few test failures in unrelated areas (without any changes) and testing coreclr against corefx can be periodically hit and miss as things change.

So I confirm the copy dlls works with current (at least in area); add a definite mistake (in area) to confirm that changes are picked up and failure happens; then test against new changes and ensure everything passes.

Probably should start raising issues for "unrelated" test failures I see and pay more attention to them...

@benaadams
Copy link
Member Author

Be nice if there was an automated dotnetbot confirm with corefx in coreclr https://github.com/dotnet/coreclr/issues/9715

@danmoseley
Copy link
Member

Info on code coverage work:

  1. Here's the all up report
    https://ci.dot.net/job/dotnet_corefx/job/master/job/code_coverage_windows/Code_Coverage_Report/
    Caveat - doesn't show coverage of any types whose implementation is in corelib (eg String, etc). If you want to look at that for now you must gather manually using instructions below.

  2. Docs on code coverage work are here
    https://github.com/dotnet/corefx/blob/master/Documentation/building/code-coverage.md
    and some in
    https://github.com/dotnet/corefx/blob/master/Documentation/project-docs/developer-guide.md

  3. There's innumerable places to add coverage -- we would love to focus on the most heavily used types and the least covered types to get the most benefit.

@benaadams
Copy link
Member Author

The fallback handling is pretty weird and could be cleaned up a lot; however I don't have a 100% handle on what its doing (clearly 😛 )

dotnet-bot referenced this issue in dotnet/corefx Feb 15, 2018
* String Create options overlaod

* Minor Change

* Feedback

* validation on options

* obsolete removed

* Implementing Iserializable and removing ignorecase

* HashCode and serialization changes

* made inline

* Space Corrected

Signed-off-by: dotnet-bot-corefx-mirror <[email protected]>
stephentoub referenced this issue in dotnet/corefx Feb 15, 2018
* String Create options overlaod

* Minor Change

* Feedback

* validation on options

* obsolete removed

* Implementing Iserializable and removing ignorecase

* HashCode and serialization changes

* made inline

* Space Corrected

Signed-off-by: dotnet-bot-corefx-mirror <[email protected]>
A-And referenced this issue in A-And/corefx Feb 21, 2018
* String Create options overlaod

* Minor Change

* Feedback

* validation on options

* obsolete removed

* Implementing Iserializable and removing ignorecase

* HashCode and serialization changes

* made inline

* Space Corrected

Signed-off-by: dotnet-bot-corefx-mirror <[email protected]>
@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the Future milestone Jan 31, 2020
@maryamariyan maryamariyan added the untriaged New issue has not been triaged by the area owner label Feb 23, 2020
@GrabYourPitchforks
Copy link
Member

@layomia Can we get new numbers for the current state of 5.0 master? Is this work already largely done?

@tarekgh tarekgh removed the untriaged New issue has not been triaged by the area owner label Jul 6, 2020
@layomia layomia removed their assignment Nov 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Text.Encoding good first issue Issue should be easy to implement, good for first-time contributors increase-code-coverage Tracking need to add more test and increase code coverage of a component test-enhancement Improvements of test source code
Projects
None yet
Development

No branches or pull requests

7 participants