Github user tzulitai commented on a diff in the pull request: https://github.com/apache/flink/pull/5193#discussion_r158399966 --- Diff: flink-streaming-java/src/test/java/org/apache/flink/streaming/util/AbstractStreamOperatorTestHarness.java --- @@ -492,6 +503,10 @@ public void close() throws Exception { processingTimeService.shutdownService(); } setupCalled = false; + + if (internalEnvironment.isPresent()) { --- End diff -- Personal preference here, can ignore if you don't agree: I somehow find it more intuitive to just say: ``` if (environmentIsInternal) { environment.close(); } ``` i.e., we keep the boolean field `environmentIsInternal` instead of an optional field called `internalEnvironment`, which I personally got a bit confused with the other `environment` (while they are actually the same environment instance).
---