Merge pull request #1657 from EDCD/fix/tests/TestJournalLock.test_release_lock

Finally(?) ensure all the tests work, no contamination or races

tests/journal_lock.py/test_journal_lock.py

@@ -1,33 +1,4 @@
-"""
-Tests for journal_lock.py code.
-
-Tests:
- - Is file actually locked after obtain_lock().  Problem: We opened the
-    file in a manner which means nothing else can open it.  Also I assume
-    that the same process will either be allowed to lock it 'again' or
-    overwrite the lock.
-
-    Expected failures if:
-
-     1. Lock already held (elsewhere).
-     2. Can't open lock file 'w+'.
-     3. Path to lock file doesn't exist.
-     4. journaldir is None (default on Linux).
-
- - Does release_lock() work?  Easier to test, if it's worked....
-     1. return True if not locked.
-     2. return True if locked, but successful unlock.
-     3. return False otherwise.
-
- - JournalLock.set_path_from_journaldir
-     1. When journaldir is None.
-     2. Succeeds otherwise?
-
- - Can any string to pathlib.Path result in an invalid path for other
-   operations?
-
- - Not sure about testing JournalAlreadyLocked class.
-"""
+"""Tests for journal_lock.py code."""
 import multiprocessing as mp
 import os
 import pathlib
@@ -70,6 +41,7 @@ def other_process_lock(continue_q: mp.Queue, exit_q: mp.Queue, lockfile: pathlib
         exit_q.get(block=True, timeout=None)
 
         # And clean up
+        _release_lock('sub-process', lf)
         os.unlink(lockfile / 'edmc-journal-lock.txt')
 
 
@@ -104,6 +76,40 @@ def _obtain_lock(prefix: str, filehandle) -> bool:
             return False
 
     return True
+
+
+def _release_lock(prefix: str, filehandle) -> bool:
+    """
+    Release the JournalLock.
+
+    :param prefix: str - what to prefix output with.
+    :param filehandle: File handle already open on the lockfile.
+    :return: bool - True if we released the lock.
+    """
+    if sys.platform == 'win32':
+        print(f'{prefix}: On win32')
+        import msvcrt
+        try:
+            print(f'{prefix}: Trying msvcrt.locking() ...')
+            filehandle.seek(0)
+            msvcrt.locking(filehandle.fileno(), msvcrt.LK_UNLCK, 4096)
+
+        except Exception as e:
+            print(f'{prefix}: Unable to unlock file: {e!r}')
+            return False
+
+    else:
+        import fcntl
+
+        print(f'{prefix}: Not win32, using fcntl')
+        try:
+            fcntl.flock(filehandle, fcntl.LOCK_UN)
+
+        except Exception as e:
+            print(f'{prefix}: Unable to unlock file: {e!r}')
+            return False
+
+    return True
 ###########################################################################
 
 
@@ -292,9 +298,13 @@ class TestJournalLock:
         # Now attempt to lock with to-test code
         jlock = JournalLock()
         second_attempt = jlock.obtain_lock()
-        # Fails on Linux, because flock(2) is per process, so we'd need to
-        # use multiprocessing to test this.
         assert second_attempt == JournalLockResult.ALREADY_LOCKED
+
+        # Need to release any handles on the lockfile else the sub-process
+        # might not be able to clean up properly, and that will impact
+        # on later tests.
+        jlock.journal_dir_lockfile.close()
+
         print('Telling sub-process to quit...')
         exit_q.put('quit')
         print('Waiting for sub-process...')
@@ -316,6 +326,7 @@ class TestJournalLock:
         # And finally check it actually IS unlocked.
         with open(mock_journaldir.getbasetemp() / 'edmc-journal-lock.txt', mode='w+') as lf:
             assert _obtain_lock('release-lock', lf)
+            assert _release_lock('release-lock', lf)
 
         # Cleanup, to avoid side-effect on other tests
         os.unlink(str(jlock.journal_dir_lockfile_name))