Determine the anyAttribute Namespace based on the NamespaceList#37409
Determine the anyAttribute Namespace based on the NamespaceList#37409krwq merged 3 commits intodotnet:masterfrom
Conversation
|
|
|
@JosVerburg can you please let me know if you think the failing NETFX tests are related to your changes? |
| [Theory] | ||
| //[Variation(Desc = "complextype Any ns - ##any, attrgroup Any ns2, allow ns2 attribute")] | ||
| [InlineData("##any", "ns2", "ns2", 0)] | ||
| [InlineData("##any", "ns2", "ns2", 0, "##targetNamespace")] |
There was a problem hiding this comment.
since the behavior is now different on full framework and corefx you might solve it in two ways:
- split test for generic tests and corefx specific and put XYZ attribute on corefx specific
- use PlatformDetection.IsFullFramework for those tests (i.e. additional bool arg) and
throw new SkipTestException()
some usage examples:
There was a problem hiding this comment.
Thanks for the pointers, made it easy to fix.
I'll try to look at the failing Linux arm64 tests tomorrow.
…on and union cases
|
@krwq I'm not sure the failing tests are caused by this chance. |
|
/azp run corefx-ci |
|
Commenter does not have sufficient privileges for PR 37409 in repo dotnet/corefx |
|
@JosVerburg sorry, I've been out of town for the last week, I've restarted the CI now. @ViktorHofer should restarting CI be allowed? |
|
We are debating this currently offline in a mail thread. |
| CompareWildcardNamespaces(expectedNs, attributeWildcard.Namespace); | ||
| } | ||
|
|
||
| private static void CompareWildcardNamespaces(string expected, string actual) |
There was a problem hiding this comment.
would it make sense to instead split, order and join back to string and compare strings instead? Alternatively Split, order and just compare IEnumerable with Assert.Equal which I believe should correctly compare elementwise
There was a problem hiding this comment.
Fair point. I've rewritten it to string compares which does improve readability.
There was a problem hiding this comment.
The failing tests are now in System.Net.Sockets.Tests and System.Reflection.Emit.ILGeneration.Tests. I don't believe this can be caused by the last commit on this PR as that only changed a test file.
|
Thanks @JosVerburg! |
…et/corefx#37409) * Determine the anyAttribute Namespace based on the NamespaceList * Do not test the namespace attribute for full framework for intersection and union cases * Compare namespaces using string comparison in unit tests Commit migrated from dotnet/corefx@2c22af8
5 participants
This fixes #36484
I implemented it as a fallback to ensure it doesn't override the private value as it can be set using the public API.