Docserver: Properly implement the Cron logic for ContentProvider and TemplateDataSource so that the…

Docserver: Properly implement the Cron logic for ContentProvider and
TemplateDataSource so that the nacl docs and partial templates are correctly
pulled in by the cronjob. Make the integration tests actually test nacl docs.

BUG=339936
R=yoz@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=248847

[not at google - send to devlin, 2014-01-31 22:42:04]

This patch depends on https://codereview.chromium.org/149513014/

[Yoyo Zhou, 2014-02-04 00:48:42]

https://chromiumcodereview.appspot.com/151773002/diff/90001/chrome/common/ext...
File chrome/common/extensions/docs/server2/integration_test.py (right):

https://chromiumcodereview.appspot.com/151773002/diff/90001/chrome/common/ext...
chrome/common/extensions/docs/server2/integration_test.py:50: dirs[:] = [d for d
in dirs if not d.startswith('.')]
Do you need to check if any of the path components start with . (because then
they will be hidden)? Or do you only care about . and ..?

https://chromiumcodereview.appspot.com/151773002/diff/90001/chrome/common/ext...
chrome/common/extensions/docs/server2/integration_test.py:166: path)
If path is a message, it should have some ... message text.

https://chromiumcodereview.appspot.com/151773002/diff/90001/chrome/common/ext...
File chrome/common/extensions/docs/server2/template_renderer.py (right):

https://chromiumcodereview.appspot.com/151773002/diff/90001/chrome/common/ext...
chrome/common/extensions/docs/server2/template_renderer.py:39: # TODO(kalman):
Figure out where "peperversion" comes from. It's used
Is this a typo?

[not at google - send to devlin, 2014-02-04 21:29:49]

Comments addressed... and I thought I rebased in a previous patch to make the
diff good, but apparently I failed at that.

Fixed one little problem in template_data_source causing Session.vim files being
compiled as HTML.

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

https://codereview.chromium.org/151773002/diff/90001/chrome/common/extensions...
chrome/common/extensions/docs/server2/integration_test.py:50: dirs[:] = [d for d
in dirs if not d.startswith('.')]
On 2014/02/04 00:48:42, Yoyo Zhou wrote:
> Do you need to check if any of the path components start with . (because then
> they will be hidden)? Or do you only care about . and ..?

These . checks are actually for hidden files. I'll make that clearer, and you're
right, this check is not quite right.

https://codereview.chromium.org/151773002/diff/90001/chrome/common/extensions...
chrome/common/extensions/docs/server2/integration_test.py:166: path)
On 2014/02/04 00:48:42, Yoyo Zhou wrote:
> If path is a message, it should have some ... message text.

Done.

https://codereview.chromium.org/151773002/diff/90001/chrome/common/extensions...
File chrome/common/extensions/docs/server2/template_renderer.py (right):

https://codereview.chromium.org/151773002/diff/90001/chrome/common/extensions...
chrome/common/extensions/docs/server2/template_renderer.py:39: # TODO(kalman):
Figure out where "peperversion" comes from. It's used
On 2014/02/04 00:48:42, Yoyo Zhou wrote:
> Is this a typo?

Done.

[Yoyo Zhou, 2014-02-04 21:42:13]

https://codereview.chromium.org/151773002/diff/180001/chrome/common/extension...
File chrome/common/extensions/docs/server2/future.py (right):

https://codereview.chromium.org/151773002/diff/180001/chrome/common/extension...
chrome/common/extensions/docs/server2/future.py:12: |future|.
futures

https://codereview.chromium.org/151773002/diff/180001/chrome/common/extension...
File chrome/common/extensions/docs/server2/integration_test.py (right):

https://codereview.chromium.org/151773002/diff/180001/chrome/common/extension...
chrome/common/extensions/docs/server2/integration_test.py:43: '''Returns a list
of the non-hidden paths from |paths|
nit: trailing .

https://codereview.chromium.org/151773002/diff/180001/chrome/common/extension...
chrome/common/extensions/docs/server2/integration_test.py:47: return [path for
path in paths if (not path.startswith('.')) or
What I meant by 'any component starting with .' is:

.foo/bar/baz: hidden
foo/.bar/baz: hidden
foo/bar/.baz: hidden
etc.
So I think this only filters out the first of these.

https://codereview.chromium.org/151773002/diff/180001/chrome/common/extension...
chrome/common/extensions/docs/server2/integration_test.py:176: '"%s" did not
(temporary) redirect to "%s.html"' % (path, path))
nit: temporarily

[not at google - send to devlin, 2014-02-04 21:57:13]

https://chromiumcodereview.appspot.com/151773002/diff/180001/chrome/common/ex...
File chrome/common/extensions/docs/server2/future.py (right):

https://chromiumcodereview.appspot.com/151773002/diff/180001/chrome/common/ex...
chrome/common/extensions/docs/server2/future.py:12: |future|.
On 2014/02/04 21:42:13, Yoyo Zhou wrote:
> futures

Done.

https://chromiumcodereview.appspot.com/151773002/diff/180001/chrome/common/ex...
File chrome/common/extensions/docs/server2/integration_test.py (right):

https://chromiumcodereview.appspot.com/151773002/diff/180001/chrome/common/ex...
chrome/common/extensions/docs/server2/integration_test.py:43: '''Returns a list
of the non-hidden paths from |paths|
On 2014/02/04 21:42:13, Yoyo Zhou wrote:
> nit: trailing .

Done.

https://chromiumcodereview.appspot.com/151773002/diff/180001/chrome/common/ex...
chrome/common/extensions/docs/server2/integration_test.py:47: return [path for
path in paths if (not path.startswith('.')) or
On 2014/02/04 21:42:13, Yoyo Zhou wrote:
> What I meant by 'any component starting with .' is:
> 
> .foo/bar/baz: hidden
> foo/.bar/baz: hidden
> foo/bar/.baz: hidden
> etc.
> So I think this only filters out the first of these.

Ah right. So - you're right in that this method would only filter out the first
of those. However the place its used, inside a Walk invocation, means that all 3
will eventually be filtered out. In the second case because Walk will descend
into 'foo' with a directory '.bar', then remove that directory. Likewise the
3rd.

https://chromiumcodereview.appspot.com/151773002/diff/180001/chrome/common/ex...
chrome/common/extensions/docs/server2/integration_test.py:176: '"%s" did not
(temporary) redirect to "%s.html"' % (path, path))
On 2014/02/04 21:42:13, Yoyo Zhou wrote:
> nit: temporarily

done, and then a bit to make it <= 80 chars :)

[Yoyo Zhou, 2014-02-04 21:59:56]

LGTM

https://chromiumcodereview.appspot.com/151773002/diff/180001/chrome/common/ex...
File chrome/common/extensions/docs/server2/integration_test.py (right):

https://chromiumcodereview.appspot.com/151773002/diff/180001/chrome/common/ex...
chrome/common/extensions/docs/server2/integration_test.py:47: return [path for
path in paths if (not path.startswith('.')) or
On 2014/02/04 21:57:13, kalman wrote:
> On 2014/02/04 21:42:13, Yoyo Zhou wrote:
> > What I meant by 'any component starting with .' is:
> > 
> > .foo/bar/baz: hidden
> > foo/.bar/baz: hidden
> > foo/bar/.baz: hidden
> > etc.
> > So I think this only filters out the first of these.
> 
> Ah right. So - you're right in that this method would only filter out the
first
> of those. However the place its used, inside a Walk invocation, means that all
3
> will eventually be filtered out. In the second case because Walk will descend
> into 'foo' with a directory '.bar', then remove that directory. Likewise the
> 3rd.

Ah, I see. I should've actually tried out os.walk. =)

[not at google - send to devlin, 2014-02-05 04:25:21]

Committed patchset #9 manually as r248847.