> See our ExecutorServiceRule which is in geode-junit. Is that similar to > what you're trying to create? Maybe we need just to > modify ExecutorServiceRule instead of introduce yet another Helper class. I > don't see why we need to add another class that is so similar to both > ExecutorServiceRule > and AsyncInvocation -- I vote to evolve those existing classes instead of > adding something new. >
I see this as potentially wrapping that ExecutorServiceRule (which, btw, I had no idea existed. Nice!). That rule lets you fire off asynchronous tasks. But generally a test doesn't want to just fire off tasks; it needs to get the results back and see what happened. For example, if you look at GMSEncryptJUnitTest which uses ExecutorServiceRule. It has to use a CountDownLatch to make sure the threads finish, and in fact it never checks to see if any of the threads throw an exception! With this helper that test could be more readable and would actually fail if it hit an error. Maybe this could be an another rule that embeds and ExecutorServiceRule, or just have a constructor that takes an executor service, to work with that existing rule? BTW, why does ExecutorServiceRule default to 1 thread? Unlimited threads seems like a more sensible default for tests that want an executor. -Dan
