Merge lp:~cjohnston/qa-dashboard/smokeng-url-part2 into lp:qa-dashboard

[Chris Johnston]

Add build number and arch to the smokeng urls

Further work to make the smokeng url's understandable by humans

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:607
http://s-jenkins:8080/job/dashboard-ci/179/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins:8080/job/dashboard-ci/179/rebuild

review: Approve (continuous-integration)

[Joe Talbott (joetalbott)]

I might be biased, but that's some of the nicest code I've seen. ;)

review: Approve

=== modified file 'smokeng/tables.py'
--- smokeng/tables.py	2013-09-09 13:06:06 +0000
+++ smokeng/tables.py	2013-09-27 17:06:21 +0000
@@ -13,10 +13,19 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+from django.utils.http import urlquote
+
 import django_tables2 as tables
 from django_tables2.utils import Accessor as A
 
 
+class LinkColumnQuoted(tables.LinkColumn):
+
+    def render_link(self, uri, text, attrs=None):
+        uri = urlquote(uri, safe='/: %')
+        return super(LinkColumnQuoted, self).render_link(uri, text, attrs)
+
+
 class SmokeBaseTable(tables.Table):
     class Meta:
         attrs = {'class': 'basic wide'}
@@ -77,7 +86,7 @@
             'arch',
         ) + SmokeBaseTable.Meta.fields
 
-    build_number = tables.LinkColumn(
+    build_number = LinkColumnQuoted(
         'smokeng.views.image_overview',
         accessor="build_number",
         order_by=(
@@ -90,6 +99,8 @@
         kwargs={
             "release": A('release'),
             "variant": A('variant'),
+            "arch": A('arch'),
+            "build_number": A('build_number'),
             "image_id": A('image_id'),
         }
     )
@@ -105,11 +116,13 @@
             'ran_at',
         ) + SmokeBaseTable.Meta.fields
 
-    name = tables.LinkColumn(
+    name = LinkColumnQuoted(
         "smokeng.views.image_test_results",
         kwargs={
             "release": A('image__release'),
             "variant": A('image__variant'),
+            "arch": A('image__arch'),
+            "build_number": A('image__build_number'),
             "image_id": A('image__id'),
             "name": A('name'),
         },
@@ -132,11 +145,13 @@
 
 
 class SmokeTestResultTable(tables.Table):
-    command_type = tables.LinkColumn(
+    command_type = LinkColumnQuoted(
         "smokeng.views.test_result_detail",
         kwargs={
             "release": A('release'),
             "variant": A('variant'),
+            "arch": A('arch'),
+            "build_number": A('build_number'),
             "image_id": A('image_id'),
             "name": A('name'),
             "id": A('id'),

=== modified file 'smokeng/tests.py'
--- smokeng/tests.py	2013-09-09 13:06:06 +0000
+++ smokeng/tests.py	2013-09-27 17:06:21 +0000
@@ -15,6 +15,7 @@
 
 import datetime
 import mock
+import urllib2
 
 from django.core.urlresolvers import reverse
 from django.test import TestCase
@@ -130,16 +131,51 @@
 
         self._test_url("{}?sort=bugs".format(url))
 
-    def test_image(self):
-
-        url = reverse(
-            'smokeng_image_overview',
-            args=(
-                self.image.release,
-                self.image.variant,
-                self.image.id,
-            ),
-        )
+    def test_image_overview_url(self):
+
+        url = reverse(
+            'smokeng_image_overview',
+            args=(
+                self.image.release,
+                self.image.variant,
+                self.image.arch,
+                self.image.build_number,
+                self.image.id,
+            ),
+        )
+        self._test_url(url)
+
+    def test_image_overview_touch_ro_url(self):
+        self.image.build_number = '56:20130920:20130920'
+        self.image.save()
+        url = reverse(
+            'smokeng_image_overview',
+            args=(
+                self.image.release,
+                self.image.variant,
+                self.image.arch,
+                self.image.build_number,
+                self.image.id,
+            ),
+        )
+        self._test_url(url)
+
+    def test_image_overview_unknown_build_number_url(self):
+        self.image.build_number = '20130920 ?'
+        self.image.save()
+        url = reverse(
+            'smokeng_image_overview',
+            args=(
+                self.image.release,
+                self.image.variant,
+                self.image.arch,
+                self.image.build_number,
+                self.image.id,
+            ),
+        )
+
+        # URL quote the url due to the '?'
+        url = urllib2.quote(url)
         self._test_url(url)
 
     def test_image_bug_sorting(self):
@@ -149,6 +185,8 @@
             args=(
                 self.image.release,
                 self.image.variant,
+                self.image.arch,
+                self.image.build_number,
                 self.image.id,
             ),
         )
@@ -161,6 +199,8 @@
             args=(
                 self.image.release,
                 self.image.variant,
+                self.image.arch,
+                self.image.build_number,
                 self.image.id,
                 self.result.name,
             ),
@@ -168,12 +208,49 @@
         response = self._test_url(url)
         self.assertTemplateUsed(response, 'smokeng/test_results.html')
 
+    def test_image_test_results_touch_ro_url(self):
+        self.image.build_number = '56:20130920:20130920'
+        self.image.save()
+        url = reverse(
+            'smokeng_image_test_results',
+            args=(
+                self.image.release,
+                self.image.variant,
+                self.image.arch,
+                self.image.build_number,
+                self.image.id,
+                self.result.name,
+            ),
+        )
+        self._test_url(url)
+
+    def test_image_test_results_unknown_build_number_url(self):
+        self.image.build_number = '20130920 ?'
+        self.image.save()
+        url = reverse(
+            'smokeng_image_test_results',
+            args=(
+                self.image.release,
+                self.image.variant,
+                self.image.arch,
+                self.image.build_number,
+                self.image.id,
+                self.result.name,
+            ),
+        )
+
+        # URL quote the url due to the '?'
+        url = urllib2.quote(url)
+        self._test_url(url)
+
     def test_test_result_detail_url(self):
         url = reverse(
             'smokeng_test_result_detail',
             args=(
                 self.image.release,
                 self.image.variant,
+                self.image.arch,
+                self.image.build_number,
                 self.image.id,
                 self.result.name,
                 self.testcaseresult.pk,
@@ -182,6 +259,43 @@
         response = self._test_url(url)
         self.assertTemplateUsed(response, 'smokeng/test_result_detail.html')
 
+    def test_image_test_result_detail_touch_ro_url(self):
+        self.image.build_number = '56:20130920:20130920'
+        self.image.save()
+        url = reverse(
+            'smokeng_test_result_detail',
+            args=(
+                self.image.release,
+                self.image.variant,
+                self.image.arch,
+                self.image.build_number,
+                self.image.id,
+                self.result.name,
+                self.testcaseresult.pk,
+            ),
+        )
+        self._test_url(url)
+
+    def test_image_test_result_detail_unknown_build_number_url(self):
+        self.image.build_number = '20130920 ?'
+        self.image.save()
+        url = reverse(
+            'smokeng_test_result_detail',
+            args=(
+                self.image.release,
+                self.image.variant,
+                self.image.arch,
+                self.image.build_number,
+                self.image.id,
+                self.result.name,
+                self.testcaseresult.pk,
+            ),
+        )
+
+        # URL quote the url due to the '?'
+        url = urllib2.quote(url)
+        self._test_url(url)
+
 
 class TestSmoke(TestCase):
     """ Tests for the smoke models. """

=== modified file 'smokeng/urls.py'
--- smokeng/urls.py	2013-09-09 13:06:06 +0000
+++ smokeng/urls.py	2013-09-27 17:06:21 +0000
@@ -26,18 +26,23 @@
         name='smokeng_overview',
     ),
     url(
-        r'^(?P<release>\w+)/(?P<variant>\w+)/(?P<image_id>\d+)/$',
+        r'^(?P<release>\w+)/(?P<variant>\w+)/(?P<arch>\w+)'
+        '/(?P<build_number>(\d+:\d+(\.\d+|):\d+(\.\d+|)|\d+(\.\d+|( |%20)\?|)))'
+        '/(?P<image_id>\d+)/$',
         'image_overview',
         name='smokeng_image_overview',
     ),
     url(
-        r'^(?P<release>\w+)/(?P<variant>\w+)/(?P<image_id>\d+)/(?P<name>.*)'
-        '/(?P<id>\d+)/$',
+        r'^(?P<release>\w+)/(?P<variant>\w+)/(?P<arch>\w+)'
+        '/(?P<build_number>(\d+:\d+(\.\d+|):\d+(\.\d+|)|\d+(\.\d+|( |%20)\?|)))'
+        '/(?P<image_id>\d+)/(?P<name>.*)/(?P<id>\d+)/$',
         'test_result_detail',
         name='smokeng_test_result_detail',
     ),
     url(
-        r'^(?P<release>\w+)/(?P<variant>\w+)/(?P<image_id>\d+)/(?P<name>.*)/$',
+        r'^(?P<release>\w+)/(?P<variant>\w+)/(?P<arch>\w+)'
+        '/(?P<build_number>(\d+:\d+(\.\d+|):\d+(\.\d+|)|\d+(\.\d+|( |%20)\?|)))'
+        '/(?P<image_id>\d+)/(?P<name>.*)/$',
         'image_test_results',
         name='smokeng_image_test_results',
     ),

=== modified file 'smokeng/views.py'
--- smokeng/views.py	2013-09-09 13:06:06 +0000
+++ smokeng/views.py	2013-09-27 17:06:21 +0000
@@ -262,10 +262,10 @@
 @BreadCrumb(
     "Image {build_number}",
     parent=overview,
-    needs=['release', 'variant', 'image_id'],
+    needs=['release', 'variant', 'arch', 'build_number', 'image_id'],
 )
 @require_GET
-def image_overview(request, release, variant, image_id):
+def image_overview(request, release, variant, arch, build_number, image_id):
 
     image = get_object_or_404(SmokeImage, id=image_id)
     summary, bug_data = image.summary
@@ -284,6 +284,7 @@
             image_overview,
             release=release,
             variant=variant,
+            arch=arch,
             image_id=image_id,
             build_number=image.build_number,
         ),
@@ -298,10 +299,18 @@
 @BreadCrumb(
     "{name}",
     parent=image_overview,
-    needs=['release', 'variant', 'image_id', 'name'],
+    needs=['release', 'variant', 'arch', 'build_number', 'image_id', 'name'],
 )
 @require_GET
-def image_test_results(request, release, variant, image_id, name):
+def image_test_results(
+    request,
+    release,
+    variant,
+    arch,
+    build_number,
+    image_id,
+    name,
+):
 
     image = get_object_or_404(SmokeImage, id=image_id)
     result = get_object_or_404(
@@ -335,7 +344,9 @@
         test_data = dict(
             release=release,
             name=name,
+            arch=arch,
             variant=variant,
+            build_number=build_number,
             testcase=test.testcase,
             url_name=os.path.basename(test.testcase),
             testsuite=os.path.basename(test.testsuite),
@@ -364,6 +375,7 @@
             image_test_results,
             release=release,
             variant=variant,
+            arch=arch,
             image_id=image_id,
             name=name,
             build_number=image.build_number,
@@ -379,10 +391,19 @@
 @BreadCrumb(
     "Testcase details",
     parent=image_test_results,
-    needs=['release', 'variant', 'image_id', 'name', 'id'],
+    needs=['release', 'variant', 'arch', 'build_number', 'image_id', 'name', 'id'],
 )
 @require_GET
-def test_result_detail(request, release, variant, image_id, name, id):
+def test_result_detail(
+    request,
+    release,
+    variant,
+    arch,
+    build_number,
+    image_id,
+    name,
+    id,
+):
     test = get_object_or_404(
         SmokeTestResult,
         pk=id,
@@ -421,6 +442,7 @@
             test_result_detail,
             release=release,
             variant=variant,
+            arch=arch,
             image_id=image_id,
             name=name,
             id=id,