Speed up integration tests.

[big-andy-coates]

Description

Our integration tests were taking up 3/4s of our build time. So I spent a little time speeding them up.

• Kafka clusters are now mostly per-class, rather than per-test.
• I've found and fixed tests that incorrectly waiting for more messages than were being published.
• I've added ConsumerGroupTestUtil to help tests to have unique consumer groups per test, so tests don't interact with each other.
• I've added TopicTestUtil to help tests to have unique topic names per test.
• I've added KsqlIndentifierTestUtil to help tests tp have unique KSQL identifiers, (think steam/table names), per test.
• I've make IntergrationTestHarness a JUnit rule, so it can be used with @ClassRule.
• I've make a TestKsqlContext, which is a JUnit rule, to simplify interacting with the KsqlContext.
• I've added ConsumerTestUtil, which has lots of handy methods for consuming messages from Kafka and asserting if too few or too many are consumed.
• I've refactored the integration tests to make use of the above new functionality.

So now, to have an integration test, all you really need is something like:

public class MyIntTest {

  private static final OrderDataProvider DATA_PROVIDER = new OrderDataProvider();

  @ClassRule
  public static final IntegrationTestHarness TEST_HARNESS = IntegrationTestHarness.build();

  @Rule
  public final TestKsqlContext ksqlContext = TEST_HARNESS.buildKsqlContext();

  private String sourceTopic;
  private String resultStream;

  @Before
  public void setUp() throws Exception {
    sourceTopic = TopicTestUtil.uniqueTopicName("source");
    resultStream = KsqlIdentifierTestUtil.uniqueIdentifierName("OUTPUT");

    TEST_HARNESS.produceRows(sourceTopic, DATA_PROVIDER, AVRO);
  }

  @Test
  public void should() {
    // Given:
    ksqlContext
        .sql("CREATE STREAM FOO WITH(KAFKA_TOPIC='" + sourceTopic + "', VALUE_FORMAT='AVRO');");

    // When:
    ksqlContext.sql("CREATE STREAM " + resultStream + " AS SELECT * FROM FOO;");

    // Then:
    TEST_HARNESS.verifyAvailableRows(resultStream, DATA_PROVIDER.data().size(), AVRO, DATA_PROVIDER.schema());
  }
}

The result of this work has brought build times mvn clean install down from something like:

[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 21:37 min
[INFO] Finished at: 2018-12-18T16:03:45Z
[INFO] ------------------------------------------------------------------------

To

[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 09:58 min
[INFO] Finished at: 2018-12-18T16:21:08Z
[INFO] ------------------------------------------------------------------------

Happy days.

note: One of the tests remains unstable. (The Window one I think). This still needs investigating.

Testing done

Test only change.

Reviewer checklist

• Ensure docs are updated if necessary. (eg. if a user visible feature is being added or changed).
• Ensure relevant issues are linked (description should include text like "Fixes #")

[apurvam]

great PR @big-andy-coates .. Will definitely boost productivity once merged.

I left a couple of comments, but this looks great besides those.

[apurvam]

I know we don't support embedded mode, but KsqlContext is the way to use embedded mode, right? If so, won't this break existing embedded mode users, especially if the rely on the return value being void, like:

   void someFunction() {
      return ksqlContext.sql(myQuery);
   }

[big-andy-coates]

I don't follow.

First off, I didn't think we'd really supported embedded mode as such. It's not documented anywhere that I'm aware of.

Second, how would they be relying on it returning void? Last time I checked:

void someFunction() {
      return ksqlContext.sql(myQuery);
   }

Will result in a compilation error, even if sql returns void.

I think you've been a manager too long!

[apurvam]

Regarding return void types, that is my C++ instinct talking. This is in fact valid C++, but not valid Java:

void myFunction() {
  return;
}

void someOtherFunction() {
  return myFunction();
}

int main() {
  someOtherFunction();
  return 0;
}

Also, while we haven't officially advertised embedded mode, we have unofficially recommended it as an option people can use and pointed to internal integration tests as examples to follow.

If we are going to break it, I think it is definitely worth adding a release note to that effect at the very least.

However, given the above, I am not sure that this change breaks anything.

[apurvam]

LGTM, thanks!

[big-andy-coates]

Test only change, so merging with one review...