URL regex for admin's app_index view isn't constructed deterministically

[Erik Bryant]

​https://github.com/django/django/blob/master/django/contrib/admin/sites.py#L277

The regex pattern that is constructed for an endpoint is not stable. For instance, in my project I have seen it construct each of the following:

^admin/^(?P<app_label>social_django|authtoken|queues|jobs|files|users|clusters|appps|auth|vdcs)/$
^admin/^(?P<app_label>social_django|auth|users|vdcs|appps|jobs|queues|authtoken|clusters|files)/$
^admin/^(?P<app_label>authtoken|appps|social_django|clusters|users|queues|jobs|vdcs|auth|files)/$
^admin/^(?P<app_label>authtoken|appps|social_django|clusters|users|queues|vdcs|jobs|auth|files)/$

This precludes the use of patterns in unit tests.

​https://github.com/django/django/pull/9230

[Tim Graham]

It seems to me that such a URL introspection test in your project is testing an internal implementation in Django that could be subject to change in the future. Can you give an idea of what regressions you're looking to test for? I wonder if a test that inspects _registry (or perhaps better yet, uses the site.is_registered() method) would be more appropriate.

[Erik Bryant]

I was asked to create a mechanism that would tell us if any new endpoints were added or if any existing endpoints were deleted. In either case we would need to update the web UI that uses our endpoints.

My less-than-ideal solution was to create a snapshot of our existing endpoints in a file. I wrote a test that compares the current set of endpoints with the snapshot. If an endpoint is added/deleted the test will fail. The expectation is that the developer who is making the change would see the failure and create a ticket to have the web UI updated. I'd be happy to know how other teams deal with this. I'm sure there is a better way.

[Tim Graham]

I don't entirely understand your setup but I wonder if you're concerned with changes to admin URLs and this URL in particular? Admin URLs may change when upgrading Django when new features are added or removed. For example, Django 2.0 adds an "autocomplete view". I wonder if your mechanism could ignore admin/ URLs entirely?

[Erik Bryant]

That's a good point. We don't plan to test the admin/ URLs. I'll remove that one from the list we scrape.

As for this ticket, should we go ahead with the fix?

[Tim Graham]

I'm not sure if there's a compelling reason to make the proposed change but I guess the overhead isn't too large.

[Erik Bryant]

I've been thinking of changing this from O(N-squared) to O(N) by using a set instead of a list. It would make the code just a little simpler. How about I do that to help offset the extra sorted() call?

​https://github.com/erikbryant/django/blob/master/django/contrib/admin/sites.py#L271

[Tim Graham]

I think that change could be made separately without a ticket.

[Erik Bryant]

We have an alternate approach, so we don't require this change. Since there is no clear other need for it, closing it as won't fix.