[clr-ios] Skip XSLT API tests on Apple mobile CoreCLR via ConditionalClass#128049
Open
kotlarmilos wants to merge 6 commits into
Open
[clr-ios] Skip XSLT API tests on Apple mobile CoreCLR via ConditionalClass#128049kotlarmilos wants to merge 6 commits into
kotlarmilos wants to merge 6 commits into
Conversation
…alClass xUnit's [ActiveIssue] does not propagate from a base test class to derived classes, so the [ActiveIssue] on XsltApiTestCaseBase2 (added by dotnet#127464, restored by the revert in dotnet#127947) does not actually skip the ~469 derived tests on iOS/tvOS + CoreCLR. They still run, instantiate the base type, and throw TypeInitializationException from the base static constructor that writes into the read-only app bundle. dotnet#127788 worked around this by duplicating [ActiveIssue] on every derived class; this commit replaces that mechanism with a single [ConditionalClass] on the base class. [ConditionalClass] does propagate to derived classes (as already demonstrated by the pre-existing IsReflectionEmitSupported condition, which silently skips derived classes on NativeAOT). ConditionalClassAttribute has AllowMultiple=false, so the reflection-emit and Apple-mobile-CoreCLR conditions are combined into a single predicate. The predicate is hosted on a sibling type (XsltApiTestRequirements) rather than on XsltApiTestCaseBase2 itself, so that evaluating it does not trigger the dangerous static constructor. See dotnet#124344. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
Tagging subscribers to this area: @dotnet/area-system-xml |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
kotlarmilos
force-pushed
the
clr-ios-xslt-conditional-class
branch
from
de9f189 to
f483175
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the XSLT API test gating in System.Private.Xml to ensure the full suite of derived XSLT API tests is skipped on Apple mobile + CoreCLR, where running them can trigger a failing base-type static constructor that writes into read-only app bundle test data.
Changes:
- Introduces a dedicated
XsltApiTestRequirements.IsSupportedpredicate to centralize skip logic without touchingXsltApiTestCaseBase2’s type initializer. - Replaces the base-class
[ConditionalClass(...IsReflectionEmitSupported...)]+[ActiveIssue(...)]pairing with a single[ConditionalClass(...IsSupported...)]that is intended to apply to all derived test classes.
Show a summary per file
| File | Description |
|---|---|
| src/libraries/System.Private.Xml/tests/Xslt/XslCompiledTransformApi/XsltApiV2.cs | Consolidates XSLT API test skip conditions into a single ConditionalClass predicate to skip derived classes on Apple mobile CoreCLR. |
Copilot's findings
Comments suppressed due to low confidence (1)
src/libraries/System.Private.Xml/tests/Xslt/XslCompiledTransformApi/XsltApiV2.cs:55
- Removing the [ActiveIssue("https://github.com//issues/124344", ...)] annotation means these skipped tests will no longer be discoverable via the tracking workflow described in the issue (grep for ActiveIssue with that URL). Consider keeping the ActiveIssue attribute on this base type for tracking/metadata (even if it doesn’t control skipping), and rely on ConditionalClass for the actual skip behavior.
public string szDefaultNS = "urn:my-object";
public string szEmpty = "";
- Files reviewed: 1/1 changed files
- Comments generated: 0
Member
Author
|
/azp run runtime-extra-platforms |
|
Azure Pipelines successfully started running 1 pipeline(s). |
This was referenced May 12, 2026
Open
Open
jkotas
reviewed
May 12, 2026
xUnit trait discovery does not walk the inheritance chain, so neither [ActiveIssue] nor [ConditionalClass] on the base class actually skips derived test classes. Replicate the new combined condition on every derived class, replacing the existing reflection-emit-only ConditionalClass in place where present (the new IsSupported predicate already requires it). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jkotas
reviewed
May 12, 2026
…rmApi/XsltApiV2.cs Co-authored-by: Jan Kotas <jkotas@microsoft.com>
Member
Author
|
/azp run runtime-extra-platforms |
|
Azure Pipelines successfully started running 1 pipeline(s). |
jkotas
reviewed
May 12, 2026
…rmApi/XsltApiV2.cs Co-authored-by: Jan Kotas <jkotas@microsoft.com>
This was referenced May 12, 2026
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Member
Author
|
/azp run runtime-extra-platforms |
|
Azure Pipelines successfully started running 1 pipeline(s). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR uses
[ConditionalClass], which does propagate (the existingIsReflectionEmitSupportedcondition already relies on that) unlike #127947.Tracked under #124344.
Fixes #128049