Skip to content

Fix detection of unsupported links #58391

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.

Sign up for GitHub

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

Jump to bottom
Merged
jozkee merged 3 commits into from
Aug 31, 2021
Merged

Fix detection of unsupported links #58391

jozkee merged 3 commits into from
Aug 31, 2021

Conversation

@jozkee
Copy link
Member

@jozkee jozkee commented Aug 30, 2021

Addresses #57575 (comment).

@jozkee jozkee added this to the 7.0.0 milestone Aug 30, 2021
@jozkee jozkee self-assigned this Aug 30, 2021
@ghost
Copy link

ghost commented Aug 30, 2021

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

Issue Details

Addresses #57575 (comment).

Author: Jozkee
Assignees: Jozkee
Labels:

area-System.IO
Milestone: 7.0.0

@jozkee
Copy link
Member Author

jozkee commented Aug 30, 2021

@danmoseley can we backport this to RC2? It was an overlook that should've been fixed in #57996.

carlossanlop
carlossanlop approved these changes Aug 30, 2021
View reviewed changes
Copy link
Contributor

@carlossanlop carlossanlop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, modulo my question.

@jozkee
Copy link
Member Author

jozkee commented Aug 30, 2021

/backport to release/6.0

@github-actions
Copy link
Contributor

github-actions bot commented Aug 30, 2021

Started backporting to release/6.0: https://github.com/dotnet/runtime/actions/runs/1183820854

@github-actions github-actions bot mentioned this pull request Aug 30, 2021
Merged
@danmoseley
Copy link
Member

danmoseley commented Aug 30, 2021

Obviously this is a good time to see whether you can think of any more missing tests in this area, or other possible similar defects we might have.

@jozkee
Copy link
Member Author

jozkee commented Aug 30, 2021 •
edited
Loading

@danmoseley I ran the coverage report (dotnet build .\src\libraries\System.IO.FileSystem\tests /t:test /p:coverage=true) but seems that it cannot link the product source with the tests coverage:

image

Opposed to another project (SystemText.Json), where you can see how much source code is being covered:

image

The only difference between both projects is that System.IO code lives in System.Private.CoreLib, I wonder if that's the reason why it doesn't work the same way.
Will try to think of any other ways to detect uncovered code-paths.

EDIT: This is a known issue #51058.

@jozkee
Copy link
Member Author

jozkee commented Aug 30, 2021

CI failure seems unrelated (GC tests failed), filed #58409.

@jozkee jozkee merged commit de6b1e9 into dotnet:main Aug 31, 2021
@jozkee jozkee deleted the fs_links branch August 31, 2021 00:20
@carlossanlop carlossanlop mentioned this pull request Sep 2, 2021
Closed
@ghost ghost locked as resolved and limited conversation to collaborators Sep 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@danmoseley danmoseley danmoseley left review comments

+1 more reviewer

@carlossanlop carlossanlop carlossanlop approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@jozkee jozkee

Labels

area-System.IO

Projects

None yet

Milestone

7.0.0

Development

Successfully merging this pull request may close these issues.

3 participants

@jozkee @danmoseley @carlossanlop