Waiting, it isn’t as simple as twiddling your thumbs
Just thought that it would be interesting post to talk about the evolution of a single method. This is used solely for the integration tests, by the way.
public void WaitForAllMessages() { while (mailBox.IsEmpty == false) { Thread.Sleep(100); } }
It started its life very simply, as just a method that waited until the mailbox was empty. But then we run into a problem, sometimes the method returned while a message was being processed, and that caused the tests to fail.
Then we had this:
public void WaitForAllMessages() { while (mailBox.IsEmpty == false || currentlyProcessingMessages > 0) { Thread.Sleep(100); } }
There are other pieces of the code, that update the currentlyProcessingMessages piece, but it isn’t really interesting. Some time passed, and we run into additional issues, which led to this beast:
public void WaitForAllMessages() { // we wait in case there are additional messages that have not yet arrived // to the listner. We arbitrarily define that if we wait for 5 consecutive // times with no new messages, there are no more messages int countOfTimesTriesToWaitAndThereWereNoMessages = 0; while(countOfTimesTriesToWaitAndThereWereNoMessages < 5) { if (WaitForAllMessagesInMemoryQueueOrProcessing()) countOfTimesTriesToWaitAndThereWereNoMessages = 0; else countOfTimesTriesToWaitAndThereWereNoMessages += 1; Thread.Sleep(100); } } private bool WaitForAllMessagesInMemoryQueueOrProcessing() { bool waited = false; while (mailBox.IsEmpty == false || currentlyProcessingMessages > 0) { waited = true; Thread.Sleep(100); } return waited; }
And that is where we are now.
Comments
Oren,
If you want to test this code, it's often helpful to replace the direct calls to Thread.Sleep() with interfaces and DI them. I've often suffered from nasty looking tests when Thread.Sleep was involved in the SUT unless I've done something like this.
Steve,
This is actually a code that is here to support the tests
Thumbs up for the aptly named "countOfTimesTriesToWaitAndThereWereNoMessages" . I would like to see more of that!
Just a small question, why do you do
countOfTimesTriesToWaitAndThereWereNoMessages += 1;
And not
countOfTimesTriesToWaitAndThereWereNoMessages++;
? Isn't the second example a bit more readable?
I find the first one more readable
www.nunit.org/index.php?p=delayedConstraint&r=2.5
This looks like some kind of job queue with threads, am I right?
Why don't you have a ManualResetEvent on the MailBox which gets set when the queue is empty and the last job has been processed? Then you'd only need this in your tests
MailBox.Emptied.WaitFor();
and the setting / resetting of the event could be done within the MailBox class using lock() in the relevant places.
I am always suspicious when I see a Thread.Sleep in a test...always :-)
Peter,
It is a message queue, yes.
That wouldn't work, I need to also wait for new messages that come into the queue after everything has processed.
The exit condition is:
queue is empty, all messages has been processed and no new messages came in for 0.5 second
Fixed my email address, sorry for the inconvenience :-)
Is there no way of knowing that a message is due to come in? The problem I see (being awkward) is this
"What happens if a message comes in after 0.51 seconds?"
Is there no central dispatcher or something which knows how many messages are dispatched, so you can wait until that many messages have been received?
Or maybe if your test knows how many messages it sends and waits for that many to turn up before completing?
Just throwing out ideas, I really can't stand Thread.Sleep in test code :-P
Peter,
This is used in the tests.
The way that the tests work is by spinning out a new scenario, which can be in a separate app domain or a separate process.
Except for the messages, there are no communication between the test code and the scenario. Trying to add one would significantly complicate the code.
Since the scenarios are one shot things (they execute only once), we can be sure that once they stopped sending messages,we are good
Comment preview