Allow creating a CuratorTestingServerExtension that starts the testing server in the constructor?

[sleberknight]

Discussed in #238

^{Originally posted by sleberknight July 14, 2021}

Background

The original (i.e. pre-GitHub) implementation of CuratorTestingServerExtension did the following in its constructor:

testingServer = new TestingServer(port);

The above is equivalent to:

testingServer = new TestingServer(port, true);  // here, true means to start the server *right now*

When we re-implemented this in kiwi-test, we chose to do this in the constructor:

testingServer = new TestingServer(port, false);  // here, false means we will NOT start the server now

And then in the beforeAll we called testingServer.start(). Doing it this way is more correct in the way JUnit Jupiter extensions should behave.

However, this caused an issue when testing Dropwizard services using the DropwizardAppExtension, specifically because Dropwizard does not follow typical Jupiter extension conventions. It looks something like this when we're using the Dropwizard extension and the CuratorTestingServerExtension:

@ExtendWith(DropwizardExtensionsSupport.class)
class AppTest {

  private static final DropwizardAppExtension<AppConfig> APP =
          new DropwizardAppExtension<>(App.class, ResourceHelpers.resourceFilePath("config-app-test.yml"));

  private static final int ZOOKEEPER_TEST_SERVER_PORT = 37_456;

  @RegisterExtension
  static final CuratorTestingServerExtension ZOOKEEPER_TEST_SERVER =
          new CuratorTestingServerExtension(ZOOKEEPER_TEST_SERVER_PORT);

  // tests...

}

But when using the kiwiproject extension, the above does not work because there is no way to ensure the Curator TestingServer starts before the Dropwizard extension launches the test application. As a result, the App classes under test can never connect to a the Curator testing Zookeeper server, so we end up mocking things like leader latches inside App by doing something like:

public static class MockLeaderLatchApp extends App {
  @Override
  protected ManagedLeaderLatch buildManagedLeaderLatch(CuratorFramework curator) {
    return mock(ManagedLeaderLatch.class);
  }
}

//  and then...

  private static final DropwizardAppExtension<AppConfig> APP =
          new DropwizardAppExtension<>(MockLeaderLatchApp.class, ResourceHelpers.resourceFilePath("config-app-test.yml"));

This is not ideal, and doesn't test the actual leader latch in the App class.

To allow us to use the Dropwizard extension and the Curator testing extension, we could add a new constructor in the kiwiproject CuratorTestingServerExtension that accepts a second argument that forces the testing server to start immediately, i.e. it would call new TestingServer(port, true).

While this is not the "correct" way to do most Jupiter extensions, we don't have a choice in this case if we want to use it with the (nonstandard) Dropwizard extension mechanism. So we might do something like:

  @RegisterExtension
  static final CuratorTestingServerExtension ZK_TEST_SERVER =
          new CuratorTestingServerExtension(ZK_TEST_SERVER_PORT, TestingServerStart.IMMEDIATE);

where ServerStart is perhaps an enum inside CuratorTestingServerExtension:

public enum TestingServerStart {

  /** Start the Curator TestingServer immediately. */
  IMMEDIATE,

  /** Start the Curator TestingServer when beforeAll is called (this is the normal JUnit Jupiter lifecycle) */
  BEFORE_ALL
}

Again, while this isn't ideal and not the recommended way, we would no longer need to create the MockLeaderLatchApp and mock things out when we're trying to do integration testing.

[sleberknight]

Note that the solution in #242 doesn't add additional constructors and the enum mentioned in the original discussion in this issue. Instead, after initially trying that route (i.e. using the TestingServerStart enum), it didn't look good, and I changed to use two new static factory methods instead:

• newStartingImmediately()
• newStartingImmediately(int port)

Doing this avoided a few issues. First, using the enum forced adding multiple new constructors, one that accepted the enum and a second one that accepted the enum and a port in order to cover the various cases. Second, and more importantly, the existing constructors already cover the situation when you want to use the regular JUnit Jupiter extension lifecycle. Having the enum contain a BEFORE_ALL constant is therefore not necessary, and redundant. Therefore, the only useful enum constant is IMMEDIATE, and because having an enum with only one constant is a bit silly, it seemed to make more sense to create the static factory methods to cover the "immediate start" use case.

And the code reads better (to me anyway). For example:

@RegisterExtension
static final CuratorTestingServerExtension ZK_TEST_SERVER = 
        CuratorTestingServerExtension.newStartingImmediately();

// or

@RegisterExtension
static final CuratorTestingServerExtension ZK_TEST_SERVER = 
        CuratorTestingServerExtension.newStartingImmediately(9090);