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

X509Certificates.Tests: Non-serializable data ('System.Object[]') #35807

Closed
vcsjones opened this issue May 4, 2020 · 10 comments · Fixed by #35837
Closed

X509Certificates.Tests: Non-serializable data ('System.Object[]') #35807

vcsjones opened this issue May 4, 2020 · 10 comments · Fixed by #35837
Labels
area-Infrastructure-libraries untriaged New issue has not been triaged by the area owner

Comments

@vcsjones
Copy link
Member

vcsjones commented May 4, 2020

Running tests in X509Certificates.Tests gives a few test warnings, I think uncovered when #35285 was merged:

[xUnit.net 00:00:00.72] System.Security.Cryptography.X509Certificates.Tests: Non-serializable data ('System.Object[]') found for 'System.Security.Cryptography.X509Certificates.Test
s.X500DistinguishedNameEncodingTests.EncodeSingleRdn'; falling back to single test case.
[xUnit.net 00:00:00.72] System.Security.Cryptography.X509Certificates.Tests: Non-serializable data ('System.Object[]') found for 'System.Security.Cryptography.X509Certificates.Test
s.X500DistinguishedNameEncodingTests.EncodeWithFlags'; falling back to single test case.
[xUnit.net 00:00:00.72] System.Security.Cryptography.X509Certificates.Tests: Non-serializable data ('System.Object[]') found for 'System.Security.Cryptography.X509Certificates.Test
s.X500DistinguishedNameEncodingTests.CheckParserBoundaryCases'; falling back to single test case.
[xUnit.net 00:00:00.80] System.Security.Cryptography.X509Certificates.Tests: Non-serializable data ('System.Object[]') found for 'System.Security.Cryptography.X509Certificates.Test
s.CertificateCreation.ECDsaX509SignatureGeneratorTests.PublicKeyEncoding'; falling back to single test case.

This leads me to believe that there are theories which, when MemberData returns nonserializable data.

What is the impact of this? Does this mean only a single test case is being run and coverage isn't happening, or does it mean that it "looks" like a single test but is actually running all of the cases?

/cc @ViktorHofer

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-System.Security untriaged New issue has not been triaged by the area owner labels May 4, 2020
@ghost
Copy link

ghost commented May 4, 2020

Tagging subscribers to this area: @bartonjs, @vcsjones, @krwq
Notify danmosemsft if you want to be subscribed.

@ViktorHofer
Copy link
Member

See #30324 for context. No easy way to disable these messages besides disabling diagnostic messages or changing the log verbosity to quiet.

@ghost
Copy link

ghost commented May 4, 2020

Tagging subscribers to this area: @safern, @ViktorHofer
Notify danmosemsft if you want to be subscribed.

@vcsjones
Copy link
Member Author

vcsjones commented May 4, 2020

@ViktorHofer thanks; I missed that issue. If you'd prefer to close this as a dupe of that one; I think that is a reasonable thing to do.

Should we be fixing tests so that member data uses serializable types or waiting on a change to the build infrastructure?

@ViktorHofer
Copy link
Member

ViktorHofer commented May 4, 2020

If my memory serves me right, @stephentoub was against fixing all these cases as there would be thousands of such and it would bloat the test code. Actually I attempted doing that when I saw the warnings: dotnet/arcade#3334. I'm not sure what the best option going forward is...

@bartonjs
Copy link
Member

bartonjs commented May 4, 2020

I'm not sure what it's complaining about. The cases print correctly in the XML output and the CLI output, and definitely work (or, at least all of this was true when the tests were written...).

Maybe the VS runner serializes the data and therefore it needs to be serializable in order to print nicely in the VS test explorer? Dunno if that means recent infrastructure changes dropped these tests, or not. Easiest answer is to break one of the expected values and see if it fails 😄.

@ViktorHofer
Copy link
Member

Yes this is purely for VS Test Explorer support to run/debug single theory test cases. It doesn't affect execution itself.

@vcsjones
Copy link
Member Author

vcsjones commented May 4, 2020

@bartonjs it seems to work. In X509Certificates there are only a handful of these messages. In S.S.C.Algorithms there is approximately 100 of them.

There are other messages in S.S.C.Algorithms that might be worth looking in to, like:

Skipping test case with duplicate ID '658dd5afec16f3397ac5ef6b298eb8b71ae5a86f' ('System.Security.Cryptograph
y.Hashing.Algorithms.Tests.BlockSizeValueTests.BlockSizeValueTest(hmacBlockSizeValue: 64, expectedBlockSizeValue: 64)' and 'System.Security.Cryptography.Hashing.Algorithms.Tests.BlockSizeValueTests.BlockSizeValueTest(hmacBlockSizeValue: 64, expectedBlockSizeValue: 64)')

Perhaps we aren't permuting something correctly.

@stephentoub
Copy link
Member

After #35837, these warnings should go away.

@vcsjones
Copy link
Member Author

vcsjones commented May 5, 2020

Okay, thanks @stephentoub. I think then it makes sense to close this.

@vcsjones vcsjones closed this as completed May 5, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Infrastructure-libraries untriaged New issue has not been triaged by the area owner
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants