Code review comment for lp:~gary/z3c.recipe.filetemplate/support-system-python

On Jul 14, 2009, at 9:27 AM, Francis J. Lacoste wrote:

> Review: Approve
> On July 13, 2009, Gary Poster wrote:
>> In general, the necessity is caused by the mtime precision. On my
>> Mac, the precision is 1 second. Here's an example run of a slightly
>> modified version of the helper (adding a print), in the interactive
>> prompt.
>>
>>>>> def write_and_wait(dir, *args):
>> ... path = os.path.join(dir, *(args[:-1]))
>> ... original = os.stat(path).st_mtime
>> ... while os.stat(path).st_mtime == original:
>> ... print original
>> ... f = open(path, 'w')
>> ... f.write(args[-1])
>> ... f.flush()
>> ... os.fsync(f.fileno())
>> ... f.close()
>> ...
>>>>> f = open('/Users/gary/dev/deleteme', 'w')
>>>>> f.write('test')
>>>>> f.close()
>>>>> write_and_wait('/Users/gary/dev/deleteme', 'test')
>> 1247536625.0
>>>>> write_and_wait('/Users/gary/dev/deleteme', 'test')
>> 1247536627.0
>>>>> write_and_wait('/Users/gary/dev/deleteme', 'test')
>> 1247536628.0
>>>>> write_and_wait('/Users/gary/dev/deleteme', 'test')
>> 1247536629.0
>> 1247536629.0
>> 1247536629.0
>> [...there are 299 of these...]
>>>>>
>>
>> So, the precision is the point. I tried a variety of things to
>> trigger the mtime, but writing an empty string didn't work, and
>> directly writing the mtime is OS-specific/dependent, and I didn't
>> really want to go there. This is a test harness. Good enough, was
>> my
>> thinking.
>>
>> As far as all the flushing and so on, that's what Jim had for his
>> ``write`` helper function, so I suspect there's a point there too.
>> It
>> could probably be somewhat refactored but...it's a test harness.
>
> Simply add a docstring explaining that we use this helper to update
> files to
> make sure that the mtime of the file is updated so that buildout
> pick up the
> changes, with a note that the resolution of mtime is system
> dependant. (And
> possibly rename it to update_file() or something like that?)
>
> And instead of busy-waiting, why don't you add a time.sleep(0.5) in
> there?

OK, cool, I like the name change in particular.

I actually left out the time.sleep as a policy-ish decision, wanting
the tests to go as fast as possible, but it was an experiment, and I'm
happy to switch back to the time.sleep approach.

Incremental diff below.

Thanks

Gary

Index: z3c/recipe/filetemplate/tests.py
===================================================================
--- z3c/recipe/filetemplate/tests.py (revision 101900)
+++ z3c/recipe/filetemplate/tests.py (revision 101901)
@@ -13,14 +13,21 @@

##############################################################################

  import os
+import time
  import zc.buildout.testing
  import zc.buildout.tests
  from zope.testing import doctest

-def write_and_wait(dir, *args):
+def update_file(dir, *args):
+ """Update a file.
+
+ Make sure that the mtime of the file is updated so that buildout
notices
+ the changes. The resolution of mtime is system dependent, so we
keep
+ trying to write until mtime has actually changed."""
      path = os.path.join(dir, *(args[:-1]))
      original = os.stat(path).st_mtime
      while os.stat(path).st_mtime == original:
+ time.sleep(0.2)
          f = open(path, 'w')
          f.write(args[-1])
          f.flush()
@@ -29,7 +36,7 @@

  def setUp(test):
      zc.buildout.tests.easy_install_SetUp(test)
- test.globs['write_and_wait'] = write_and_wait
+ test.globs['update_file'] = update_file
      zc.buildout.testing.install_develop('z3c.recipe.filetemplate',
test)

  def test_suite():
Index: z3c/recipe/filetemplate/README.txt
===================================================================
--- z3c/recipe/filetemplate/README.txt (revision 101900)
+++ z3c/recipe/filetemplate/README.txt (revision 101901)
@@ -52,7 +52,7 @@
  If you need to escape the ${...} pattern, you can do so by repeating
the dollar
  sign.

- >>> write_and_wait(sample_buildout, 'helloworld.txt.in',
+ >>> update_file(sample_buildout, 'helloworld.txt.in',
      ... """
      ... Hello world! The double $${dollar-sign} escapes!
      ... """)
@@ -66,7 +66,7 @@

  Note that dollar signs alone, without curly braces, are not parsed.

- >>> write_and_wait(sample_buildout, 'helloworld.txt.in',
+ >>> update_file(sample_buildout, 'helloworld.txt.in',
      ... """
      ... $Hello $$world! $$$profit!
      ... """)