Issue 17816005: Doc server broken link detection

Issue 17816005: Doc server broken link detection (Closed)

Created:
7 years, 6 months ago by jshumway

Modified:
7 years, 5 months ago

Reviewers:
not at google - send to devlin

CC:
chromium-reviews, chromium-apps-reviews_chromium.org, extensions-reviews_chromium.org

Base URL:
https://chromium.googlesource.com/chromium/src.git@master

Visibility:
Public.

More Reviews

Description

Doc server broken link detection The integration test, when run with -a, now detects and reports broken links and orphaned pages. Broken links either link to an invalid page (404) or have an invalid anchor. Orphaned pages are pages that cannot be navigated to from the home pages just by clicking links. BUG=147747 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=212240

Patch Set 1 #

Patch Set 2 : style changes #

Patch Set 3 : Additional information for broken anchor links #

Total comments: 7

Patch Set 4 : patch rewrite #

Total comments: 60

Patch Set 5 : cleanup, special cases, polish #

Total comments: 18

Patch Set 6 : finalization #

Created: 7 years, 5 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+387 lines, -18 lines)			Patch
M	chrome/common/extensions/docs/server2/app.yaml	View	1 2 3 4	1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/common/extensions/docs/server2/cron.yaml	View	1 2 3 4	1 chunk	+4 lines, -4 lines	0 comments	Download
M	chrome/common/extensions/docs/server2/cron_servlet.py	View	1 2 3 4	2 chunks	+4 lines, -9 lines	0 comments	Download
A	chrome/common/extensions/docs/server2/file_system_util.py	View	1 2 3 4	1 chunk	+14 lines, -0 lines	0 comments	Download
M	chrome/common/extensions/docs/server2/handler.py	View	1 2 3	1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/common/extensions/docs/server2/integration_test.py	View	1 2 3 4 5	4 chunks	+52 lines, -1 line	0 comments	Download
A	chrome/common/extensions/docs/server2/link_error_detector.py	View	1 2 3 4	1 chunk	+219 lines, -0 lines	0 comments	Download
A	chrome/common/extensions/docs/server2/link_error_detector_test.py	View	1 2 3 4	1 chunk	+89 lines, -0 lines	0 comments	Download
M	chrome/common/extensions/docs/server2/server_instance.py	View	1 2 3 4	1 chunk	+1 line, -0 lines	0 comments	Download
M	chrome/common/extensions/docs/templates/articles/apps.html	View	1 2 3 4	1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/common/extensions/docs/templates/json/apps_sidenav.json	View	1 2 3 4	1 chunk	+1 line, -1 line	0 comments	Download

Messages

Total messages: 11 (0 generated)

Expand Messages | Collapse Messages

jshumway

The good news, we can finally find broken links systematically! The bad new, my test ...

7 years, 6 months ago (2013-06-26 17:49:10 UTC) #1

jshumway

Added an additional bit of output to the report of broken links that contain anchors, ...

7 years, 5 months ago (2013-07-01 18:50:08 UTC) #3

not at google - send to devlin

Some things to go on with. Didn't look at the logic itself in much detail ...

7 years, 5 months ago (2013-07-01 23:16:21 UTC) #4

Some things to go on with. Didn't look at the logic itself in much detail yet.

https://codereview.chromium.org/17816005/diff/7001/chrome/common/extensions/d...
File chrome/common/extensions/docs/server2/find_broken_links.py (right):

https://codereview.chromium.org/17816005/diff/7001/chrome/common/extensions/d...
chrome/common/extensions/docs/server2/find_broken_links.py:21: def
Memoize(func):
can you use the implementation from json schema compiler? and for that matter,
use it with the decorator style wherever it needs to be used?

https://codereview.chromium.org/17816005/diff/7001/chrome/common/extensions/d...
chrome/common/extensions/docs/server2/find_broken_links.py:139: varying number
of arguments, all of which can be converted to strings.
Just return the broken links in some format. The caller can decide how to
present it.

https://codereview.chromium.org/17816005/diff/7001/chrome/common/extensions/d...
chrome/common/extensions/docs/server2/find_broken_links.py:182: 
I would find this file easier to follow if it were done in an OO style.

BrokenLinkFinder -> construct with excluded files -> have a Find method which
takes the seed paths.

Actually, I just had a thought - another interesting thing to check for would be
public pages that aren't accessible from anywhere. Add that later, if ever, but
maybe we should call this class "Crawler" with "FindBrokenLinks" etc.

Hm, or maybe we should add that now? I say that because they're very similar
problems. I would actually prefer if Crawler took a FileSystem in its
constructor, FindBrokenLinks didn't have any arguments (just crawled for
publicly accessible files - maybe reuse logic from the integration tests?) - and
the "seed" logic used for implementing the FindOrphanedPages method.

https://codereview.chromium.org/17816005/diff/7001/chrome/common/extensions/d...
File chrome/common/extensions/docs/server2/handler.py (right):

https://codereview.chromium.org/17816005/diff/7001/chrome/common/extensions/d...
chrome/common/extensions/docs/server2/handler.py:48: return
Response.Ok('\n'.join(output))
This is kinda cool, I actually once did add a _test handler. You should do that
rather than hard-coding it in here, a path like

_test/integration

would run the integration tests.

And yeah, all we should really need is to run the integration tests right?

jshumway

Considerable rewrite, now finds a few more broken links (130) and and detects orphaned pages ...

7 years, 5 months ago (2013-07-05 18:48:48 UTC) #5

Considerable rewrite, now finds a few more broken links (130) and and detects
orphaned pages (52).

https://codereview.chromium.org/17816005/diff/7001/chrome/common/extensions/d...
File chrome/common/extensions/docs/server2/find_broken_links.py (right):

https://codereview.chromium.org/17816005/diff/7001/chrome/common/extensions/d...
chrome/common/extensions/docs/server2/find_broken_links.py:21: def
Memoize(func):
On 2013/07/01 23:16:21, kalman wrote:
> can you use the implementation from json schema compiler? and for that matter,
> use it with the decorator style wherever it needs to be used?

Oh right. I changed how most of the code works and now keep an explicit
dictionary instead.

https://codereview.chromium.org/17816005/diff/7001/chrome/common/extensions/d...
chrome/common/extensions/docs/server2/find_broken_links.py:139: varying number
of arguments, all of which can be converted to strings.
On 2013/07/01 23:16:21, kalman wrote:
> Just return the broken links in some format. The caller can decide how to
> present it.

Done.

https://codereview.chromium.org/17816005/diff/7001/chrome/common/extensions/d...
chrome/common/extensions/docs/server2/find_broken_links.py:182: 
On 2013/07/01 23:16:21, kalman wrote:
> I would find this file easier to follow if it were done in an OO style.
> 
> BrokenLinkFinder -> construct with excluded files -> have a Find method which
> takes the seed paths.

Restructured everything to be more object oriented. I have been doing quite a
bit of functional programming lately and it bled through a little bit, sorry.

> Actually, I just had a thought - another interesting thing to check for would
be
> public pages that aren't accessible from anywhere. Add that later, if ever,
but
> maybe we should call this class "Crawler" with "FindBrokenLinks" etc.
> 
> Hm, or maybe we should add that now? I say that because they're very similar
> problems. I would actually prefer if Crawler took a FileSystem in its
> constructor, FindBrokenLinks didn't have any arguments (just crawled for
> publicly accessible files - maybe reuse logic from the integration tests?) -
and
> the "seed" logic used for implementing the FindOrphanedPages method.

I was thinking of this as well. I rewrote a good section of the patch to instead
traverse the templates generating URLs, processing the contents of the files
from those URLs and checking for broken links. Then a separate method that does
a graph traversal to find disconnected nodes (orphaned pages).

not at google - send to devlin

feeling a bit ill doing this on the bus, so sending comments now. https://codereview.chromium.org/17816005/diff/13001/chrome/common/extensions/docs/server2/cron_servlet.py File ...

7 years, 5 months ago (2013-07-09 00:17:32 UTC) #6

jshumway

https://codereview.chromium.org/17816005/diff/13001/chrome/common/extensions/docs/server2/cron_servlet.py File chrome/common/extensions/docs/server2/cron_servlet.py (right): https://codereview.chromium.org/17816005/diff/13001/chrome/common/extensions/docs/server2/cron_servlet.py#newcode27 chrome/common/extensions/docs/server2/cron_servlet.py:27: ''' Create a mapping between all of the files ...

7 years, 5 months ago (2013-07-17 00:49:55 UTC) #7

https://codereview.chromium.org/17816005/diff/13001/chrome/common/extensions/...
File chrome/common/extensions/docs/server2/cron_servlet.py (right):

https://codereview.chromium.org/17816005/diff/13001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/cron_servlet.py:27: ''' Create a mapping
between all of the files in the directory of |dirpath|
On 2013/07/09 00:17:32, kalman wrote:
> '''Yields a tuple (url, path) for every file in |directory|, where the URL is
> given relative to |urlprefix|.
> 
> Can we move this function to a file like file_system_util.py?

Done.

https://codereview.chromium.org/17816005/diff/13001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/cron_servlet.py:30: for root, dirs, files
in file_system.Walk(dirpath):
On 2013/07/09 00:17:32, kalman wrote:
> s/dirs/_/

Done.

https://codereview.chromium.org/17816005/diff/13001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/cron_servlet.py:99: # TODO(kalman): delete
me where it's set.
On 2013/07/09 00:17:32, kalman wrote:
> could you move this todo into server_instance on content_cache, doesn't make
> sense here anymore.

Done.

https://codereview.chromium.org/17816005/diff/13001/chrome/common/extensions/...
File chrome/common/extensions/docs/server2/integration_test.py (right):

https://codereview.chromium.org/17816005/diff/13001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/integration_test.py:65: print "checking
for broken links..."
On 2013/07/09 00:17:32, kalman wrote:
> print(..) not print ..., print without () is deprecated.
> 
> also Checking not checking.

Done.

https://codereview.chromium.org/17816005/diff/13001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/integration_test.py:72: renderer,
On 2013/07/09 00:17:32, kalman wrote:
> maybe make this a lambda?

Done.

https://codereview.chromium.org/17816005/diff/13001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/integration_test.py:80: print 'finding %d
broken links took %s seconds' % (
On 2013/07/09 00:17:32, kalman wrote:
> likewise, and here Took %s seconds.

Done.

https://codereview.chromium.org/17816005/diff/13001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/integration_test.py:83: print 'searching
for orphan pages...'
On 2013/07/09 00:17:32, kalman wrote:
> likewise here and below.

Done.

https://codereview.chromium.org/17816005/diff/13001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/integration_test.py:87: print page
On 2013/07/09 00:17:32, kalman wrote:
> I think we can do a better job of being informative here.

Done.

https://codereview.chromium.org/17816005/diff/13001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/integration_test.py:131: print('Took %s
seconds' % (time.time() - start_time))
On 2013/07/09 00:17:32, kalman wrote:
> like I mentioned - can we make the test run here too, but just for that one
> file?
> 
> (provided it doesn't take too long)

I built out this functionality then discovered that LocalRenderer was too slow
(on the magnitude of a minute) to test a page dense with links. Thus a full cron
run would be required, which would bring the time down to 20 seconds on a good
computer which is still too slow. As we decided in IRC, I removed the check.

The error detection code is still structured in a way that it is easy (4-5
lines) to add a function that checks a single file back in, if in the future
everything is faster, or if we want to run the checks server side.

https://codereview.chromium.org/17816005/diff/13001/chrome/common/extensions/...
File chrome/common/extensions/docs/server2/link_error_detector.py (right):

https://codereview.chromium.org/17816005/diff/13001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/link_error_detector.py:16: ''' Find all
the links inside a section of the document with an id of
On 2013/07/09 00:17:32, kalman wrote:
> s/''' Find/'''Finds/

Done.

https://codereview.chromium.org/17816005/diff/13001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/link_error_detector.py:20: def
__init__(self, container_id, exclude):
On 2013/07/09 00:17:32, kalman wrote:
> what is exclude?

It used to be a way to tell the parser to ignore certain links, which I needed a
few versions of this patch ago to ignore examples and samples in some cases. It
is no longer needed.

https://codereview.chromium.org/17816005/diff/13001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/link_error_detector.py:32: if
attrs.get('id', '') == self._container_id or self._in_content:
On 2013/07/09 00:17:32, kalman wrote:
> the ", ''" part seems unnecessary.

indeed it is, removed.

https://codereview.chromium.org/17816005/diff/13001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/link_error_detector.py:34: 
On 2013/07/09 00:17:32, kalman wrote:
> no blank line here

Done.

https://codereview.chromium.org/17816005/diff/13001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/link_error_detector.py:37: if tag == 'a'
and href and not urlsplit(href).scheme in ('http', 'https'):
On 2013/07/09 00:17:32, kalman wrote:
> let's have:
> 
> if tag == 'a':
>   href = attrs.get('href')
>   if href and urlsplit(href) not in ('http', 'https'):
>     ...

Done.

https://codereview.chromium.org/17816005/diff/13001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/link_error_detector.py:39: if all([e not
in href for e in self._exclude]):
On 2013/07/09 00:17:32, kalman wrote:
> []s not needed, it can just be all(e not in href...)
> 
> or rather exclusion not in href for exclusion in...
> 
> though when does the href not end with .html or contain an anchor?
> 
> and speaking of which, can we just add a function somewhere which strips the
> anchor component (and I believe that urlparse can do it). You do it (or should
> do it) in a couple of places.

I created such a function.

There check for endswith .html was a little to inclusive here and has been
refined into ignoring .zip files and hrefs that contain javascript: .

https://codereview.chromium.org/17816005/diff/13001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/link_error_detector.py:44: 
On 2013/07/09 00:17:32, kalman wrote:
> no blank line here or below (I know, it's just that python looks weird with
> blank lines and I'm used to reading it without them)

Done.

https://codereview.chromium.org/17816005/diff/13001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/link_error_detector.py:52: class
LinkErrorDetector(object):
On 2013/07/09 00:17:32, kalman wrote:
> please write tests for this.

Done.

https://codereview.chromium.org/17816005/diff/13001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/link_error_detector.py:63: exclude=()):
On 2013/07/09 00:17:32, kalman wrote:
> explain these arguments

Done.

https://codereview.chromium.org/17816005/diff/13001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/link_error_detector.py:70:
self._ignore_detached = frozenset(('apps/404.html', 'extensions/404.html'))
On 2013/07/09 00:17:32, kalman wrote:
> always_detached?

Done.

https://codereview.chromium.org/17816005/diff/13001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/link_error_detector.py:139: url,
'gc-container' if url in self._root_pages else 'gc-pagecontent')
On 2013/07/09 00:17:32, kalman wrote:
> please tell me we can implement this without needing to look at gc-container
or
> gc-pagecontent. we might delete that some day and I don't want this test to
> mysteriously start breaking.

As we talked about in IRC, we can implement this without specific knowledge of
the IDs. It is  possible that a broken link in the sidenav or header would
generating hundreds of warnings for the same link and logic has been added to
squash such output into the form of "aoeu.html and 58 others ->
broken_link.html".

https://codereview.chromium.org/17816005/diff/13001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/link_error_detector.py:152: and an
optional third value with some debugging information.
On 2013/07/09 00:17:32, kalman wrote:
> Can we either always or never have a third argument? It seems useful to be
able
> to distinguish between a page not existing and an anchor within that page (or
> the current page) not existing. perhaps this third argument could be 'path' or
> 'anchor' and that is all. another alternative would for it to be the component
> of the path that wasn't found, but this seems a bit overdesigned.

The method now returns two lists, the first list contains only links that lead
to a page that 404s, the second contains only links with an invalid anchor. Each
list is made up of 2 item tuples.

https://codereview.chromium.org/17816005/diff/13001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/link_error_detector.py:166: # Then find
broken links with an anchor component.
On 2013/07/09 00:17:32, kalman wrote:
> use urlparse here?

Done.

https://codereview.chromium.org/17816005/diff/13001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/link_error_detector.py:182: def
GetOrphanPages(self, seed_urls=None):
On 2013/07/09 00:17:32, kalman wrote:
> Orphaned nor Orphan
> 
> Orphan sounds so sad :(

Agreed.

https://codereview.chromium.org/17816005/diff/13001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/link_error_detector.py:191: queue =
deque(seeds)
On 2013/07/09 00:17:32, kalman wrote:
> pages_to_check?

Done.

https://codereview.chromium.org/17816005/diff/13001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/link_error_detector.py:192: connected =
set(seeds)
On 2013/07/09 00:17:32, kalman wrote:
> found?

Done.

https://codereview.chromium.org/17816005/diff/13001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/link_error_detector.py:196: 
On 2013/07/09 00:17:32, kalman wrote:
> no blank line here

Done.

https://codereview.chromium.org/17816005/diff/13001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/link_error_detector.py:200:
queue.append(link)
On 2013/07/09 00:17:32, kalman wrote:
> if links were a set this would be a lot simpler.
> 
> more = self._pages[item].links - connected
> connected.update(more)
> queue.extend(more)

Perhaps. Is it better that is resembles the pseudo code for a breadth first
search, which is exactly what this method is? Would it be simpler to as

more = set(self._pages[item].links) -connected
connected...

the actual links field needs to be a list because the item are not unique.

https://codereview.chromium.org/17816005/diff/13001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/link_error_detector.py:202: connected |=
self._ignore_detached
On 2013/07/09 00:17:32, kalman wrote:
> instead of this, subtract ignore_detached from the comprehension below.

I moved a bunch of stuff around and | the set into connected at the top to keep
the comprehension on one line. I can rearrange things again if it would look
cleaner.

https://codereview.chromium.org/17816005/diff/13001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/link_error_detector.py:204: return [p for
p in set(self._pages.keys()) - connected if '.html' in p]
On 2013/07/09 00:17:32, kalman wrote:
> when does "if '.html' in p" fail? and see comment above about stripping the #.

In many places. png, zip, jpg, gif, py, js, css, txt, ico, and json are all
files that come up on the server without this exclusion.

The endswith('.html') check prevents quite a bit of output from the
FindOrphanedPages call, most of which is just noise that distracts from the most
helpful information: the list of orphaned html pages. I can leave the check out,
but then >200 of output is generated, which is a little unwieldy.

https://codereview.chromium.org/17816005/diff/13001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/link_error_detector.py:206: def
pprint_broken_links(links):
On 2013/07/09 00:17:32, kalman wrote:
> move this to integration_test - and call it PrettyPrintBrokenLinks (or inline
> it).

Done. Moved as a function because it is called twice.

not at google - send to devlin

lgtm https://codereview.chromium.org/17816005/diff/21001/chrome/common/extensions/docs/server2/integration_test.py File chrome/common/extensions/docs/server2/integration_test.py (right): https://codereview.chromium.org/17816005/diff/21001/chrome/common/extensions/docs/server2/integration_test.py#newcode49 chrome/common/extensions/docs/server2/integration_test.py:49: col_width = max([len(l[0]) for l in broken_links]) the ...

7 years, 5 months ago (2013-07-17 20:19:40 UTC) #8

jshumway

https://codereview.chromium.org/17816005/diff/21001/chrome/common/extensions/docs/server2/integration_test.py File chrome/common/extensions/docs/server2/integration_test.py (right): https://codereview.chromium.org/17816005/diff/21001/chrome/common/extensions/docs/server2/integration_test.py#newcode49 chrome/common/extensions/docs/server2/integration_test.py:49: col_width = max([len(l[0]) for l in broken_links]) On 2013/07/17 ...

7 years, 5 months ago (2013-07-17 22:15:47 UTC) #9

https://codereview.chromium.org/17816005/diff/21001/chrome/common/extensions/...
File chrome/common/extensions/docs/server2/integration_test.py (right):

https://codereview.chromium.org/17816005/diff/21001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/integration_test.py:49: col_width =
max([len(l[0]) for l in broken_links])
On 2013/07/17 20:19:40, kalman wrote:
> 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...

Done.

https://codereview.chromium.org/17816005/diff/21001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/integration_test.py:59: print("%s%s -> %s"
% (link, (col_width - len(link)) * ' ', target))
On 2013/07/17 20:19:40, kalman wrote:
> 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...

Done.

https://codereview.chromium.org/17816005/diff/21001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/integration_test.py:82: # Check for broken
links.
On 2013/07/17 20:19:40, kalman wrote:
> comment is redundant given the next line

Done.

https://codereview.chromium.org/17816005/diff/21001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/integration_test.py:94: len(broken_links)
+ len(broken_anchors)))
On 2013/07/17 20:19:40, kalman wrote:
> 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...

Done.

https://codereview.chromium.org/17816005/diff/21001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/integration_test.py:98:
_PrintBrokenLinks(broken_anchors)
On 2013/07/17 20:19:40, kalman wrote:
> make sure you actually fail the test at this point or chances are nobody will
> notice.

Prints a warning.

https://codereview.chromium.org/17816005/diff/21001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/integration_test.py:102: # Check for
orphaned pages.
On 2013/07/17 20:19:40, kalman wrote:
> comment also redundant

Done.

https://codereview.chromium.org/17816005/diff/21001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/integration_test.py:107: print('[%d
orphaned pages]' % len(orphaned_pages))
On 2013/07/17 20:19:40, kalman wrote:
> Found %d orphaned pages:

Done.

https://codereview.chromium.org/17816005/diff/21001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/integration_test.py:110: print('Took %s
seconds.' % (time.time() - start_time))
On 2013/07/17 20:19:40, kalman wrote:
> likewise, fail the test (self.fail maybe?)

Prints a warning.

https://codereview.chromium.org/17816005/diff/21001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/integration_test.py:154: # TODO: Check
page for broken links (currently prohibited by the time it
On 2013/07/17 20:19:40, kalman wrote:
> always name TODOs. so TODO(jshumway) here.

Done.

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jaredshumway94@gmail.com/17816005/28002

7 years, 5 months ago (2013-07-17 22:18:23 UTC) #10

Message was sent while issue was closed.

Change committed as 212240

Expand Messages | Collapse Messages