xintongsong commented on a change in pull request #11248: [FLINK-16299] Release
containers recovered from previous attempt in w…
URL: https://github.com/apache/flink/pull/11248#discussion_r385525529
##########
File path:
flink-yarn/src/test/java/org/apache/flink/yarn/YarnResourceManagerTest.java
##########
@@ -176,9 +179,37 @@ public void teardown() throws Exception {
}
}
+ static class TestingNMClientAsync extends NMClientAsync {
+
+ public List<ContainerStatus> containerStatuses = new
ArrayList<>();
+
+ protected TestingNMClientAsync(CallbackHandler callbackHandler)
{
+ super(callbackHandler);
+ }
+
+ @Override
+ public void startContainerAsync(Container container,
ContainerLaunchContext containerLaunchContext) {
+ // Do nothing.
+ }
+
+ @Override
+ public void stopContainerAsync(ContainerId containerId, NodeId
nodeId) {
+ // Do nothing.
+ }
+
+ @Override
+ public void getContainerStatusAsync(ContainerId containerId,
NodeId nodeId) {
+ for (ContainerStatus containerStatus:
containerStatuses) {
+ if
(containerStatus.getContainerId().equals(containerId)) {
+
callbackHandler.onContainerStatusReceived(containerId, containerStatus);
+ }
+ }
+ }
+ }
Review comment:
Let's make this class more general by using `Function` and `Consumer` to
define its behaviors. Something like the following, taking
`getContainerStatusAsync` as an example.
```
static class TestingNMClientAsync extends NMClientAsync {
// ...
private final BiFunction<ContainerId, NodeId, ContainerStatus>
getContainerStatusAsyncFunction;
// ...
public void getContainerStatusAsync(ContainerId containerId, NodeId
nodeId) {
callbackHandler.onContainerStatusReceived(containerId,
getContainerStatusAsyncFunction.apply(containerId, NodeId));
}
// ...
}
```
We can use a builder class to create this class, allowing setting custom
`Function` and `Consumer`. If the codes for this class grows too much, we can
also put it in a separate file.
There are several benefit for doing this.
- It allows defining per-test-case behavior, which makes is easier to reuse
this class in the future.
- It avoids using `mock`, `spy` and `verify`.
- It avoids having a public accessible `containerStatuses`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
