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

6 files changed, 95 insertions, 25 deletions

diff --git a/django/test/testcases.py b/django/test/testcases.py
index 1fac9ef931..488b8e2cd7 100644
--- a/django/test/testcases.py
+++ b/django/test/testcases.py
@@ -9,7 +9,9 @@ from contextlib import contextmanager
 from copy import copy
 from functools import wraps
 from unittest.util import safe_repr
-from urllib.parse import unquote, urljoin, urlparse, urlsplit
+from urllib.parse import (
+    parse_qsl, unquote, urlencode, urljoin, urlparse, urlsplit, urlunparse,
+)
 from urllib.request import url2pathname
 
 from django.apps import apps
@@ -313,11 +315,30 @@ class SimpleTestCase(unittest.TestCase):
                     % (path, redirect_response.status_code, target_status_code)
                 )
 
-        self.assertEqual(
+        self.assertURLEqual(
             url, expected_url,
             msg_prefix + "Response redirected to '%s', expected '%s'" % (url, expected_url)
         )
 
+    def assertURLEqual(self, url1, url2, msg_prefix=''):
+        """
+        Assert that two URLs are the same, ignoring the order of query string
+        parameters except for parameters with the same name.
+
+        For example, /path/?x=1&y=2 is equal to /path/?y=2&x=1, but
+        /path/?a=1&a=2 isn't equal to /path/?a=2&a=1.
+        """
+        def normalize(url):
+            """Sort the URL's query string parameters."""
+            scheme, netloc, path, params, query, fragment = urlparse(url)
+            query_parts = sorted(parse_qsl(query))
+            return urlunparse((scheme, netloc, path, params, urlencode(query_parts), fragment))
+
+        self.assertEqual(
+            normalize(url1), normalize(url2),
+            msg_prefix + "Expected '%s' to equal '%s'." % (url1, url2)
+        )
+
     def _assert_contains(self, response, text, status_code, msg_prefix, html):
         # If the response supports deferred rendering and hasn't been rendered
         # yet, then ensure that it does get rendered before proceeding further.
diff --git a/docs/releases/2.2.txt b/docs/releases/2.2.txt
index 5994a054a6..a968ffa53c 100644
--- a/docs/releases/2.2.txt
+++ b/docs/releases/2.2.txt
@@ -183,7 +183,9 @@ Templates
 Tests
 ~~~~~
 
-* ...
+* The new :meth:`.SimpleTestCase.assertURLEqual` assertion checks for a given
+  URL, ignoring the ordering of the query string.
+  :meth:`~.SimpleTestCase.assertRedirects` uses the new assertion.
 
 URLs
 ~~~~
diff --git a/docs/topics/testing/tools.txt b/docs/topics/testing/tools.txt
index 1f7d3b4f06..6e0cfad80b 100644
--- a/docs/topics/testing/tools.txt
+++ b/docs/topics/testing/tools.txt
@@ -700,6 +700,7 @@ A subclass of :class:`unittest.TestCase` that adds this functionality:
     <SimpleTestCase.assertContains>`.
   * Verifying that a template :meth:`has/hasn't been used to generate a given
     response content <SimpleTestCase.assertTemplateUsed>`.
+  * Verifying that two :meth:`URLs <SimpleTestCase.assertURLEqual>` are equal.
   * Verifying a HTTP :meth:`redirect <SimpleTestCase.assertRedirects>` is
     performed by the app.
   * Robustly testing two :meth:`HTML fragments <SimpleTestCase.assertHTMLEqual>`
@@ -1477,6 +1478,15 @@ your test suite.
     You can use this as a context manager in the same way as
     :meth:`~SimpleTestCase.assertTemplateUsed`.
 
+.. method:: SimpleTestCase.assertURLEqual(url1, url2, msg_prefix='')
+
+    .. versionadded:: 2.2
+
+    Asserts that two URLs are the same, ignoring the order of query string
+    parameters except for parameters with the same name. For example,
+    ``/path/?x=1&y=2`` is equal to ``/path/?y=2&x=1``, but
+    ``/path/?a=1&a=2`` isn't equal to ``/path/?a=2&a=1``.
+
 .. method:: SimpleTestCase.assertRedirects(response, expected_url, status_code=302, target_status_code=200, msg_prefix='', fetch_redirect_response=True)
 
     Asserts that the response returned a ``status_code`` redirect status,
diff --git a/tests/auth_tests/test_views.py b/tests/auth_tests/test_views.py
index c7d64d5d48..6482f92c5d 100644
--- a/tests/auth_tests/test_views.py
+++ b/tests/auth_tests/test_views.py
@@ -3,7 +3,7 @@ import itertools
 import os
 import re
 from importlib import import_module
-from urllib.parse import ParseResult, quote, urlparse
+from urllib.parse import quote
 
 from django.apps import apps
 from django.conf import settings
@@ -23,7 +23,7 @@ from django.contrib.sessions.middleware import SessionMiddleware
 from django.contrib.sites.requests import RequestSite
 from django.core import mail
 from django.db import connection
-from django.http import HttpRequest, QueryDict
+from django.http import HttpRequest
 from django.middleware.csrf import CsrfViewMiddleware, get_token
 from django.test import Client, TestCase, override_settings
 from django.test.client import RedirectCycleError
@@ -70,23 +70,6 @@ class AuthViewsTestCase(TestCase):
         form_errors = list(itertools.chain(*response.context['form'].errors.values()))
         self.assertIn(str(error), form_errors)
 
-    def assertURLEqual(self, url, expected, parse_qs=False):
-        """
-        Given two URLs, make sure all their components (the ones given by
-        urlparse) are equal, only comparing components that are present in both
-        URLs.
-        If `parse_qs` is True, then the querystrings are parsed with QueryDict.
-        This is useful if you don't want the order of parameters to matter.
-        Otherwise, the query strings are compared as-is.
-        """
-        fields = ParseResult._fields
-
-        for attr, x, y in zip(fields, urlparse(url), urlparse(expected)):
-            if parse_qs and attr == 'query':
-                x, y = QueryDict(x), QueryDict(y)
-            if x and y and x != y:
-                self.fail("%r != %r (%s doesn't match)" % (url, expected, attr))
-
 
 @override_settings(ROOT_URLCONF='django.contrib.auth.urls')
 class AuthViewNamedURLTests(AuthViewsTestCase):
@@ -724,10 +707,10 @@ class LoginTest(AuthViewsTestCase):
 
 class LoginURLSettings(AuthViewsTestCase):
     """Tests for settings.LOGIN_URL."""
-    def assertLoginURLEquals(self, url, parse_qs=False):
+    def assertLoginURLEquals(self, url):
         response = self.client.get('/login_required/')
         self.assertEqual(response.status_code, 302)
-        self.assertURLEqual(response.url, url, parse_qs=parse_qs)
+        self.assertURLEqual(response.url, url)
 
     @override_settings(LOGIN_URL='/login/')
     def test_standard_login_url(self):
@@ -751,7 +734,7 @@ class LoginURLSettings(AuthViewsTestCase):
 
     @override_settings(LOGIN_URL='/login/?pretty=1')
     def test_login_url_with_querystring(self):
-        self.assertLoginURLEquals('/login/?pretty=1&next=/login_required/', parse_qs=True)
+        self.assertLoginURLEquals('/login/?pretty=1&next=/login_required/')
 
     @override_settings(LOGIN_URL='http://remote.example.com/login/?next=/default/')
     def test_remote_login_url_with_next_querystring(self):
diff --git a/tests/test_client/tests.py b/tests/test_client/tests.py
index fb506e7ca9..e45a743f22 100644
--- a/tests/test_client/tests.py
+++ b/tests/test_client/tests.py
@@ -205,6 +205,12 @@ class ClientTest(TestCase):
         response = self.client.get('/redirect_view/', {'var': 'value'})
         self.assertRedirects(response, '/get_view/?var=value')
 
+    def test_redirect_with_query_ordering(self):
+        """assertRedirects() ignores the order of query string parameters."""
+        response = self.client.get('/redirect_view/', {'var': 'value', 'foo': 'bar'})
+        self.assertRedirects(response, '/get_view/?var=value&foo=bar')
+        self.assertRedirects(response, '/get_view/?foo=bar&var=value')
+
     def test_permanent_redirect(self):
         "GET a URL that redirects permanently elsewhere"
         response = self.client.get('/permanent_redirect_view/')
diff --git a/tests/test_utils/tests.py b/tests/test_utils/tests.py
index 439ca0b7c7..d908b52a8a 100644
--- a/tests/test_utils/tests.py
+++ b/tests/test_utils/tests.py
@@ -911,6 +911,54 @@ class AssertFieldOutputTests(SimpleTestCase):
         self.assertFieldOutput(MyCustomField, {}, {}, empty_value=None)
 
 
+class AssertURLEqualTests(SimpleTestCase):
+    def test_equal(self):
+        valid_tests = (
+            ('http://example.com/?', 'http://example.com/'),
+            ('http://example.com/?x=1&', 'http://example.com/?x=1'),
+            ('http://example.com/?x=1&y=2', 'http://example.com/?y=2&x=1'),
+            ('http://example.com/?x=1&y=2', 'http://example.com/?y=2&x=1'),
+            ('http://example.com/?x=1&y=2&a=1&a=2', 'http://example.com/?a=1&a=2&y=2&x=1'),
+            ('/path/to/?x=1&y=2&z=3', '/path/to/?z=3&y=2&x=1'),
+            ('?x=1&y=2&z=3', '?z=3&y=2&x=1'),
+        )
+        for url1, url2 in valid_tests:
+            with self.subTest(url=url1):
+                self.assertURLEqual(url1, url2)
+
+    def test_not_equal(self):
+        invalid_tests = (
+            # Protocol must be the same.
+            ('http://example.com/', 'https://example.com/'),
+            ('http://example.com/?x=1&x=2', 'https://example.com/?x=2&x=1'),
+            ('http://example.com/?x=1&y=bar&x=2', 'https://example.com/?y=bar&x=2&x=1'),
+            # Parameters of the same name must be in the same order.
+            ('/path/to?a=1&a=2', '/path/to/?a=2&a=1')
+        )
+        for url1, url2 in invalid_tests:
+            with self.subTest(url=url1), self.assertRaises(AssertionError):
+                self.assertURLEqual(url1, url2)
+
+    def test_message(self):
+        msg = (
+            "Expected 'http://example.com/?x=1&x=2' to equal "
+            "'https://example.com/?x=2&x=1'"
+        )
+        with self.assertRaisesMessage(AssertionError, msg):
+            self.assertURLEqual('http://example.com/?x=1&x=2', 'https://example.com/?x=2&x=1')
+
+    def test_msg_prefix(self):
+        msg = (
+            "Prefix: Expected 'http://example.com/?x=1&x=2' to equal "
+            "'https://example.com/?x=2&x=1'"
+        )
+        with self.assertRaisesMessage(AssertionError, msg):
+            self.assertURLEqual(
+                'http://example.com/?x=1&x=2', 'https://example.com/?x=2&x=1',
+                msg_prefix='Prefix: ',
+            )
+
+
 class FirstUrls:
     urlpatterns = [url(r'first/$', empty_response, name='first')]