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

xunit warnings: Non-serializable data #30324

Open
ViktorHofer opened this issue Jul 21, 2019 · 7 comments
Open

xunit warnings: Non-serializable data #30324

ViktorHofer opened this issue Jul 21, 2019 · 7 comments
Labels
area-Meta test-bug Problem in test source code (most likely)
Milestone

Comments

@ViktorHofer
Copy link
Member

We are currently hiding thousands of warnings that indicate that our test data is not serializable. See aspnet/SignalR#1649 for more details.

Switching from xunit.console to dotnet test/vstest displays those warnings by default. I think we should at least investigate if the warnings are valid and consider fixing them.

Test branch with dotnet vstest for local validation: https://github.com/ViktorHofer/corefx/tree/DotNetTest
Related: dotnet/arcade#3333

Attaching the xunit log from System.Text.RegularExpressions.Tests: xunit-warnings.txt

cc @danmosemsft @safern

@ViktorHofer
Copy link
Member Author

ViktorHofer commented Jul 21, 2019

XUnit unfortunately doesn't support theory member data serialization which means you need to implement IXunitSerializable to make that work: xunit/xunit#429 (comment)

This is really unfortunate and I'm surprised that there isn't a solution for that yet. Maybe just switching off the diagnostics warnings (if possible) is easier... The downside, you won't be able to debug individual test cases in VS Test Explorer.

@ViktorHofer
Copy link
Member Author

Submitted dotnet/arcade#3334

@ViktorHofer
Copy link
Member Author

What aspnet did was to just ignore those warnings... I'm not exactly sure how we should proceed here. This is a feature omission in xunit and should be fixed there but I don't think this will happen anytime soon. When switching to dotnet vstest those warnings will shop up and can only be turned off by also turning-off diagnostic messages which are needed for diagnosing long-running tests.

cc @stephentoub for opinions

@stephentoub
Copy link
Member

stephentoub commented Jul 21, 2019

XUnit unfortunately doesn't support theory member data serialization which means you need to implement IXunitSerializable to make that work

We use a bunch of types in theories that will never be serializable. I don't see xunit being able to address that fully, nor should we bend over backwards to make theory data serializable in order to have individual inputs show up in VS Test explorer; it's simply not worth it.

When switching to dotnet vstest those warnings will shop up and can only be turned off by also turning-off diagnostic messages which are needed for diagnosing long-running tests.

We can't suppress individual warning types? That's the feature I'd want from xunit if it doesn't already exist, ala being able to suppress C# compiler warning X but not Y.

@ViktorHofer
Copy link
Member Author

We can't suppress individual warning types?

Unfortunately not. Currently it's either all diagnostics or none. I'm not sure if contributions are accepted at the moment given the staleness of the repo. The last official release was 9+ months ago.

@stephentoub
Copy link
Member

Currently it's either all diagnostics or none

@bradwilson, would xunit welcome a PR to address this in some way?

@bradwilson
Copy link

Yes, with a couple obvious caveats:

  1. We don't currently have message ID/categorization, so we would need to create that;
  2. We don't have any plans to ship any more v2 builds, which means the soonest you'd see this is when I'm able to ship a v3 (and when you'd be able/willing to move to it).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Meta test-bug Problem in test source code (most likely)
Projects
No open projects
Development

No branches or pull requests

6 participants