-
Notifications
You must be signed in to change notification settings - Fork 4.1k
Update xunit #76196
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
Update xunit #76196
Conversation
@jaredpar The updated versions are not present in the internal feeds. Would you consider adding them and update? My goal here is to catch up early with updates just in case there are new analyzer warnings that need to be addressed and keeping up with newer xunit versions to avoid having a future "large" jump in versions. Smaller jumps are usually a lot easier. Note that when xunit v3 is stable, it may be really good to update to it, and even switching to Microsoft.Testing.Platform instead of VSTest. |
/azp run |
Azure Pipelines successfully started running 3 pipeline(s). |
@Youssef1313 looks like real build ambiguities |
@@ -65,7 +65,7 @@ | |||
await Console.Out.WriteLineAsync($"Discovering tests in {testDescriptor}...").ConfigureAwait(false); | |||
|
|||
using var xunit = new XunitFrontController(AppDomainSupport.IfAvailable, assemblyFileName, shadowCopy: false); | |||
var configuration = ConfigReader.Load(assemblyFileName); | |||
var configuration = ConfigReader.Load(assemblyFileName, configFileName: null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For reference: A new overload was added that also has optional parameter, causing ambiguity.
@@ -28,7 +29,7 @@ public static TheoryData<LanguageVersion> LangVersions() | |||
} | |||
|
|||
private sealed class CombinatorialLangVersions() | |||
: CombinatorialValuesAttribute(LangVersions().Select(d => d.Single()).ToArray()); | |||
: CombinatorialValuesAttribute(((IEnumerable<object[]>)LangVersions()).Select(d => d.Single()).ToArray()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For reference: Previously TheoryData<T>
was an IEnumerable<object[]>
, but now it's both IEnumerable<object[]>
and IEnumerable<T>
, causing Linq's Select to not be resolved properly without the cast.
Thanks @jaredpar. Let's see now maybe. |
@jaredpar Are you interested in fixing https://xunit.net/xunit.analyzers/rules/xUnit2031, or do you think it should be suppressed instead? There is not much violations. |
I pushed a fix for now. If you prefer to disable xUnit2031, let me know and I'll revert the changes and disable the analyzer. |
I'm fine with a fix here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM Thanks (iteration 4)
@Youssef1313 I'm looking forward to xunit v3. I think it adds a "PrintMaxStringLength" setting which lets us change how to xunit truncates actual and expected strings in its output. The default is 50 characters, which is often annoying... |
@jcouv The current main issue for upgrading to xunit.v3 is the integration test harness. I started some work in microsoft/vs-extension-testing#179 but so far not yet finished. The migration is much more involving that I thought, as you see by the size of that unfinished PR. I'm also hoping that at the end, Roslyn can migrate to Microsoft.Testing.Platform. |
@Youssef1313 By any chance, is there a way for us to update to xUnit v3 without switching to Microsoft.Testing.Platform? |
@jcouv Yes. It's vice-versa. The first step is updating to xUnit v3 (which isn't an easy step), then the second step is to update to MTP. But if the update to xUnit v3 happened, I'd strongly recommend that you update to MTP after. (We can have a chat about that internally if you want) |
No description provided.