tests: Separate tests for llnl.util.lock and spack.util.lock

- llnl.util.lock tests are now independent of Spack

lib/spack/spack/cmd/test.py

@@ -96,7 +96,7 @@ def test(parser, args, unknown_args):
         pytest.main(['-h'])
         return
 
-    # pytest.ini lives in the root of the spack repository.
+    # pytest.ini lives in lib/spack/spack/test
     with working_dir(spack.paths.test_path):
         # --list and --long-list print the test output better.
         if args.list or args.long_list:

lib/spack/spack/test/cmd/__init__.py

@@ -1,24 +0,0 @@
-##############################################################################
-# Copyright (c) 2013-2018, Lawrence Livermore National Security, LLC.
-# Produced at the Lawrence Livermore National Laboratory.
-#
-# This file is part of Spack.
-# Created by Todd Gamblin, tgamblin@llnl.gov, All rights reserved.
-# LLNL-CODE-647188
-#
-# For details, see https://github.com/spack/spack
-# Please also see the NOTICE and LICENSE files for our notice and the LGPL.
-#
-# This program is free software; you can redistribute it and/or modify
-# it under the terms of the GNU Lesser General Public License (as
-# published by the Free Software Foundation) version 2.1, February 1999.
-#
-# This program is distributed in the hope that it will be useful, but
-# WITHOUT ANY WARRANTY; without even the IMPLIED WARRANTY OF
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the terms and
-# conditions of the GNU Lesser General Public License for more details.
-#
-# You should have received a copy of the GNU Lesser General Public
-# License along with this program; if not, write to the Free Software
-# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
-##############################################################################

lib/spack/spack/test/llnl/util/lock.py

@@ -72,11 +72,9 @@
 
 import pytest
 
+import llnl.util.lock as lk
 import llnl.util.multiproc as mp
-from llnl.util.filesystem import touch, group_ids
+from llnl.util.filesystem import touch
-
-import spack.util.lock
-from spack.util.lock import Lock, WriteTransaction, ReadTransaction, LockError
 
 
 #
@@ -267,7 +265,7 @@ def wait(self):
 #
 def acquire_write(lock_path, start=0, length=0):
     def fn(barrier):
-        lock = Lock(lock_path, start, length)
+        lock = lk.Lock(lock_path, start, length)
         lock.acquire_write()  # grab exclusive lock
         barrier.wait()
         barrier.wait()  # hold the lock until timeout in other procs.
@@ -276,7 +274,7 @@ def fn(barrier):
 
 def acquire_read(lock_path, start=0, length=0):
     def fn(barrier):
-        lock = Lock(lock_path, start, length)
+        lock = lk.Lock(lock_path, start, length)
         lock.acquire_read()  # grab shared lock
         barrier.wait()
         barrier.wait()  # hold the lock until timeout in other procs.
@@ -285,9 +283,9 @@ def fn(barrier):
 
 def timeout_write(lock_path, start=0, length=0):
     def fn(barrier):
-        lock = Lock(lock_path, start, length)
+        lock = lk.Lock(lock_path, start, length)
         barrier.wait()  # wait for lock acquire in first process
-        with pytest.raises(LockError):
+        with pytest.raises(lk.LockError):
             lock.acquire_write(lock_fail_timeout)
         barrier.wait()
     return fn
@@ -295,9 +293,9 @@ def fn(barrier):
 
 def timeout_read(lock_path, start=0, length=0):
     def fn(barrier):
-        lock = Lock(lock_path, start, length)
+        lock = lk.Lock(lock_path, start, length)
         barrier.wait()  # wait for lock acquire in first process
-        with pytest.raises(LockError):
+        with pytest.raises(lk.LockError):
             lock.acquire_read(lock_fail_timeout)
         barrier.wait()
     return fn
@@ -552,7 +550,7 @@ def test_upgrade_read_to_write(private_lock_path):
     # to begin wtih.
     touch(private_lock_path)
 
-    lock = Lock(private_lock_path)
+    lock = lk.Lock(private_lock_path)
     assert lock._reads == 0
     assert lock._writes == 0
 
@@ -577,16 +575,14 @@ def test_upgrade_read_to_write(private_lock_path):
     assert lock._file is None
 
 
-#
-# Test that read-only file can be read-locked but not write-locked.
-#
 def test_upgrade_read_to_write_fails_with_readonly_file(private_lock_path):
-    # ensure lock file exists the first time, so we open it read-only
+    """Test that read-only file can be read-locked but not write-locked."""
-    # to begin wtih.
+    # ensure lock file exists the first time
     touch(private_lock_path)
 
+    # open it read-only to begin wtih.
     with read_only(private_lock_path):
-        lock = Lock(private_lock_path)
+        lock = lk.Lock(private_lock_path)
         assert lock._reads == 0
         assert lock._writes == 0
 
@@ -595,7 +591,8 @@ def test_upgrade_read_to_write_fails_with_readonly_file(private_lock_path):
         assert lock._writes == 0
         assert lock._file.mode == 'r'
 
-        with pytest.raises(LockError):
+        # upgrade to writ here
+        with pytest.raises(lk.LockError):
             lock.acquire_write()
 
 
@@ -605,7 +602,7 @@ def test_upgrade_read_to_write_fails_with_readonly_file(private_lock_path):
 #
 def test_complex_acquire_and_release_chain(lock_path):
     def p1(barrier):
-        lock = Lock(lock_path)
+        lock = lk.Lock(lock_path)
 
         lock.acquire_write()
         barrier.wait()  # ---------------------------------------- 1
@@ -613,7 +610,7 @@ def p1(barrier):
         barrier.wait()  # ---------------------------------------- 2
         lock.release_write()   # release and others acquire read
         barrier.wait()  # ---------------------------------------- 3
-        with pytest.raises(LockError):
+        with pytest.raises(lk.LockError):
             lock.acquire_write(lock_fail_timeout)
         lock.acquire_read()
         barrier.wait()  # ---------------------------------------- 4
@@ -622,9 +619,9 @@ def p1(barrier):
 
         # p2 upgrades read to write
         barrier.wait()  # ---------------------------------------- 6
-        with pytest.raises(LockError):
+        with pytest.raises(lk.LockError):
             lock.acquire_write(lock_fail_timeout)
-        with pytest.raises(LockError):
+        with pytest.raises(lk.LockError):
             lock.acquire_read(lock_fail_timeout)
         barrier.wait()  # ---------------------------------------- 7
         # p2 releases write and read
@@ -634,9 +631,9 @@ def p1(barrier):
         barrier.wait()  # ---------------------------------------- 9
         # p3 upgrades read to write
         barrier.wait()  # ---------------------------------------- 10
-        with pytest.raises(LockError):
+        with pytest.raises(lk.LockError):
             lock.acquire_write(lock_fail_timeout)
-        with pytest.raises(LockError):
+        with pytest.raises(lk.LockError):
             lock.acquire_read(lock_fail_timeout)
         barrier.wait()  # ---------------------------------------- 11
         # p3 releases locks
@@ -646,13 +643,13 @@ def p1(barrier):
         lock.release_read()
 
     def p2(barrier):
-        lock = Lock(lock_path)
+        lock = lk.Lock(lock_path)
 
         # p1 acquires write
         barrier.wait()  # ---------------------------------------- 1
-        with pytest.raises(LockError):
+        with pytest.raises(lk.LockError):
             lock.acquire_write(lock_fail_timeout)
-        with pytest.raises(LockError):
+        with pytest.raises(lk.LockError):
             lock.acquire_read(lock_fail_timeout)
         barrier.wait()  # ---------------------------------------- 2
         lock.acquire_read()
@@ -674,9 +671,9 @@ def p2(barrier):
         barrier.wait()  # ---------------------------------------- 9
         # p3 upgrades read to write
         barrier.wait()  # ---------------------------------------- 10
-        with pytest.raises(LockError):
+        with pytest.raises(lk.LockError):
             lock.acquire_write(lock_fail_timeout)
-        with pytest.raises(LockError):
+        with pytest.raises(lk.LockError):
             lock.acquire_read(lock_fail_timeout)
         barrier.wait()  # ---------------------------------------- 11
         # p3 releases locks
@@ -686,13 +683,13 @@ def p2(barrier):
         lock.release_read()
 
     def p3(barrier):
-        lock = Lock(lock_path)
+        lock = lk.Lock(lock_path)
 
         # p1 acquires write
         barrier.wait()  # ---------------------------------------- 1
-        with pytest.raises(LockError):
+        with pytest.raises(lk.LockError):
             lock.acquire_write(lock_fail_timeout)
-        with pytest.raises(LockError):
+        with pytest.raises(lk.LockError):
             lock.acquire_read(lock_fail_timeout)
         barrier.wait()  # ---------------------------------------- 2
         lock.acquire_read()
@@ -704,9 +701,9 @@ def p3(barrier):
 
         # p2 upgrades read to write
         barrier.wait()  # ---------------------------------------- 6
-        with pytest.raises(LockError):
+        with pytest.raises(lk.LockError):
             lock.acquire_write(lock_fail_timeout)
-        with pytest.raises(LockError):
+        with pytest.raises(lk.LockError):
             lock.acquire_read(lock_fail_timeout)
         barrier.wait()  # ---------------------------------------- 7
         # p2 releases write & read
@@ -736,9 +733,9 @@ def exit_fn(t, v, tb):
         vals['exited'] = True
         vals['exception'] = (t or v or tb)
 
-    lock = Lock(lock_path)
+    lock = lk.Lock(lock_path)
     vals = {'entered': False, 'exited': False, 'exception': False}
-    with ReadTransaction(lock, enter_fn, exit_fn):
+    with lk.ReadTransaction(lock, enter_fn, exit_fn):
         pass
 
     assert vals['entered']
@@ -746,7 +743,7 @@ def exit_fn(t, v, tb):
     assert not vals['exception']
 
     vals = {'entered': False, 'exited': False, 'exception': False}
-    with WriteTransaction(lock, enter_fn, exit_fn):
+    with lk.WriteTransaction(lock, enter_fn, exit_fn):
         pass
 
     assert vals['entered']
@@ -762,14 +759,14 @@ def exit_fn(t, v, tb):
         vals['exited'] = True
         vals['exception'] = (t or v or tb)
 
-    lock = Lock(lock_path)
+    lock = lk.Lock(lock_path)
 
     def do_read_with_exception():
-        with ReadTransaction(lock, enter_fn, exit_fn):
+        with lk.ReadTransaction(lock, enter_fn, exit_fn):
             raise Exception()
 
     def do_write_with_exception():
-        with WriteTransaction(lock, enter_fn, exit_fn):
+        with lk.WriteTransaction(lock, enter_fn, exit_fn):
             raise Exception()
 
     vals = {'entered': False, 'exited': False, 'exception': False}
@@ -801,11 +798,11 @@ def exit_fn(t, v, tb):
         vals['exited_fn'] = True
         vals['exception_fn'] = (t or v or tb)
 
-    lock = Lock(lock_path)
+    lock = lk.Lock(lock_path)
 
     vals = {'entered': False, 'exited': False, 'exited_fn': False,
             'exception': False, 'exception_fn': False}
-    with ReadTransaction(lock, TestContextManager, exit_fn):
+    with lk.ReadTransaction(lock, TestContextManager, exit_fn):
         pass
 
     assert vals['entered']
@@ -816,7 +813,7 @@ def exit_fn(t, v, tb):
 
     vals = {'entered': False, 'exited': False, 'exited_fn': False,
             'exception': False, 'exception_fn': False}
-    with ReadTransaction(lock, TestContextManager):
+    with lk.ReadTransaction(lock, TestContextManager):
         pass
 
     assert vals['entered']
@@ -827,7 +824,7 @@ def exit_fn(t, v, tb):
 
     vals = {'entered': False, 'exited': False, 'exited_fn': False,
             'exception': False, 'exception_fn': False}
-    with WriteTransaction(lock, TestContextManager, exit_fn):
+    with lk.WriteTransaction(lock, TestContextManager, exit_fn):
         pass
 
     assert vals['entered']
@@ -838,7 +835,7 @@ def exit_fn(t, v, tb):
 
     vals = {'entered': False, 'exited': False, 'exited_fn': False,
             'exception': False, 'exception_fn': False}
-    with WriteTransaction(lock, TestContextManager):
+    with lk.WriteTransaction(lock, TestContextManager):
         pass
 
     assert vals['entered']
@@ -861,14 +858,14 @@ def exit_fn(t, v, tb):
         vals['exited_fn'] = True
         vals['exception_fn'] = (t or v or tb)
 
-    lock = Lock(lock_path)
+    lock = lk.Lock(lock_path)
 
     def do_read_with_exception(exit_fn):
-        with ReadTransaction(lock, TestContextManager, exit_fn):
+        with lk.ReadTransaction(lock, TestContextManager, exit_fn):
             raise Exception()
 
     def do_write_with_exception(exit_fn):
-        with WriteTransaction(lock, TestContextManager, exit_fn):
+        with lk.WriteTransaction(lock, TestContextManager, exit_fn):
             raise Exception()
 
     vals = {'entered': False, 'exited': False, 'exited_fn': False,
@@ -910,91 +907,3 @@ def do_write_with_exception(exit_fn):
     assert vals['exception']
     assert not vals['exited_fn']
     assert not vals['exception_fn']
-
-
-def test_disable_locking(private_lock_path):
-    """Ensure that locks do no real locking when disabled."""
-    old_value = spack.config.get('config:locks')
-
-    with spack.config.override('config:locks', False):
-        lock = Lock(private_lock_path)
-
-        lock.acquire_read()
-        assert not os.path.exists(private_lock_path)
-
-        lock.acquire_write()
-        assert not os.path.exists(private_lock_path)
-
-        lock.release_write()
-        assert not os.path.exists(private_lock_path)
-
-        lock.release_read()
-        assert not os.path.exists(private_lock_path)
-
-    assert old_value == spack.config.get('config:locks')
-
-
-def test_lock_checks_user(tmpdir):
-    """Ensure lock checks work with a self-owned, self-group repo."""
-    uid = os.getuid()
-    if uid not in group_ids():
-        pytest.skip("user has no group with gid == uid")
-
-    # self-owned, own group
-    tmpdir.chown(uid, uid)
-
-    # safe
-    path = str(tmpdir)
-    tmpdir.chmod(0o744)
-    spack.util.lock.check_lock_safety(path)
-
-    # safe
-    tmpdir.chmod(0o774)
-    spack.util.lock.check_lock_safety(path)
-
-    # unsafe
-    tmpdir.chmod(0o777)
-    with pytest.raises(spack.error.SpackError):
-        spack.util.lock.check_lock_safety(path)
-
-    # safe
-    tmpdir.chmod(0o474)
-    spack.util.lock.check_lock_safety(path)
-
-    # safe
-    tmpdir.chmod(0o477)
-    spack.util.lock.check_lock_safety(path)
-
-
-def test_lock_checks_group(tmpdir):
-    """Ensure lock checks work with a self-owned, non-self-group repo."""
-    uid = os.getuid()
-    gid = next((g for g in group_ids() if g != uid), None)
-    if not gid:
-        pytest.skip("user has no group with gid != uid")
-
-    # self-owned, another group
-    tmpdir.chown(uid, gid)
-
-    # safe
-    path = str(tmpdir)
-    tmpdir.chmod(0o744)
-    spack.util.lock.check_lock_safety(path)
-
-    # unsafe
-    tmpdir.chmod(0o774)
-    with pytest.raises(spack.error.SpackError):
-        spack.util.lock.check_lock_safety(path)
-
-    # unsafe
-    tmpdir.chmod(0o777)
-    with pytest.raises(spack.error.SpackError):
-        spack.util.lock.check_lock_safety(path)
-
-    # safe
-    tmpdir.chmod(0o474)
-    spack.util.lock.check_lock_safety(path)
-
-    # safe
-    tmpdir.chmod(0o477)
-    spack.util.lock.check_lock_safety(path)

lib/spack/spack/test/util/spack_lock_wrapper.py

@@ -0,0 +1,123 @@
+##############################################################################
+# Copyright (c) 2013-2018, Lawrence Livermore National Security, LLC.
+# Produced at the Lawrence Livermore National Laboratory.
+#
+# This file is part of Spack.
+# Created by Todd Gamblin, tgamblin@llnl.gov, All rights reserved.
+# LLNL-CODE-647188
+#
+# For details, see https://github.com/spack/spack
+# Please also see the NOTICE and LICENSE files for our notice and the LGPL.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU Lesser General Public License (as
+# published by the Free Software Foundation) version 2.1, February 1999.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the IMPLIED WARRANTY OF
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the terms and
+# conditions of the GNU Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+##############################################################################
+"""Tests for Spack's wrapper module around llnl.util.lock."""
+import os
+
+import pytest
+
+from llnl.util.filesystem import group_ids
+
+import spack.config
+import spack.util.lock as lk
+
+
+def test_disable_locking(tmpdir):
+    """Ensure that locks do no real locking when disabled."""
+    lock_path = str(tmpdir.join('lockfile'))
+
+    old_value = spack.config.get('config:locks')
+
+    with spack.config.override('config:locks', False):
+        lock = lk.Lock(lock_path)
+
+        lock.acquire_read()
+        assert not os.path.exists(lock_path)
+
+        lock.acquire_write()
+        assert not os.path.exists(lock_path)
+
+        lock.release_write()
+        assert not os.path.exists(lock_path)
+
+        lock.release_read()
+        assert not os.path.exists(lock_path)
+
+    assert old_value == spack.config.get('config:locks')
+
+
+def test_lock_checks_user(tmpdir):
+    """Ensure lock checks work with a self-owned, self-group repo."""
+    uid = os.getuid()
+    if uid not in group_ids():
+        pytest.skip("user has no group with gid == uid")
+
+    # self-owned, own group
+    tmpdir.chown(uid, uid)
+
+    # safe
+    path = str(tmpdir)
+    tmpdir.chmod(0o744)
+    lk.check_lock_safety(path)
+
+    # safe
+    tmpdir.chmod(0o774)
+    lk.check_lock_safety(path)
+
+    # unsafe
+    tmpdir.chmod(0o777)
+    with pytest.raises(spack.error.SpackError):
+        lk.check_lock_safety(path)
+
+    # safe
+    tmpdir.chmod(0o474)
+    lk.check_lock_safety(path)
+
+    # safe
+    tmpdir.chmod(0o477)
+    lk.check_lock_safety(path)
+
+
+def test_lock_checks_group(tmpdir):
+    """Ensure lock checks work with a self-owned, non-self-group repo."""
+    uid = os.getuid()
+    gid = next((g for g in group_ids() if g != uid), None)
+    if not gid:
+        pytest.skip("user has no group with gid != uid")
+
+    # self-owned, another group
+    tmpdir.chown(uid, gid)
+
+    # safe
+    path = str(tmpdir)
+    tmpdir.chmod(0o744)
+    lk.check_lock_safety(path)
+
+    # unsafe
+    tmpdir.chmod(0o774)
+    with pytest.raises(spack.error.SpackError):
+        lk.check_lock_safety(path)
+
+    # unsafe
+    tmpdir.chmod(0o777)
+    with pytest.raises(spack.error.SpackError):
+        lk.check_lock_safety(path)
+
+    # safe
+    tmpdir.chmod(0o474)
+    lk.check_lock_safety(path)
+
+    # safe
+    tmpdir.chmod(0o477)
+    lk.check_lock_safety(path)