Make SimpleTestCase.assertRedirects() URL comparison ignore ordering of query parameters

[Cédric Codet]

When checking for a url redirection in tests with assertRedirects, the order of redirection url query parameters matters (since it is matched with expected_url string argument).

Example: if the expected url is example.com/?foo=1&bar=2, example.com/?bar=2&foo=1 is not matched (while example.com/?foo=1&bar=2 is)

This is an issue since Django does not enforce query parameters ordering itself

[Tim Graham]

Seems useful as Django's test suite has ​a workaround for this issue. I'm not sure if we need a way to toggle the behavior -- always ignoring ordering seems fine unless anyone can think of a use case where it would be problematic?

[Jan Pieter Waagmeester]

Patch adding normalization to the url/expected_url prior to self.assertEqual

[Jan Pieter Waagmeester]

Just attached a patch which fixes this issue, if the general direction of this fix is considered the way to go, I am willing to open a pull request on Github.

Do we need the strict_query_order param to provide a way to fall back to current behavior?

[Claude Paroz]

Please make the PR.
I wouldn't keep the strict_query_order param, unless we can find a reference stating that param ordering in query string is significant and kept.

[Jan Pieter Waagmeester]

Pull request: ​https://github.com/django/django/pull/9480

[Simon Charette]

I'm not sure if this is relevant here but Django makes a distinction between ?foo=1&foo=2 and ?foo=2&foo=1 so we might have to make the strict_query_order parameter default to True?

[Jan Pieter Waagmeester]

I just pushed a commit allowing multiple values for a key in the redirect_view test view. Indeed this allows reordering foo=1&foo=2 to foo=2&foo=1 in assertRedirects without failures.

There are currently no tests to test that.

There is also ​AuthViewTestCase.assertURLEqual() which compares two QueryDict objects, which ensures order for multiple values is checked.

We can move the assertUrlRedirects method to SimpleTestCase and use that method (or a similar implementation) to do the final url equality assertion in SimpleTestCase.assertRedirects().

[Carlton Gibson]

Hi Jan,

Thanks for your effort here!

I think the points you make in your last comment are all correct.

The first step is adding the test cases to cover the foo=1&foo=2 vs foo=2&foo=1 example. These are indeed different URLs.

You have foo=1&bar=1 vs bar=1&foo=1 already. Are there other examples we need to cover?

Excepting the multi-value case, I don't see order being important. (Anyone?)

I think your proposal to move and reuse assertURLEqual() method seems very sensible. (Going via QueryDict should be more reliable that trying to normalise the URL by-hand.)

[Jan Pieter Waagmeester]

I just pushed a commit splitting of assertURLRedirects(), and included some extra tests.

I tried using QueryDict, but they preserve their creation order when .urlencode() is called and thus do not allow re-creating the url. I like to do that because it simplifies the output, but if you still think the way it was done in the implementation I removed, I'll update my implementation to look more like that one.

Do we need to add tests checking the message produced if the assertion fails?

[Jan Pieter Waagmeester]

I just pushed a new commit:

• documentation for the new assertion to docs/topics/testing/tools.txt
• Wrapped some docsgtrings to 79 chars.
• Added a changelog entry

[Tim Graham <timograham@…>]

In 24959e48:

Fixed #27398 -- Added an assertion to compare URLs, ignoring the order of their query strings.

[Tim Graham <timograham@…>]

In 5d98d53:

Refs #27398 -- Simplified some tests with assertRedirects().