Doc server broken link detection

chrome/common/extensions/docs/server2/integration_test.py

Index: chrome/common/extensions/docs/server2/integration_test.py
diff --git a/chrome/common/extensions/docs/server2/integration_test.py b/chrome/common/extensions/docs/server2/integration_test.py
index acd4d44c1e6ce18283581712f6e556148c4f00c7..052045e5f2750221f92ccf461b1f7a0a28d979ca 100755
--- a/chrome/common/extensions/docs/server2/integration_test.py
+++ b/chrome/common/extensions/docs/server2/integration_test.py
@@ -8,13 +8,16 @@
 import build_server
 build_server.main()
 
-import logging
+from itertools import groupby
+from operator import itemgetter
 import optparse
 import os
 import sys
 import time
 import unittest
 
+from link_error_detector import LinkErrorDetector
+from local_file_system import LocalFileSystem
 from local_renderer import LocalRenderer
 from fake_fetchers import ConfigureFakeFetchers
 from handler import Handler
@@ -40,6 +43,21 @@ def _GetPublicFiles():
         public_files['/'.join((relative_posix_path, filename))] = f.read()
   return public_files
 
+def _PrintBrokenLinks(broken_links):
+  '''Prints out broken links in a more readable format.
+  '''
+  col_width = max([len(l[0]) for l in broken_links])

[not at google - send to devlin, 2013/07/17 20:19:40]

the square brackets aren't needed (just creates redundant list).

max(len(link[0]) for link in broken_links)

and yeah, never use l as a variable name...

[jshumway, 2013/07/17 22:15:47]

Done.

+  getter = itemgetter(1)
+
+  for target, links in groupby(sorted(broken_links, key=getter), getter):
+    links = [l[0] for l in links]
+    if len(links) > 50:
+      out = "%s and %d others" % (links[0], len(links) - 1)
+      print("%s%s -> %s" % (out, (col_width - len(out)) * ' ', target))
+    else:
+      for link in links:
+        print("%s%s -> %s" % (link, (col_width - len(link)) * ' ', target))

[not at google - send to devlin, 2013/07/17 20:19:40]

can you pull this print logic into a function? Like

pretty_print(out, target)
vs
pretty_print(link, target)

with pretty_print defined under col_width like

def pretty_print(prefix, message):
  print('%s%s - %s' % (prefix, len(prefix), message)

I think that's right...

[jshumway, 2013/07/17 22:15:47]

Done.

+
 class IntegrationTest(unittest.TestCase):
   def setUp(self):
     ConfigureFakeFetchers()
@@ -61,6 +79,36 @@ class IntegrationTest(unittest.TestCase):
     finally:
       print('Took %s seconds' % (time.time() - start_time))
 
+    # Check for broken links.

[not at google - send to devlin, 2013/07/17 20:19:40]

comment is redundant given the next line

[jshumway, 2013/07/17 22:15:47]

Done.

+    print("Checking for broken links...")
+    start_time = time.time()
+    link_error_detector = LinkErrorDetector(
+        LocalFileSystem(os.path.join(sys.path[0], os.pardir, os.pardir)),
+        lambda path: Handler(Request.ForTest(path)).Get(),
+        'templates/public',
+        ('extensions/index.html', 'apps/about_apps.html'))
+
+    broken_links, broken_anchors = link_error_detector.GetBrokenLinks()
+    if broken_links or broken_anchors:
+      print('[%d broken links] page of occurrence\t-> href of broken link' % (
+          len(broken_links) + len(broken_anchors)))

[not at google - send to devlin, 2013/07/17 20:19:40]

logging will look a little bit awkward since alignment depends on tab width.

Let's just say:

print('Found %s broken links:' % len(...))
_PrintBrokenLinks(broken_links + broken_anchors)

or maybe the + isn't possible? I can't see what the effect of breaking this up
is...

[jshumway, 2013/07/17 22:15:47]

Done.

+      if broken_links:
+        _PrintBrokenLinks(broken_links)
+      if broken_anchors:
+        _PrintBrokenLinks(broken_anchors)

[not at google - send to devlin, 2013/07/17 20:19:40]

make sure you actually fail the test at this point or chances are nobody will
notice.

[jshumway, 2013/07/17 22:15:47]

Prints a warning.

+
+    print('Took %s seconds.' % (time.time() - start_time))
+
+    # Check for orphaned pages.

[not at google - send to devlin, 2013/07/17 20:19:40]

comment also redundant

[jshumway, 2013/07/17 22:15:47]

Done.

+    print('Searching for orphaned pages...')
+    start_time = time.time()
+    orphaned_pages = link_error_detector.GetOrphanedPages()
+    if orphaned_pages:
+      print('[%d orphaned pages]' % len(orphaned_pages))

[not at google - send to devlin, 2013/07/17 20:19:40]

Found %d orphaned pages:

[jshumway, 2013/07/17 22:15:47]

Done.

+      for page in orphaned_pages:
+        print(page)
+    print('Took %s seconds.' % (time.time() - start_time))

[not at google - send to devlin, 2013/07/17 20:19:40]

likewise, fail the test (self.fail maybe?)

[jshumway, 2013/07/17 22:15:47]

Prints a warning.

+
     public_files = _GetPublicFiles()
 
     print('Rendering %s public files...' % len(public_files.keys()))
@@ -103,6 +151,9 @@ class IntegrationTest(unittest.TestCase):
       finally:
         print('Took %s seconds' % (time.time() - start_time))
 
+    # TODO: Check page for broken links (currently prohibited by the time it

[not at google - send to devlin, 2013/07/17 20:19:40]

always name TODOs. so TODO(jshumway) here.

[jshumway, 2013/07/17 22:15:47]

Done.

+    # takes to render the pages).
+
   @DisableLogging('warning')
   def testFileNotFound(self):
     response = Handler(Request.ForTest('/extensions/notfound.html')).Get()