Two methods which can't run concurrently, how to write unit tests for this case, or make it testable?

Question

Let's say I have two methods in my class MethodA and MethodB. Those methods are run in async, and I don't want that they run at the same time. In the app, MethodA is called by the user, but MethodB is run by a Timer in a background thread.

How I see the implementation in C#ish pseudocode:

class MyClass { private static object _lock = new object(); public async Task MethodA() { lock(_lock) { await DoSomeStuff(); } } public async Task MethodB() { if(Monitor.IsEntered(_lock) return; lock(_lock) { await DoSomeStuff(); } } }

So first question is - is above approach correct? I guess that's more a question for https://codereview.stackexchange.com/.

So the second questions is - assuming that the approach is correct, how I can unit test, that MethodA waits for MethodB, and that MethodB doesn't run when MethodA is running? Or how can I refactor it so that's testable?

EDIT: accordingly to comments, changed from using flags to a lock.

What is WaitForBToFinish ? Can you show that please, it seems to be a critical method. + How are you calling MethodA and MethodB ? — Zein Makki
– Zein Makki, Commented Aug 15, 2016 at 14:39
The problem with flags is if both the method enter at the same time, you could get past the check and have them running concurrently. Can't you just use a lock instead? — DavidG
– DavidG, Commented Aug 15, 2016 at 14:46
There is no reason, my current concern is to make this testable, I changed the "implementation" to use a lock. — alek kowalczyk
– alek kowalczyk, Commented Aug 15, 2016 at 14:54

Zein Makki · Accepted Answer · 2016-08-15 15:14:54Z

Boolean flags are the obsolete way to synchronize two threads. It causes race conditions when one thread can read a value of false while other thread is updating the value to true;

Since your case it not straightforward (B shouldn't way for B to end, while A should wait), then I would change the class use a Semaphore like this:

public class MyClass { private SemaphoreSlim semaphore = new SemaphoreSlim(1); public async Task MethodA() { await semaphore.WaitAsync(); await DoSomeStuff(); semaphore.Release(); } public async Task MethodB() { bool success = await semaphore.WaitAsync(1); if (!success) return; await DoSomeStuff(); await Task.Delay(TimeSpan.FromSeconds(5)); semaphore.Release(); } }

I would consider putting all that in try..catch..finally block and release the semaphore in the finally block, but i'm trying to keep it simple while you can add that yourself.

Unit testing:

This is not straight forward to test. Taking threads into account, you might need to repeat the test multiple times to reach a case of failure. You might need to introduce an overload for method A that waits for some times, which might prove that method B is waiting for it. Here is the test. To test the case of failure, change new SemaphoreSlim(1); to new SemaphoreSlim(2); and the test would fail because MethodB would start before MethodA ends.

[TestMethod] public async Task TestingMyClassThreadSync() { int repeatTimes = 100; int counter = 0; while (counter++ <= repeatTimes) { MyClass myClass = new MyClass(); Task tA = myClass.MethodA(); Task tB = myClass.MethodB(); Task finishedTask = await Task.WhenAny(tA, tB); bool bFinishedBeforeA = finishedTask == tA; if (bFinishedBeforeA) Assert.Fail(); } }

I would introduce an overload:

public async Task MethodA(long waitMilliseconds = 0) { await semaphore.WaitAsync(); await DoSomeStuff(); await Task.Delay(waitMilliseconds); semaphore.Release(); }

Then Call it from unit testing as MethodA(5000);

just wanted to edit it to something like you wrote after realizing that await is not allowed in a lock :)
but do you thing it's possible to somehow test this scenarios in unit tests?
@alekkowalczyk I'm editing the question now to post on how to test this.
@alekkowalczyk you can return Task<bool> from MethodB in order to check if it really executed or just returned due to A being running. you can then return success from MethodB and check the condition to if (bFinishedBeforeA) && finishedTask.Result)
@alekkowalczyk Another approach would be to wait for method A to finish inside methodB, but not call DoSomeStuff and just end MethodB After that.

Collectives™ on Stack Overflow

Two methods which can't run concurrently, how to write unit tests for this case, or make it testable?

1 Answer 1

Unit testing:

7 Comments

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

Unit testing:

7 Comments

Related