Skip to content

skip ignoring Content-Length test on NodeJs #69109

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.

Sign up for GitHub

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

Jump to bottom
Merged
wfurt merged 2 commits into from
May 10, 2022
Merged

skip ignoring Content-Length test on NodeJs #69109

wfurt merged 2 commits into from
May 10, 2022

Conversation

@pedrobsaila
Copy link
Contributor

@pedrobsaila pedrobsaila commented May 10, 2022

Fixes #69085

@ghost ghost added area-System.Net.Http community-contribution Indicates that the PR has been added by a community member labels May 10, 2022
@ghost
Copy link

ghost commented May 10, 2022

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes #69085

Author: pedrobsaila
Assignees: -
Labels:

area-System.Net.Http
Milestone: -

maraf
maraf reviewed May 10, 2022
View reviewed changes
src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Headers.cs Outdated
}

[Fact]
[SkipOnPlatform(TestPlatforms.Browser, "Content-Length can't be present with Transfer-Encoding")]
Copy link
Member

@maraf maraf May 10, 2022

Choose a reason for hiding this comment

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

Suggested change
[SkipOnPlatform(TestPlatforms.Browser, "Content-Length can't be present with Transfer-Encoding")]
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotNodeJS))]

The test works on browser, we can disable it just for NodeJS.

@pedrobsaila pedrobsaila changed the title skip test on ignoring Content-Length on browser skip test on ignoring Content-Length on NodeJs May 10, 2022
@pedrobsaila pedrobsaila changed the title skip test on ignoring Content-Length on NodeJs skip ignoring Content-Length test on NodeJs May 10, 2022
wfurt
wfurt approved these changes May 10, 2022
View reviewed changes
Copy link
Member

@wfurt wfurt left a comment

Choose a reason for hiding this comment

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

LGTM

@wfurt wfurt merged commit f7e0873 into dotnet:main May 10, 2022
@pedrobsaila pedrobsaila deleted the 69085 branch May 10, 2022 22:37
@ghost ghost locked as resolved and limited conversation to collaborators Jun 10, 2022
@karelz karelz added this to the 7.0.0 milestone Jul 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@maraf maraf maraf left review comments

@wfurt wfurt wfurt approved these changes

Assignees

No one assigned

Labels

area-System.Net.Http community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Milestone

7.0.0

4 participants

@pedrobsaila @maraf @wfurt @karelz