Empty query params in httpx 0.23.1

[g-as]

encode/httpx#2354

Empty query param is now kept:

def test_empty_query_params():
    q = httpx.QueryParams({"a": ""})
    assert str(q) == "a="

    q = httpx.QueryParams("a=")
    assert str(q) == "a="

    q = httpx.QueryParams("a")
    assert str(q) == "a="

which breaks at least one test:

@pytest.mark.parametrize(
    ("lookup", "params", "url", "expected"),
    [
        ...
        (Lookup.CONTAINS, "x=", "https://foo.bar/?x=1", True),  # query, not params
        ...
    ],
)
def test_params_pattern(lookup, params, url, expected):
    request = httpx.Request("GET", url)
    assert bool(Params(params, lookup=lookup).match(request)) is expected

[lundberg]

Spot on again! 😉

[lundberg]

Not sure if we should keep that test, i.e. keep support and allow matching ?x=1 contains ?x= 🤔 ?

We already have the support for ANY when using params map, e.g. params={"x": ANY}.

Maybe better to be strict now that HTTPX keeps empty value params.

[g-as]

Agreed.

Either drop the test case, or set the expected result to False.

(I edited my initial comment with the actual value from the code (True), since I copied my modifs where I changed it to False)

[g-as]

But dropping it ensures tests pass for all httpx versions if you don't want to add a version constraint on httpx.

[lundberg]

Either drop the test case, or set the expected result to False.

Yeah, I'd say flipping the expected test result to False is best.

But dropping it ensures tests pass for all httpx versions if you don't want to add a version constraint on httpx.

I think it's fine leaving it even though the test suite breaks on this particular test-case on an old httpx version, I mean that specific contains-match is an undocumented feature so I don't think we need to require httpx 0.23.1 for this fix.

[lundberg]

Fixed by #223

[g-as]

Thanks!

[lundberg]

Thanks! ... now released in 0.20.1 🎉

[g-as]

All good!