Remove RunnableOnService with tests with ExpectedException

[tgroh]

These tests aren't truly portable across runners, as excepitons thorwn
on failure may be different.

[lukecwik]

Not according to the Beam TestPipeline docs:
https://github.com/apache/incubator-beam/blob/master/sdks/java/core/src/main/java/org/apache/beam/sdk/testing/TestPipeline.java#L79

• For pipeline runners, it is required that they must throw an {@link AssertionError}

• containing the message from the {@link PAssert} that failed.

[kennknowles]

I agree that the exception raised by a runner in this situation should be predictable enough to at least check assertions. (though I'm not sure TestPipeline has the authority to add specs - should move that spec to PipelineRunner)

Can you be specific about what problem arose?

[tgroh]

The expected exceptions expect a PipelineExecutionException which contains the underlying cause (in this case an IllegalStateException). The exception we get back from the TestDataflowPipelineRunner in case of Pipeline failure is not the exception that caused the Pipeline to fail, but a broader exception that the pipeline failed.

Additionally, the documented behavior for the Dataflow SDK TestPipeline makes no specification on the expected exceptions that are thrown in the case of failure: https://github.com/GoogleCloudPlatform/DataflowJavaSDK/blob/master/sdk/src/main/java/com/google/cloud/dataflow/sdk/testing/TestPipeline.java#L48

The run method is documented to unwrap any AssertionError that is thrown during testing, but the failure that is returned by the TestDataflowPipelineRunner is a locally constructed IllegalStateException

[lukecwik]

We can remove it but in reality TestPipeline should throw the underlying cause but because the only runner here is Dataflow and it doesn't support this right now. Can be added if we ever get to adding support for this here.