Ejegg has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/342968 )
Change subject: Tests for the lock module
......................................................................
Tests for the lock module
Change-Id: I242a42518e1eaab5c8b039af2ac75615db9d01cc
---
M lock.py
M tests/data/timeout.yaml
M tests/test_job_wrapper.py
A tests/test_lock.py
M tox.ini
5 files changed, 101 insertions(+), 21 deletions(-)
Approvals:
Ejegg: Verified; Looks good to me, approved
diff --git a/lock.py b/lock.py
index 523b56c..67e2cdd 100644
--- a/lock.py
+++ b/lock.py
@@ -3,6 +3,7 @@
Self-corrects stale locks unless "failopen" is True.
'''
+from __future__ import print_function
import os
import os.path
import sys
@@ -17,7 +18,7 @@
filename = "/tmp/{name}.lock".format(name=job_tag)
if os.path.exists(filename):
- log.warn("Lockfile found!")
+ print("Lockfile found!", file=sys.stderr)
f = open(filename, "r")
pid = None
try:
@@ -26,18 +27,16 @@
pass
f.close()
if not pid:
- log.error("Invalid lockfile contents.")
+ print("Invalid lockfile contents.", file=sys.stderr)
else:
try:
os.getpgid(pid)
- log.error("Aborting! Previous process ({pid}) is still alive.
Remove lockfile manually if in error: {path}".format(pid=pid, path=filename))
- sys.exit(1)
+ raise LockError("Aborting! Previous process ({pid}) is still
alive. Remove lockfile manually if in error: {path}".format(pid=pid,
path=filename))
except OSError:
if failopen:
- log.fatal("Aborting until stale lockfile is investigated:
{path}".format(path=filename))
- sys.exit(1)
- log.error("Lockfile is stale.")
- log.info("Removing old lockfile.")
+ raise LockError("Aborting until stale lockfile is
investigated: {path}".format(path=filename))
+ print("Lockfile is stale.", file=sys.stderr)
+ print("Removing old lockfile.", file=sys.stderr)
os.unlink(filename)
f = open(filename, "w")
@@ -50,7 +49,11 @@
def end():
global lockfile
- if lockfile:
+ if lockfile and os.path.exists(lockfile):
os.unlink(lockfile)
else:
- raise RuntimeError("Already unlocked!")
+ raise LockError("Already unlocked!")
+
+
+class LockError(RuntimeError):
+ pass
diff --git a/tests/data/timeout.yaml b/tests/data/timeout.yaml
index 5dfe4ee..9e01920 100644
--- a/tests/data/timeout.yaml
+++ b/tests/data/timeout.yaml
@@ -1,3 +1,3 @@
name: Timing out job
-command: /bin/sleep 2
+command: /bin/sleep 10
timeout: 0.1
diff --git a/tests/test_job_wrapper.py b/tests/test_job_wrapper.py
index 463edbd..cea8ef6 100644
--- a/tests/test_job_wrapper.py
+++ b/tests/test_job_wrapper.py
@@ -1,5 +1,6 @@
import datetime
import iocapture
+import nose
import os
import job_wrapper
@@ -31,9 +32,9 @@
assert captured.stderr == "Job False job failed with code 1\n"
+# Must finish in less than two seconds, i.e. must have timed out.
+@nose.tools.timed(2)
def test_timeout():
- start_time = datetime.datetime.utcnow()
-
with iocapture.capture() as captured:
run_job("timeout.yaml")
@@ -42,11 +43,6 @@
"Job Timing out job timed out after 0.1 seconds\n"
"Job Timing out job failed with code -9\n"
)
-
- end_time = datetime.datetime.utcnow()
- delta = end_time - start_time
-
- assert delta.total_seconds() < 2
def test_stderr():
diff --git a/tests/test_lock.py b/tests/test_lock.py
new file mode 100644
index 0000000..cc3d71b
--- /dev/null
+++ b/tests/test_lock.py
@@ -0,0 +1,84 @@
+import nose
+import os.path
+
+import lock
+
+
+def tearDown():
+ # Clean up any old lockfiles.
+ if lock.lockfile != None:
+ if os.path.exists(lock.lockfile):
+ os.unlink(lock.lockfile)
+
+
+def test_success():
+ tag = "success"
+ lock.begin(job_tag=tag)
+ assert lock.lockfile
+ assert os.path.exists(lock.lockfile)
+
+ lock.end()
+ assert not os.path.exists(lock.lockfile)
+
+
+@nose.tools.raises(lock.LockError)
+def test_live_lock():
+ tag = "live"
+ lock.begin(job_tag=tag)
+
+ # Will die because the process (this one) is still running.
+ lock.begin(job_tag=tag)
+
+
+def test_stale_lock():
+ tag = "stale"
+ lock.begin(job_tag=tag)
+
+ # Make the lockfile stale by changing the process ID.
+ assert lock.lockfile
+ f = open(lock.lockfile, "w")
+ f.write("-1")
+ f.close()
+
+ lock.begin(job_tag=tag)
+
+ # Check that we overwrote the contents with the current PID.
+ assert lock.lockfile
+ f = open(lock.lockfile, "r")
+ pid = int(f.read())
+ f.close()
+ assert pid == os.getpid()
+
+ lock.end()
+
+
+def test_invalid_lock():
+ tag = "stale"
+ lock.begin(job_tag=tag)
+
+ # Make the lockfile invalid by changing the process ID to non-numeric.
+ assert lock.lockfile
+ f = open(lock.lockfile, "w")
+ f.write("ABC")
+ f.close()
+
+ lock.begin(job_tag=tag)
+
+ # Check that we overwrote the contents with the current PID.
+ assert lock.lockfile
+ f = open(lock.lockfile, "r")
+ pid = int(f.read())
+ f.close()
+ assert pid == os.getpid()
+
+ lock.end()
+
+
+@nose.tools.raises(lock.LockError)
+def test_double_unlock():
+ tag = "unlock-unlock"
+ lock.begin(job_tag=tag)
+ lock.end()
+
+ # Should throw an exception.
+ lock.end()
diff --git a/tox.ini b/tox.ini
index a26b3ba..e950c92 100644
--- a/tox.ini
+++ b/tox.ini
@@ -17,6 +17,3 @@
ignore=E501
exclude =
.tox
-
-[pytest]
-python_paths = .
--
To view, visit https://gerrit.wikimedia.org/r/342968
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I242a42518e1eaab5c8b039af2ac75615db9d01cc
Gerrit-PatchSet: 3
Gerrit-Project: wikimedia/fundraising/process-control
Gerrit-Branch: master
Gerrit-Owner: Awight <awi...@wikimedia.org>
Gerrit-Reviewer: Awight <awi...@wikimedia.org>
Gerrit-Reviewer: Ejegg <eeggles...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
