Skip to content

Fix facade reference and unskip unit test#3098

Merged
RussKie merged 1 commit into
dotnet:masterfrom
weltkante:issue2413
Apr 21, 2020
Merged

Fix facade reference and unskip unit test#3098
RussKie merged 1 commit into
dotnet:masterfrom
weltkante:issue2413

Conversation

@weltkante
Copy link
Copy Markdown
Contributor

@weltkante weltkante commented Apr 20, 2020

Fixes #2413

Proposed changes

Add reference to System.Drawing facade in order to be able to unit-test it. If you don't do this you inherit the System.Drawing from shared framework Microsoft.NetCore.App which does not contain the same redirects as the WinForms version, in particular the UITypeEditor redirect is missing.

Open questions

  • In the issue discussion it was noted that WinForms should not update the facade versions to 5.0 but instead keep them at 4.0 like the shared framework does.

    • The facades don't contain any code, they'll always be a redirect of the Desktop Version for compatibility and not get any new stuff
    • WinForms and the shared framework should agree about versioning, if the base framework doesn't level its facades for 5.0 neither should do WinForms for facades its replacing.

    The versioning problem does not prevent fixing the unit tests so it could be split off into a separate issue

  • The PR contains a workaround for deps.json for ProjectReferences is not emitted correctly sdk#3254 - will create a follow-up issue to track removal of the workaround unless something better can be figured out

Customer Impact

as is the PR has no customer impact

once we roll back the 5.0 versioning of System.Drawing and other facades this change is visible to users who already compiled against a 5.0 preview

Regression?

unclear, probably not (the regression part of not redirecting UITypeEditor should already have been fixed, we just couldn't test it)

Risk

low, as-is the PR only affects unit tests

once we roll back the 5.0 versioning of System.Drawing and other facades there may be some risk of breaking people who already have been compiling against earlier 5.0 previews and built nuget packages and stuff, but thats probably acceptable since its a preview?

Test methodology

skipped unit test now passes, test is now able to confirm what already was present in the product

Microsoft Reviewers: Open in CodeFlow

@weltkante weltkante requested a review from a team as a code owner April 20, 2020 18:09
@ghost ghost assigned weltkante Apr 20, 2020
@RussKie
Copy link
Copy Markdown
Contributor

RussKie commented Apr 21, 2020

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@RussKie RussKie added the test-bug Problem in test source code (most likely) label Apr 21, 2020
@RussKie RussKie merged commit 86c3519 into dotnet:master Apr 21, 2020
@ghost ghost added this to the 5.0 Preview5 milestone Apr 21, 2020
@RussKie
Copy link
Copy Markdown
Contributor

RussKie commented Apr 21, 2020

Thank you

@weltkante weltkante deleted the issue2413 branch April 29, 2020 17:54
@ghost ghost locked as resolved and limited conversation to collaborators Feb 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

test-bug Problem in test source code (most likely)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

UITypeEditor does not type-forward from System.Drawing

2 participants