madecoste / swarming

Automatically exported from code.google.com/p/swarming
Apache License 2.0
0 stars 1 forks source link

tools.cached on listdir() has bad interactions with isolate_test.py. #175

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Repro:
1. On OSX, run ./tests/isolate_test.py

Expected:
Succeeds.

Actual:
Fails.  Applying the following patch clears the problem:
--- CUT HERE ---
--- a/utils/file_path.py
+++ b/utils/file_path.py
@@ -489,7 +489,6 @@ if sys.platform != 'win32':  # All non-Windows OSes.

 @tools.profile
-@tools.cached
 def listdir(abspath):
   """Lists a directory given an absolute path to it."""
   if not isabs(abspath):
--- CUT HERE ---

I'm not sure what is the best route here;
- Disabling caching in unit tests.
- Manually handling this issue in 
IsolateLoad.test_load_isolate_include_command_and_variables
- Having a way to clear the cache and clear it when relevant.

I created a check with:
--- CUT HERE ---
--- a/utils/tools.py
+++ b/utils/tools.py
@@ -260,11 +263,13 @@ def cached(func):

   @functools.wraps(func)
   def wrapper(*args):
-    v = cache.get(args, empty)
-    if v is empty:
-      v = func(*args)
-      cache[args] = v
-    return v
+    new = func(*args)
+    cached = cache.get(args, empty)
+    if cached is empty:
+      cache[args] = new
+    elif new != cached:
+      raise Exception('%r != %r' % (new, cached))
+    return new

   return wrapper
 --- CUT HERE ---

which does throw:
Exception: [u'bar', u'foo', u'x.isolate', u'x.isolated', 
u'x.isolated.gen.json', u'x.isolated.state', u'y.isolate', 
u'y.isolated.gen.json'] != [u'bar', u'foo', u'x.isolate', 
u'x.isolated.gen.json', u'y.isolate', u'y.isolated.gen.json']

since the .state is missing. I'll disable caching on listdir() for now.

In the meantime I'll disable cache on listdir().

Original issue reported on code.google.com by maruel@chromium.org on 21 Nov 2014 at 9:22

GoogleCodeExporter commented 9 years ago
Superseded by issue 181.

Original comment by maruel@chromium.org on 9 Jan 2015 at 7:57