geode-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Kirk Lund <kl...@apache.org>
Subject Re: [DISCUSS] Deprecating JUnit4CacheTestCase (and maybe JUnit4DistributedTestCase?)
Date Fri, 09 Nov 2018 19:31:28 GMT
*I would like to encourage all Geode developers to start writing tests
directly against the Geode User APIs* even in DistributedTests. I'm no
longer using *CacheRule, ClientCacheRule, ServerStarterRule,
LocatorStarterRule, or ClusterStarterRule* and I'm against encouraging
their use any longer. I'll explain why below.

Here's an example for an IntegrationTest that needs a Cache but not any
CacheServers:

private Cache cache;

@Before
public void setUp() {
  Properties config = new Properties();
  config.setProperty(LOCATORS, "");
  cache = new CacheFactory(config).create();
}

@After
public void tearDown() {
  cache.close();
}

That's some pretty simple code and as a Developer, I can tell exactly what
it's doing and what the config is.

Here's an example of the kind of Geode User API code that I use to create
Servers in a DistributedTests now:

  private void createServer(String serverName, File serverDir, int
locatorPort) {
    ServerLauncher.Builder builder = new ServerLauncher.Builder();
    builder.setMemberName(serverName);
    builder.setWorkingDirectory(serverDir.getAbsolutePath());
    builder.setServerPort(0);
    builder.set(LOCATORS, "localHost[" + locatorPort + "]");
    builder.set(DISABLE_AUTO_RECONNECT, "false");
    builder.set(ENABLE_CLUSTER_CONFIGURATION, "false");
    builder.set(MAX_WAIT_TIME_RECONNECT, "1000");
    builder.set(MEMBER_TIMEOUT, "2000");

    serverLauncher = builder.build();
    serverLauncher.start();
    assertThat(serverLauncher.isRunning()).isTrue();
  }

In particular, I think we should be using ServerLauncher and
LocatorLauncher instead of Rules when we want a full-stack Locator or
full-stack Server that looks like what a User is going to startup.

Here are my reasons:

1) I want to learn and use the Geode User APIs directly, not someone's
(even mine) Testing API that hides the Geode User APIs. If I see a test
fail, I want to see exactly what was configured and what User APIs were
used right there in the test without having to open other classes. I don't
want to have to spend even 15 minutes digging through some JUnit Rule to
figure out how PDX was configured.

2) We need to make sure that the Geode User APIs are easy to use and are
complete. If we're writing tests against Testing APIs instead then we don't
feel the Users' pain if our API is painful. If the reason to use a Rule is
because our User API is overly-verbose of difficult, then that's even more
reason to use the Geode User API, so we recognize that it needs to change!

GemFire had a long history of hiding its User APIs behind elaborate Testing
APIs and we all used these fancy, easier to use, more compact Testing APIs.
This promotes complicated, inconsistent and potentially incomplete User
APIs for Users to actually use. The result: difficult to use product with
difficult to use APIs and User APIs that are missing important things that
then Users have to resort to internal APIs to use. I'm strongly convinced
that using elaborate Testing APIs is at least largely responsible for
making GemFire and now Geode difficult to use and that's why I'm pushing so
hard to write tests with Geode User APIs instead of convenient custom Rules

Since I started using ServerLauncher and LocatorLauncher APIs directly in
my DistributedTests I made a very important discovery: the User has no way
to get a reference to the Cache. This is why I recently started a
discussion thread about add getCache and getLocator to these Launcher APIs.
If we keep using elaborate Testing APIs including custom Geode JUnit Rules
to hide these APIs, we'll never make these discoveries that I feel are
vital for our Users. We need to make things a LOT easier for the Users
going forward.

The above is why I think we should be using User APIs in the tests even for
setUp and tearDown. Save the custom JUnit Rules for NON-GEODE things like
configuring JSON or LOG4J or allowing use of ErrorCollector in all DUnit
VMs.

Thanks,
Kirk

On Fri, Nov 9, 2018 at 10:49 AM, Galen O'Sullivan <gosullivan@pivotal.io>
wrote:

> I was looking at a test recently that extended JUnit4CacheTestCase and only
> used a single server, and changed it to use ClusterStartupRule.
>
> JUnit4CacheTestCase adds additional complexity to JUnit4DistributedTestCase
> and with the move to ClusterStartupRule for distributed tests, rather than
> class inheritance, I think we should deprecate JUnit4CacheTestCase and
> change the comment to imply that classes should inherit from it just
> because they require a Cache.
>
> Is is worth deprecating JUnit4DistributedTestCase as well and encouraging
> the use of ClusterStartupRule instead?
>
> Thanks,
> Galen
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message