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

Fix System.Configuration.ConfigurationManager.Tests on Android and reenable on non-Windows platforms #56558

Merged
merged 1 commit into from
Jul 30, 2021

Conversation

akoeplinger
Copy link
Member

@akoeplinger akoeplinger commented Jul 29, 2021

The test assembly got changed to $(NetCoreAppCurrent)-windows in 809a06f#diff-4d639cb37fe53cdeae4262c1f5be7936cdd81e3b4f256f9c59ad02ec69b300d9R7 but when talking to Viktor we're not sure why, probably a mistake.

Target $(NetCoreAppCurrent) instead so it runs on all platforms and fix a test issue that occurs on Android due to BaseDirectory not having a trailing slash.
Fixes #37071
Incidentally, that's likely also the reason why the .csproj set TestDisableAppDomain so we can remove that as well.

Also replace TestDisableParallelization with the assembly attribute equivalent which is what we use everywhere else.

…enable on non-Windows platforms

The test assembly got changed to `$(NetCoreAppCurrent)-windows` in dotnet@809a06f#diff-4d639cb37fe53cdeae4262c1f5be7936cdd81e3b4f256f9c59ad02ec69b300d9R7 but when talking to Viktor we're not sure why, probably a mistake.

Target `$(NetCoreAppCurrent)` instead so it runs on all platforms and fix a test issue that occurs on Android due to BaseDirectory not having a trailing slash. Fixes dotnet#37071
Incidentally, that's likely also the reason why the .csproj set `TestDisableAppDomain` so we can remove that as well.

Also replace `TestDisableParallelization` with the assembly attribute equivalent which is what we use everywhere else.
@ghost
Copy link

ghost commented Jul 29, 2021

Tagging subscribers to this area: @safern
See info in area-owners.md if you want to be subscribed.

Issue Details

The test assembly got changed to $(NetCoreAppCurrent)-windows in 809a06f#diff-4d639cb37fe53cdeae4262c1f5be7936cdd81e3b4f256f9c59ad02ec69b300d9R7 but when talking to Viktor we're not sure why, probably a mistake.

Target $(NetCoreAppCurrent) instead so it runs on all platforms and fix a test issue that occurs on Android due to BaseDirectory not having a trailing slash. Fixes #37071
Incidentally, that's likely also the reason why the .csproj set TestDisableAppDomain so we can remove that as well.

Also replace TestDisableParallelization with the assembly attribute equivalent which is what we use everywhere else.

Author: akoeplinger
Assignees: -
Labels:

area-System.Configuration

Milestone: -

@steveisok steveisok self-requested a review July 29, 2021 16:53
Copy link
Member

@ViktorHofer ViktorHofer left a comment

Choose a reason for hiding this comment

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

Thanks

Copy link
Member

@ViktorHofer ViktorHofer left a comment

Choose a reason for hiding this comment

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

Thanks

@akoeplinger akoeplinger merged commit 38af2b8 into dotnet:main Jul 30, 2021
@akoeplinger akoeplinger deleted the sys-configurationmanager branch July 30, 2021 09:06
Copy link
Member

@safern safern left a comment

Choose a reason for hiding this comment

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

LGTM

@ghost ghost locked as resolved and limited conversation to collaborators Aug 29, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

System.Configuration tests fail on Android
4 participants