Merge lp:~jtv/juju-core/generic-locking-test into lp:~go-bot/juju-core/trunk
Status: | Merged |
---|---|
Approved by: | Jeroen T. Vermeulen |
Approved revision: | no longer in the source branch. |
Merged at revision: | 1314 |
Proposed branch: | lp:~jtv/juju-core/generic-locking-test |
Merge into: | lp:~go-bot/juju-core/trunk |
Diff against target: |
259 lines (+201/-1) 4 files modified
environs/azure/environ.go (+34/-1) environs/azure/environ_test.go (+51/-0) testing/locking.go (+71/-0) testing/locking_test.go (+45/-0) |
To merge this branch: | bzr merge lp:~jtv/juju-core/generic-locking-test |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Tim Penhey (community) | Approve | ||
Review via email: mp+170477@code.launchpad.net |
Commit message
Helper: test a function for correct locking.
This is the outcome of discussion at
https:/
but produces a helper that is much, much simpler. It could become generic,
although one thing is still missing: it currently only works for locks that
are of type sync.Mutex. It probably ought to work for any type of lock.
Failure of a function to acquire the right lock(s) is a constant risk in
concurrent programs, so one of the things most worth testing, but it's also
relatively hard to test for and so nobody ever does it. Of course if you
can forget the locking you can also forget the test, but this helper makes it
very easy to guard against regressions.
Description of the change
Helper: test a function for correct locking.
This is the outcome of discussion at
https:/
but produces a helper that is much, much simpler. It could become generic,
although one thing is still missing: it currently only works for locks that
are of type sync.Mutex. It probably ought to work for any type of lock.
Failure of a function to acquire the right lock(s) is a constant risk in
concurrent programs, so one of the things most worth testing, but it's also
relatively hard to test for and so nobody ever does it. Of course if you
can forget the locking you can also forget the test, but this helper makes it
very easy to guard against regressions.
LGTM for what it is, but might be nice to have as a checker.
https:/ /codereview. appspot. com/10433043/ diff/1/ environs/ azure/environ_ test.go azure/environ_ test.go (right):
File environs/
https:/ /codereview. appspot. com/10433043/ diff/1/ environs/ azure/environ_ test.go# newcode39 azure/environ_ test.go: 39: func testLockingFunc tion(lock
environs/
*sync.Mutex, function func()) {
If this is to be a generic testing function, you should move it to the
testing package and export it.
Here is a thought. Want to write this as a checker?
c.Assert(some_func, ObservesLockOn( some_mutex) ) ?
https:/ /codereview. appspot. com/10433043/ diff/1/ environs/ azure/environ_ test.go# newcode60 azure/environ_ test.go: 60: // misbehaved "function" plenty of
environs/
rope to hang itself.
You could have a few sleep(0) type calls, as this should do the same
thing.
https:/ /codereview. appspot. com/10433043/ diff/1/ environs/ azure/environ_ test.go# newcode74 azure/environ_ test.go: 74: blankLock := sync.Mutex{}
environs/
Wow, surprised there is not a way to ask if a mutex is locked.
https:/ /codereview. appspot. com/10433043/