Upgrade to xunit 2.3.0-beta4 - #258
Conversation
There was a problem hiding this comment.
Why make changes like this now? Shouldn't this file stay mostly unchanged until the repo is switched to lineups?
There was a problem hiding this comment.
The lineup PR is already queued in my local changes. This one bled over from it as I'm doing the work as incrementally as possible. I can remove it for the sake of purity.
There was a problem hiding this comment.
This is what was triggering the analyzer exception. They already have a PR to fix it: xunit/xunit.analyzers#77, but it's easy for us to just workaround it by using braces instead.
There was a problem hiding this comment.
FWIW - most of these are just warnings, like this one. But because we have TreatWarningsAsErrors on, we have to fix or suppress.
There was a problem hiding this comment.
Hi @mikeharder. Glad to still be causing problems for .Net, even if indirectly :)
There was a problem hiding this comment.
Hi @marcind. The problems were caused by us, you're helping fix them :).
There was a problem hiding this comment.
Why suppress instead of removing the unused parameter?
There was a problem hiding this comment.
Removing the parameter causes xunit to fail at runtime:
System.InvalidOperationException : The test method expected 1 parameter value, but 2 parameter values were provided.
There was a problem hiding this comment.
Of course the shape of the MemberData needs to match the parameters, but I think it would be cleaner to remove the expected values from the member data, rather than supressing the xunit warning. There is already a FilledStringValues TheoryData, but it has different keys than FilledStringValuesWithExpected.
There was a problem hiding this comment.
Ok, I've remove it. We have other member data that provide the same test coverage, FilledStringValues, DefaultOrNullStringValues, and EmptyStringValues
ghost
locked as resolved and limited conversation to collaborators
4 participants
Required a few fixed because of new issues discovered by xunit.analyzers.
https://github.com/aspnet/Coherence-Signed/issues/652