-
Notifications
You must be signed in to change notification settings - Fork 4.9k
Add test for ExpectContinue with no content #30065
Conversation
[Fact] | ||
public async Task GetAsync_ExpectContinueTrue_NoContent_StillSendsHeader() | ||
{ | ||
if (IsNetfxHandler) |
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.
Why use the 'if' instead of SkipOnTargetFramework(...FullFramework) ?
I know that we use IsCurlHandler, IsWinHttpHandler, UseSocketsHttpHandler in the 'if' because we can't use SkipOnTargetFramework for those things. But for this that just skips on NETFX, maybe it would be better to use that?
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.
Makes sense. Will change.
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.
Also, have you tested UAP? It doesn't support Expect: 100-continue. Not sure what it does if you manually put that header on it.
I recently got the UAP tests to run clean so I wouldn't want this new test to regress things. UAP tests don't run in CI right now (only the test source is built). So, you would need to run the tests on your dev box.
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.
I'm not set up to run them right now. Would you prefer I mark it as skipped right now on uwp or submit without that?
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.
How about mark it against my current umbrella UAP test issue?
[ActiveIssue(29802, TargetFrameworkMonikers.Uap)]
* Add test for ExpectContinue with no content * Address PR feedback Commit migrated from dotnet/corefx@3ae01e9
Related to https://github.com/dotnet/corefx/issues/30038
cc: @davidsh