Skip to content

Comments

Use ConditionalFact/ConditionalTheory ctor that passes type#124791

Open
akoeplinger wants to merge 4 commits intodotnet:mainfrom
akoeplinger:conditionalfact-typeof
Open

Use ConditionalFact/ConditionalTheory ctor that passes type#124791
akoeplinger wants to merge 4 commits intodotnet:mainfrom
akoeplinger:conditionalfact-typeof

Conversation

@akoeplinger
Copy link
Member

@akoeplinger akoeplinger commented Feb 24, 2026

Required for xunit v3 and the other ctor is marked obsolete as of dotnet/arcade#16537

Copilot AI review requested due to automatic review settings February 24, 2026 12:42
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot wasn't able to review this pull request. There are 300 or more changed files, try reducing the number of files in this pull request and requesting a review from Copilot again.

@stephentoub
Copy link
Member

The only thing we can't do in the new system is ConditionalTheory with only a member name [..] It looks like there are relatively few in use

Pan to me looking at PR with 306 files changed...

@akoeplinger
Copy link
Member Author

Pan to me looking at PR with 306 files changed...

lol yeah. not sure if @agocke missed something but at least this was a relatively painless edit with copilot. only the generic cases are a little painful (last commit)...

@agocke
Copy link
Member

agocke commented Feb 24, 2026

Yeah I was expecting ~50 changes, clearly my query missed something

@agocke
Copy link
Member

agocke commented Feb 24, 2026

I would look into an alternative but @akoeplinger has already done the work (thanks!).

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot wasn't able to review this pull request. There are 300 or more changed files, try reducing the number of files in this pull request and requesting a review from Copilot again.

@akoeplinger
Copy link
Member Author

yeah it seems to work, not sure how/if you want to review it though :)

@agocke
Copy link
Member

agocke commented Feb 25, 2026

I’ll look through it tonight, I don’t mind reviewing mechanical prs

This was hidden before because ConditionalFact just skipped the test in that case, but we now call this as part of the test itself in SmtpClientAuthTest.TestGssapiAuthentication().
Copy the try-catch from NegotiateAuthenticationPal.Unix.cs to fix it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants