Shutdown is not deterministic - race condition in ExecutorService shutdown

[Mahoney]

We're using the analytics client in our tests as so:

// before test
val analytics: Analytics = Analytics
  .builder(writeKey.value)
  .endpoint(wireMockUrl.toASCIIString())
  .build()

// during test
val message = TrackMessage
      .builder("my-event-name")

analytics.enqueue(message)

// before asserting anything
analytics.flush()
analytics.shutdown()

with a mock HTTP client that just gathers all requests.

We are seeing our tests be flaky because sometimes that message never hits the mock server.

Looking at AnalyticsClient.java I'm pretty sure it's possible for the call to AnalyticsClient.shutdown() to shutdown the networkExecutor before the looper has submitted the last batch to it; you never interrogate the Future<?> returned when you call looperExecutor.submit(new Looper()) so the RejectedExecutionException thrown when Looper calls networkExecutor.submit(BatchUploadTask) after the networkExecutor has been shutdown is silently swallowed.

[Mahoney]

I suspect the fundamental issue for us is we are running our tests at a high concurrency level and your thread factory sets the thread priority to minimum for all your executors, so the hardcoded 1 second wait for the looper ExecutorService to exit before shutting down the networkExecutor can be insufficient.