Skip to content
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

Disable TestClient follow_redirects by default #1872

Closed
wants to merge 1 commit into from

Conversation

aminalaee
Copy link
Member

As discussed in #1871

In the migration of TestClient from requests to httpx in #1376 we enabled follow_redirects in httpx by default. I think this was a side effect of that PR and was not on purpose.

There are two points to disable it by default:

  • This was not enabled by default before, even though it's not a major breaking change, it can still break some code and it was not planned for.
  • In httpx this is also disabled by default so if you have both async and sync tests, you have to also enable it for AsyncClient to get the same behaviour of TestClient.

There's a sample code in #1871 if needed.

@aminalaee aminalaee changed the title Avoid TestClient follow_redirects by default Disable TestClient follow_redirects by default Sep 27, 2022
@aminalaee aminalaee marked this pull request as draft September 27, 2022 09:24
@aminalaee
Copy link
Member Author

aminalaee commented Sep 27, 2022

@Kludex Not entirely sure but seems like this change only was for POST requests and now we are in a better position compared to Starlette 0.20.1:

def test_sync():
    client = TestClient(app)

    response = client.get("/bar")
    assert response.status_code == 200
    assert response.url == "http://testserver/foo"

    # Starlette 0.21.0
    response = client.post("/bar")
    assert response.url == "http://testserver/foo"
    assert response.status_code == 200

    # Starlette 0.20.1
    response = client.post("/bar")
    assert response.url == "http://testserver/bar"
    assert response.status_code == 302. # Not following redirects for POST but it does for GET

Even though this is different from httpx AsyncClient, I think it's at least behaving the same way for both POST and GET.
I'll spend some time on it and close it if I have nothing to add.

@euri10
Copy link
Member

euri10 commented Sep 27, 2022

unless I'm mistaken that's one of the difference between requests and httpx.

requests follows redirects by default, httpx doesnt

so unless the previous TestClient was overriding requests default behaviour (or requests in fact does not follow redirects by default, which means this part in httpx docs should then be corrected: https://www.python-httpx.org/compatibility/) it's fair to say the new TestClient should force the flag to True.

@aminalaee
Copy link
Member Author

@euri10 yeah looks like it’s ok.
I was only seeing an edge case that POST request with httpx TestClient follows redirects but previously with requestst it didn’t follow.
But both now and in the past they followed redurrcts for GET requests. I have tried to do a minimal example.

if I understand it correctly it’s actually even better now so I will close it.

@aminalaee aminalaee closed this Sep 27, 2022
@Kludex Kludex deleted the avoid-testclient-follow-redirects branch September 27, 2022 14:34
@Kludex
Copy link
Sponsor Member

Kludex commented Sep 27, 2022

Thanks @aminalaee 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants