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).
---
