Skip to content

Conversation

@Yoticc
Copy link
Contributor

@Yoticc Yoticc commented Aug 5, 2025

Contributing towards #118321

Added a check for the framework version to avoid a test that is known in advance to fail for the .netstandard target.

Regarding the additional condition check besides ActiveIssue, I noticed that xunit-console does not recognize the attribute and launches the test. Therefore, a separate condition check is required.

@github-actions github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Aug 5, 2025
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Aug 5, 2025
@kasperk81
Copy link
Contributor

[System.Management] Skip DateTime_RoundTrip test for corefx implementation

corefx is the name of .NET Core Libraries, you want to say .NET Framework implementation

Regarding the additional condition check besides ActiveIssue, I noticed that xunit-console does not recognize the attribute and launches the test. Therefore, a separate condition check is required.

what about the other 2k usages of [ActiveIssue] in the repo? you shouldn't care for it in this single instance

@Yoticc
Copy link
Contributor Author

Yoticc commented Aug 5, 2025

corefx is the name of .NET Core Libraries, you want to say .NET Framework implementation

You are right, it is worth changing the term "corefx" to ".NET Framework". I was mistaken in my comment. Initially, it pointed to the root of the issue, which lay in corefx repository.

what about the other 2k usages of [ActiveIssue] in the repo? you shouldn't care for it in this single instance

I believe this is a good practice. Removing the ActiveIssueAttribute and leaving the exception seems like a bad idea because it leaves no reference to the issue.
Rather, the question is how appropriate it is to mark "permanent issue for unsupported target" using ActiveIssue, which is intended for temporary test skipping. It would be helpful to hear the member's opinion on the best practice.
Perhaps it should be replaced with a simple comment containing the issue ID or link.

Yoticc added a commit to Yoticc/runtime that referenced this pull request Aug 5, 2025
@kasperk81
Copy link
Contributor

I believe this is a good practice. Removing the ActiveIssueAttribute and leaving the exception seems like a bad idea because it leaves no reference to the issue.

i was suggesting to do the opposite, remove the excessive condition

@dotnet-policy-service
Copy link
Contributor

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

@jeffhandley jeffhandley merged commit 8b172df into dotnet:main Sep 1, 2025
87 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Oct 2, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-System.Management community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants