[Django] #29098: Allow assertRedirects to handle regex matches.

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

[Django] #29098: Allow assertRedirects to handle regex matches.

Django
#29098: Allow assertRedirects to handle regex matches.
-------------------------------------+-------------------------------------
               Reporter:  Dan J      |          Owner:  nobody
  Strohl                             |
                   Type:             |         Status:  new
  Cleanup/optimization               |
              Component:  Testing    |        Version:  1.11
  framework                          |
               Severity:  Normal     |       Keywords:  unittest redirect
           Triage Stage:             |      Has patch:  0
  Unreviewed                         |
    Needs documentation:  0          |    Needs tests:  0
Patch needs improvement:  0          |  Easy pickings:  0
                  UI/UX:  0          |
-------------------------------------+-------------------------------------
 or, perhaps, allow it to use the patterns from the url's file.  Either
 way, the issue is that I have a view that gets a request, looks at it, and
 redirects it to a url such as /labs/12345/running, or /labs/4567/start.
 this is a similar pattern to what is recommended and used in the admin, so
 I don't think I am doing something weird here, but I may not know what the
 redirect url will look like before I send the request (if I am sending
 something like /labs/new, and it returns /labs/12345 for example).

 as a hack, I did this:
 {{{
 #!div style="font-size: 80%"
 Code highlighting:
   {{{#!python
 def fix_response_for_test(response, re_pattern, replace, count=0,
 flags=0):

     if hasattr(response, 'redirect_chain'):
         url, status_code = response.redirect_chain[-1]

         tmp_replaced = re.search(re_pattern, url, flags=flags)
         new_url = re.sub(re_pattern, replace, url, count=count,
 flags=flags)

         # print('redirect - new: %s' % new_url)

         response.redirect_chain[-1] = (new_url, status_code)

     else:
         # Not a followed redirect
         url = response.url
         scheme, netloc, path, query, fragment = urlsplit(url)

         # Prepend the request path to handle relative path redirects.
         if not path.startswith('/'):
             url = urljoin(response.request['PATH_INFO'], url)

         tmp_replaced = re.search(re_pattern, url, flags=flags)
         new_url = re.sub(re_pattern, replace, url, count=count,
 flags=flags)

         # print('no redirected - new: %s' % new_url)

         response['Location'] = new_url

     return tmp_replaced.group(0)
   }}}
 }}}

 and is run like this:

 {{{
 #!div style="font-size: 80%"
 Code highlighting:
   {{{#!python

         session_id = fix_response_for_test(response, UUID_REGEX, '<uuid>')

         redirect_url = '/lab/<uuid>/error/'

         with self.subTest('%s - response url' % name):
                 self.assertRedirects(response, redirect_url,
 fetch_redirect_response=False, msg_prefix=tmp_msg)
         test_session = Sessions.objects.get(session_id=session_id)
         # do more testing on the session object to make sure it was
 created correctly.
   }}}
 }}}

 The returning the pulled content is nice, but probably not required as I
 COULD simply build two tests, one to check the redirect, and another to
 test the actual session object.

 If I had my druthers, I would love to see something like:

 {{{
 #!div style="font-size: 80%"
 Code highlighting:
   {{{#!python

 args_obj=None
 self.assertRedirects(response, r'/labs/(?P<foobar>.+)/(.+)',
 get_args=args_obj)

 # assuming this passes the assertion, args_obj then would ==
 # args_obj = {
 #    'args': ['list of un-named items'],
 #    'kwargs': {dict of kwargs}
   }}}
 }}}

 This coudl also be approached by adding the ability to get this kind of
 thing directly from the response object, along the lines of:
 {{{
 #!div style="font-size: 80%"
 Code highlighting:
   {{{#!python

 (assuming the request was '/labs/12344/test_page
 > my_response.seed_url()
 ' '//labs//(?P<foobar>.+)//(.+)'  # which could then be matched in a
 redirect url match.
 > my_response.url_params(1)
 'test_page'
 > my_response.url_params('foobar')
 '12344'

   }}}
 }}}

--
Ticket URL: <https://code.djangoproject.com/ticket/29098>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/050.34efc0ad438354ebf1af22af2ed6a1e2%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29098: Allow assertRedirects to handle regex matches.

Django
#29098: Allow assertRedirects to handle regex matches.
-------------------------------------+-------------------------------------
     Reporter:  Dan J Strohl         |                    Owner:  nobody
         Type:                       |                   Status:  new
  Cleanup/optimization               |
    Component:  Testing framework    |                  Version:  1.11
     Severity:  Normal               |               Resolution:
     Keywords:  unittest redirect    |             Triage Stage:
                                     |  Unreviewed
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by Tim Graham):

 I'm not really in favor of the additional complexity. In the admin tests,
 for example, the problem is solved like this:
 {{{
 response = <post request to create new object>
 new_user = User.objects.get(username='newuser')
 self.assertRedirects(response, reverse('admin:auth_user_change',
 args=(new_user.pk,)))
 }}}
 Does that work for you?

--
Ticket URL: <https://code.djangoproject.com/ticket/29098#comment:1>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/065.5d816f57cd4e6ae7856a92cdc9d8319c%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29098: Allow assertRedirects to handle regex matches.

Django
In reply to this post by Django
#29098: Allow assertRedirects to handle regex matches.
-------------------------------------+-------------------------------------
     Reporter:  Dan J Strohl         |                    Owner:  nobody
         Type:                       |                   Status:  new
  Cleanup/optimization               |
    Component:  Testing framework    |                  Version:  1.11
     Severity:  Normal               |               Resolution:
     Keywords:  unittest redirect    |             Triage Stage:
                                     |  Unreviewed
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by Dan J Strohl):

 Replying to [comment:1 Tim Graham]:
 > I'm not really in favor of the additional complexity. In the admin
 tests, for example, the problem is solved like this:
 > {{{
 > response = <post request to create new object>
 > new_user = User.objects.get(username='newuser')
 > self.assertRedirects(response, reverse('admin:auth_user_change',
 args=(new_user.pk,)))
 > }}}
 > Does that work for you?

 I understand and am generally in agreement with keeping things simple...

 While your suggestion would generally work (I think at least, I haven't
 tried it), it feels like it would be easier (from the developers pov) to
 be able to test if a redirect matched a url pattern, or one of the new url
 matching.. (objects?),  in addition to simply matching a string.  that way
 a developer could be as specific (or fuzzy) as they wished.  and it would
 directly link back to checking the url patterns as well.

 The ability to pull out the string is nice, but not really crucial to the
 specific feature request.

 I would not put this as a critical feature request, but perhaps a bit more
 than a "nice to have".

--
Ticket URL: <https://code.djangoproject.com/ticket/29098#comment:2>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/065.c1caebb2e13a158d3a7249e5e6d398bf%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29098: Allow assertRedirects to handle regex matches.

Django
In reply to this post by Django
#29098: Allow assertRedirects to handle regex matches.
-------------------------------------+-------------------------------------
     Reporter:  Dan J Strohl         |                    Owner:  nobody
         Type:                       |                   Status:  new
  Cleanup/optimization               |
    Component:  Testing framework    |                  Version:  1.11
     Severity:  Normal               |               Resolution:
     Keywords:  unittest redirect    |             Triage Stage:
                                     |  Unreviewed
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by Tim Graham):

 It sounds like you'd like `assertRedirects()` to be able to  do the
 `reverse()` itself. I don't see much advantage to that (complicating
 things by allowing `assertRedirects()` to take all the parameters of
 `reverse()`). If I misunderstood, can you clarify what API change you're
 proposing?

--
Ticket URL: <https://code.djangoproject.com/ticket/29098#comment:3>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/065.66a4f88752df1e0cd07d1e8b20cace75%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29098: Allow assertRedirects to handle regex matches.

Django
In reply to this post by Django
#29098: Allow assertRedirects to handle regex matches.
-------------------------------------+-------------------------------------
     Reporter:  Dan J Strohl         |                    Owner:  nobody
         Type:                       |                   Status:  new
  Cleanup/optimization               |
    Component:  Testing framework    |                  Version:  1.11
     Severity:  Normal               |               Resolution:
     Keywords:  unittest redirect    |             Triage Stage:
                                     |  Unreviewed
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by Dan J Strohl):

 Replying to [comment:3 Tim Graham]:
 > It sounds like you'd like `assertRedirects()` to be able to  do the
 `reverse()` itself. I don't see much advantage to that (complicating
 things by allowing `assertRedirects()` to take all the parameters of
 `reverse()`). If I misunderstood, can you clarify what API change you're
 proposing?

 Ummm, not totally, the reverse() expects enough parameters to be able to
 create the url, I am proposing being able to validate without knowing the
 specific variables needed to complete the reverse.

 basically, at something like the changes below... (all I did was pull out
 the last assertion lines and put them into their own method.)

 Taking this a bit further, you could also pull out the assertion for the
 response codes and make them into their own assertion (which would be
 called by the assertRedirects) and/or even put the code to pull the
 response codes and response url into a helper function (like
 "get_redirected_url(response, fetch_redirect_response=True)" ) further
 modularizing it into even simpler pieces, and allowing for more
 flexibility in usage.


 {{{
 #!div style="font-size: 80%"
 Code highlighting:
   {{{#!python


 def _assertRedirects(self, response, expected_url, status_code=302,
                         target_status_code=200, msg_prefix='',
                         fetch_redirect_response=True):
         """
         Assert that a response redirected to a specific URL and that the
         redirect URL can be loaded.

         Won't work for external links since it uses the test client to do
 a
         request (use fetch_redirect_response=False to check such links
 without
         fetching them).
         """
         if msg_prefix:
             msg_prefix += ": "

         if hasattr(response, 'redirect_chain'):
             # The request was a followed redirect
             self.assertTrue(
                 len(response.redirect_chain) > 0,
                 msg_prefix + "Response didn't redirect as expected:
 Response code was %d (expected %d)"
                 % (response.status_code, status_code)
             )

             self.assertEqual(
                 response.redirect_chain[0][1], status_code,
                 msg_prefix + "Initial response didn't redirect as
 expected: Response code was %d (expected %d)"
                 % (response.redirect_chain[0][1], status_code)
             )

             url, status_code = response.redirect_chain[-1]
             scheme, netloc, path, query, fragment = urlsplit(url)

             self.assertEqual(
                 response.status_code, target_status_code,
                 msg_prefix + "Response didn't redirect as expected: Final
 Response code was %d (expected %d)"
                 % (response.status_code, target_status_code)
             )

         else:
             # Not a followed redirect
             self.assertEqual(
                 response.status_code, status_code,
                 msg_prefix + "Response didn't redirect as expected:
 Response code was %d (expected %d)"
                 % (response.status_code, status_code)
             )

             url = response.url
             scheme, netloc, path, query, fragment = urlsplit(url)

             # Prepend the request path to handle relative path redirects.
             if not path.startswith('/'):
                 url = urljoin(response.request['PATH_INFO'], url)
                 path = urljoin(response.request['PATH_INFO'], path)

             if fetch_redirect_response:
                 # netloc might be empty, or in cases where Django tests
 the
                 # HTTP scheme, the convention is for netloc to be
 'testserver'.
                 # Trust both as "internal" URLs here.
                 domain, port = split_domain_port(netloc)
                 if domain and not validate_host(domain,
 settings.ALLOWED_HOSTS):
                     raise ValueError(
                         "The test client is unable to fetch remote URLs
 (got %s). "
                         "If the host is served by Django, add '%s' to
 ALLOWED_HOSTS. "
                         "Otherwise, use assertRedirects(...,
 fetch_redirect_response=False)."
                         % (url, domain)
                     )
                 redirect_response = response.client.get(path,
 QueryDict(query), secure=(scheme == 'https'))

                 # Get the redirection page, using the same client that was
 used
                 # to obtain the original response.
                 self.assertEqual(
                     redirect_response.status_code, target_status_code,
                     msg_prefix + "Couldn't retrieve redirection page '%s':
 response code was %d (expected %d)"
                     % (path, redirect_response.status_code,
 target_status_code)
                 )
         return url

 def assertRedirects(self, response, expected_url, status_code=302,
                         target_status_code=200, msg_prefix='',
                         fetch_redirect_response=True):
         url = self._assertRedirects(
             response,
             expected_url,
             status_code=status_code,
             target_status_code=target_status_code,
             msg_prefix=msg_prefix,
             fetch_redirect_response=fetch_redirect_response)
         self.assertEqual(
             url, expected_url,
             msg_prefix + "Response redirected to '%s', expected '%s'" %
 (url, expected_url)
         )

 def assertRedirectsRegex(self, response, expected_url_regex,
 status_code=302,
                     target_status_code=200, msg_prefix='',
                     fetch_redirect_response=True):
         url = self._assertRedirects(
             response,
             expected_url,
             status_code=status_code,
             target_status_code=target_status_code,
             msg_prefix=msg_prefix,
             fetch_redirect_response=fetch_redirect_response)
         self.assertRegex(
             url, expected_url_regex,
             msg_prefix + "Response redirected to '%s', expected '%s'" %
 (url, expected_url)
         )


   }}}
 }}}

--
Ticket URL: <https://code.djangoproject.com/ticket/29098#comment:4>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/065.a18a40f301777e594b4f2cc40d1da325%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29098: Add SimpleTestCase.assertRedirectsRegex() (was: Allow assertRedirects to handle regex matches.)

Django
In reply to this post by Django
#29098: Add SimpleTestCase.assertRedirectsRegex()
-----------------------------------+------------------------------------
     Reporter:  Dan J Strohl       |                    Owner:  nobody
         Type:  New feature        |                   Status:  new
    Component:  Testing framework  |                  Version:  1.11
     Severity:  Normal             |               Resolution:
     Keywords:  unittest redirect  |             Triage Stage:  Accepted
    Has patch:  0                  |      Needs documentation:  0
  Needs tests:  0                  |  Patch needs improvement:  0
Easy pickings:  0                  |                    UI/UX:  0
-----------------------------------+------------------------------------
Changes (by Tim Graham):

 * type:  Cleanup/optimization => New feature
 * stage:  Unreviewed => Accepted


Old description:

> or, perhaps, allow it to use the patterns from the url's file.  Either
> way, the issue is that I have a view that gets a request, looks at it,
> and redirects it to a url such as /labs/12345/running, or
> /labs/4567/start.  this is a similar pattern to what is recommended and
> used in the admin, so I don't think I am doing something weird here, but
> I may not know what the redirect url will look like before I send the
> request (if I am sending something like /labs/new, and it returns
> /labs/12345 for example).
>
> as a hack, I did this:
> {{{
> #!div style="font-size: 80%"
> Code highlighting:
>   {{{#!python
> def fix_response_for_test(response, re_pattern, replace, count=0,
> flags=0):
>
>     if hasattr(response, 'redirect_chain'):
>         url, status_code = response.redirect_chain[-1]
>
>         tmp_replaced = re.search(re_pattern, url, flags=flags)
>         new_url = re.sub(re_pattern, replace, url, count=count,
> flags=flags)
>
>         # print('redirect - new: %s' % new_url)
>
>         response.redirect_chain[-1] = (new_url, status_code)
>
>     else:
>         # Not a followed redirect
>         url = response.url
>         scheme, netloc, path, query, fragment = urlsplit(url)
>
>         # Prepend the request path to handle relative path redirects.
>         if not path.startswith('/'):
>             url = urljoin(response.request['PATH_INFO'], url)
>
>         tmp_replaced = re.search(re_pattern, url, flags=flags)
>         new_url = re.sub(re_pattern, replace, url, count=count,
> flags=flags)
>
>         # print('no redirected - new: %s' % new_url)
>
>         response['Location'] = new_url
>
>     return tmp_replaced.group(0)
>   }}}
> }}}
>
> and is run like this:
>
> {{{
> #!div style="font-size: 80%"
> Code highlighting:
>   {{{#!python
>
>         session_id = fix_response_for_test(response, UUID_REGEX,
> '<uuid>')
>
>         redirect_url = '/lab/<uuid>/error/'
>
>         with self.subTest('%s - response url' % name):
>                 self.assertRedirects(response, redirect_url,
> fetch_redirect_response=False, msg_prefix=tmp_msg)
>         test_session = Sessions.objects.get(session_id=session_id)
>         # do more testing on the session object to make sure it was
> created correctly.
>   }}}
> }}}
>
> The returning the pulled content is nice, but probably not required as I
> COULD simply build two tests, one to check the redirect, and another to
> test the actual session object.
>
> If I had my druthers, I would love to see something like:
>
> {{{
> #!div style="font-size: 80%"
> Code highlighting:
>   {{{#!python
>
> args_obj=None
> self.assertRedirects(response, r'/labs/(?P<foobar>.+)/(.+)',
> get_args=args_obj)
>
> # assuming this passes the assertion, args_obj then would ==
> # args_obj = {
> #    'args': ['list of un-named items'],
> #    'kwargs': {dict of kwargs}
>   }}}
> }}}
>
> This coudl also be approached by adding the ability to get this kind of
> thing directly from the response object, along the lines of:
> {{{
> #!div style="font-size: 80%"
> Code highlighting:
>   {{{#!python
>
> (assuming the request was '/labs/12344/test_page
> > my_response.seed_url()
> ' '//labs//(?P<foobar>.+)//(.+)'  # which could then be matched in a
> redirect url match.
> > my_response.url_params(1)
> 'test_page'
> > my_response.url_params('foobar')
> '12344'
>
>   }}}
> }}}
New description:

 or, perhaps, allow it to use the patterns from the url's file.  Either
 way, the issue is that I have a view that gets a request, looks at it, and
 redirects it to a url such as /labs/12345/running, or /labs/4567/start.
 this is a similar pattern to what is recommended and used in the admin, so
 I don't think I am doing something weird here, but I may not know what the
 redirect url will look like before I send the request (if I am sending
 something like /labs/new, and it returns /labs/12345 for example).

 as a hack, I did this:
 {{{
 #!div style="font-size: 80%"
 Code highlighting:
   {{{#!python
 def fix_response_for_test(response, re_pattern, replace, count=0,
 flags=0):

     if hasattr(response, 'redirect_chain'):
         url, status_code = response.redirect_chain[-1]

         tmp_replaced = re.search(re_pattern, url, flags=flags)
         new_url = re.sub(re_pattern, replace, url, count=count,
 flags=flags)

         # print('redirect - new: %s' % new_url)

         response.redirect_chain[-1] = (new_url, status_code)

     else:
         # Not a followed redirect
         url = response.url
         scheme, netloc, path, query, fragment = urlsplit(url)

         # Prepend the request path to handle relative path redirects.
         if not path.startswith('/'):
             url = urljoin(response.request['PATH_INFO'], url)

         tmp_replaced = re.search(re_pattern, url, flags=flags)
         new_url = re.sub(re_pattern, replace, url, count=count,
 flags=flags)

         # print('no redirected - new: %s' % new_url)

         response['Location'] = new_url

     return tmp_replaced.group(0)
   }}}
 }}}

 and is run like this:

 {{{
 #!div style="font-size: 80%"
 Code highlighting:
   {{{#!python

         session_id = fix_response_for_test(response, UUID_REGEX, '<uuid>')

         redirect_url = '/lab/<uuid>/error/'

         with self.subTest('%s - response url' % name):
                 self.assertRedirects(response, redirect_url,
 fetch_redirect_response=False, msg_prefix=tmp_msg)
         test_session = Sessions.objects.get(session_id=session_id)
         # do more testing on the session object to make sure it was
 created correctly.
   }}}
 }}}

 The returning the pulled content is nice, but probably not required as I
 COULD simply build two tests, one to check the redirect, and another to
 test the actual session object.

 If I had my druthers, I would love to see something like:

 {{{
 #!div style="font-size: 80%"
 Code highlighting:
   {{{#!python

 args_obj=None
 self.assertRedirects(response, r'/labs/(?P<foobar>.+)/(.+)',
 get_args=args_obj)

 # assuming this passes the assertion, args_obj then would ==
 # args_obj = {
 #    'args': ['list of un-named items'],
 #    'kwargs': {dict of kwargs}
   }}}
 }}}

 This could also be approached by adding the ability to get this kind of
 thing directly from the response object, along the lines of:
 {{{
 #!div style="font-size: 80%"
 Code highlighting:
   {{{#!python

 (assuming the request was '/labs/12344/test_page
 > my_response.seed_url()
 ' '//labs//(?P<foobar>.+)//(.+)'  # which could then be matched in a
 redirect url match.
 > my_response.url_params(1)
 'test_page'
 > my_response.url_params('foobar')
 '12344'

   }}}
 }}}

--

Comment:

 I don't like it, but I suppose it would be consistent with other unittest
 assertions.

--
Ticket URL: <https://code.djangoproject.com/ticket/29098#comment:5>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/065.76b527a891ba9728e840eabe3afdc4d9%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.