[GitHub] flink pull request #5535: [FLINK-8703][tests] Migrate tests from LocalFlinkM...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/flink/pull/5535


---

[GitHub] flink pull request #5535: [FLINK-8703][tests] Migrate tests from LocalFlinkM...

[aljoscha]

Github user aljoscha commented on a diff in the pull request:

https://github.com/apache/flink/pull/5535#discussion_r169589778
  
--- Diff: 
flink-tests/src/test/java/org/apache/flink/test/misc/SuccessAfterNetworkBuffersFailureITCase.java
 ---
@@ -50,22 +50,27 @@
 public class SuccessAfterNetworkBuffersFailureITCase extends TestLogger {
 
private static final int PARALLELISM = 16;
+
+   @ClassRule
+   public static final MiniClusterResource MINI_CLUSTER_RESOURCE = new 
MiniClusterResource(
+   new MiniClusterResource.MiniClusterResourceConfiguration(
+   getConfiguration(),
+   2,
--- End diff --

This could be derived from `PARALLELISM`


---

[GitHub] flink pull request #5535: [FLINK-8703][tests] Migrate tests from LocalFlinkM...

[aljoscha]

Github user aljoscha commented on a diff in the pull request:

https://github.com/apache/flink/pull/5535#discussion_r169589443
  
--- Diff: 
flink-libraries/flink-gelly/src/test/java/org/apache/flink/graph/test/operations/DegreesWithExceptionITCase.java
 ---
@@ -38,29 +32,10 @@
  * Test expected errors for {@link Graph#inDegrees()},
  * {@link Graph#outDegrees()}, and {@link Graph#getDegrees()}.
  */
-public class DegreesWithExceptionITCase extends TestLogger {
+public class DegreesWithExceptionITCase extends AbstractTestBase {
 
private static final int PARALLELISM = 4;
--- End diff --

I think this could be replaced in all the tests by making 
`DEFAULT_PARALLELISM` in `AbstractTestBase` protected and using that instead.


---

[GitHub] flink pull request #5535: [FLINK-8703][tests] Migrate tests from LocalFlinkM...

[zentol]

Github user zentol commented on a diff in the pull request:

https://github.com/apache/flink/pull/5535#discussion_r169370975
  
--- Diff: 
flink-tests/src/test/java/org/apache/flink/test/misc/MiscellaneousIssuesITCase.java
 ---
@@ -134,7 +106,8 @@ public void testAccumulatorsAfterNoOp() {
final String accName = "test_accumulator";
 
try {
-   env.setParallelism(6);
+   ExecutionEnvironment env = 
ExecutionEnvironment.getExecutionEnvironment();
+   env.setParallelism(4);
--- End diff --

yeah, that's probably the better option.


---

[GitHub] flink pull request #5535: [FLINK-8703][tests] Migrate tests from LocalFlinkM...

[aljoscha]

Github user aljoscha commented on a diff in the pull request:

https://github.com/apache/flink/pull/5535#discussion_r169369295
  
--- Diff: 
flink-tests/src/test/java/org/apache/flink/test/misc/MiscellaneousIssuesITCase.java
 ---
@@ -134,7 +106,8 @@ public void testAccumulatorsAfterNoOp() {
final String accName = "test_accumulator";
 
try {
-   env.setParallelism(6);
+   ExecutionEnvironment env = 
ExecutionEnvironment.getExecutionEnvironment();
+   env.setParallelism(4);
--- End diff --

Why not start a mini cluster with 2 TMs and 3 slots as before?


---

[GitHub] flink pull request #5535: [FLINK-8703][tests] Migrate tests from LocalFlinkM...

[zhangminglei]

Github user zhangminglei commented on a diff in the pull request:

https://github.com/apache/flink/pull/5535#discussion_r169346590
  
--- Diff: 
flink-tests/src/test/java/org/apache/flink/test/streaming/experimental/CollectITCase.java
 ---
@@ -38,33 +35,23 @@
  * This experimental class is relocated from flink-streaming-contrib. 
Please see package-info.java
  * for more information.
  */
-public class CollectITCase extends TestLogger {
+public class CollectITCase extends AbstractTestBase {
 
--- End diff --

Thanks @zentol.


---

[GitHub] flink pull request #5535: [FLINK-8703][tests] Migrate tests from LocalFlinkM...

[zentol]

Github user zentol commented on a diff in the pull request:

https://github.com/apache/flink/pull/5535#discussion_r169341512
  
--- Diff: 
flink-tests/src/test/java/org/apache/flink/test/streaming/experimental/CollectITCase.java
 ---
@@ -38,33 +35,23 @@
  * This experimental class is relocated from flink-streaming-contrib. 
Please see package-info.java
  * for more information.
  */
-public class CollectITCase extends TestLogger {
+public class CollectITCase extends AbstractTestBase {
 
--- End diff --

Please have a look at the javadocs for AbstractTestBase.

The logs should still be printed as `AbstractTestBase` extends 
`TestBaseUtils` which extends `TestLogger`.


---

[GitHub] flink pull request #5535: [FLINK-8703][tests] Migrate tests from LocalFlinkM...

[zhangminglei]

Github user zhangminglei commented on a diff in the pull request:

https://github.com/apache/flink/pull/5535#discussion_r169328894
  
--- Diff: 
flink-tests/src/test/java/org/apache/flink/test/streaming/experimental/CollectITCase.java
 ---
@@ -38,33 +35,23 @@
  * This experimental class is relocated from flink-streaming-contrib. 
Please see package-info.java
  * for more information.
  */
-public class CollectITCase extends TestLogger {
+public class CollectITCase extends AbstractTestBase {
 
--- End diff --

One thing I found the difference from them , If we extends AbstractTestBase 
class. No logs will print to console. 


---

[GitHub] flink pull request #5535: [FLINK-8703][tests] Migrate tests from LocalFlinkM...

[zhangminglei]

Github user zhangminglei commented on a diff in the pull request:

https://github.com/apache/flink/pull/5535#discussion_r169327476
  
--- Diff: 
flink-tests/src/test/java/org/apache/flink/test/streaming/experimental/CollectITCase.java
 ---
@@ -38,33 +35,23 @@
  * This experimental class is relocated from flink-streaming-contrib. 
Please see package-info.java
  * for more information.
  */
-public class CollectITCase extends TestLogger {
+public class CollectITCase extends AbstractTestBase {
 
--- End diff --

It seems that we do not use anything in AbstractTestBase class. Why do we 
still extend this class ?


---

[GitHub] flink pull request #5535: [FLINK-8703][tests] Migrate tests from LocalFlinkM...

[zentol]

Github user zentol commented on a diff in the pull request:

https://github.com/apache/flink/pull/5535#discussion_r169312549
  
--- Diff: 
flink-tests/src/test/java/org/apache/flink/test/misc/MiscellaneousIssuesITCase.java
 ---
@@ -134,7 +106,8 @@ public void testAccumulatorsAfterNoOp() {
final String accName = "test_accumulator";
 
try {
-   env.setParallelism(6);
+   ExecutionEnvironment env = 
ExecutionEnvironment.getExecutionEnvironment();
+   env.setParallelism(4);
--- End diff --

i had to reduce the parallelism to 4 as it failed with 6 due to not enough 
slots.


---

[GitHub] flink pull request #5535: [FLINK-8703][tests] Migrate tests from LocalFlinkM...

[zentol]

GitHub user zentol opened a pull request:

https://github.com/apache/flink/pull/5535

[FLINK-8703][tests] Migrate tests from LocalFlinkMiniCluster to 
MiniClusterResource

## What is the purpose of the change

This PR contains the first batch of test ports from `LocalFlinkMiniCluster` 
to `MiniClusterResource`. No port required changes to `MiniClusterResource`.

Each test has it's own commit. If a test was not explicitly creating a 
configuration and the number of taskmanagers/slots matched the settings in 
`AbstractTestBase` the test was modified to extend that.

Otherwise, a separate `MiniClusterResource` `@ClassRule` was created.

## Verifying this change

This change is covered by existing tests.


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/zentol/flink 8703

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/flink/pull/5535.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #5535


commit 2b4168c932304d0a855d330cfe902b361cf7b174
Author: zentol 
Date:   2018-02-20T12:41:45Z

[FLINK-8703][tests] Port CollectITCase to AbstractTestBase

commit f00f69ca1eb347d4d235cb24bb1d49656548cc98
Author: zentol 
Date:   2018-02-20T12:42:01Z

[FLINK-8703][tests] Port MiscellaneuousIssuesITCase to AbstractTestBase

commit e6b8751b982e3b80c74b7e93ef26e60cd9059b37
Author: zentol 
Date:   2018-02-20T12:43:01Z

[FLINK-8703][tests] Port StreamFaultToleranceTestBase to MiniClusterResource

commit a2163be631695f78da7f40cebae1c62a3016fe17
Author: zentol 
Date:   2018-02-20T12:43:17Z

[FLINK-8703][tests] Port KeyedStateCheckpointITCase to MiniClusterResource

commit f85d9577df4002e98c34606984ee69573a78a6f7
Author: zentol 
Date:   2018-02-20T12:43:34Z

[FLINK-8703][tests] Port AccumulatorErrorITCase to AbstractTestBase

commit 06dfdb7c75298c1b501599a25e7fdae93de92c5f
Author: zentol 
Date:   2018-02-20T12:46:10Z

[FLINK-8703][tests] Port FastFailuresITCase to MiniClusterResource

commit 12e29e0fec70824415216ee738853d45b204fc03
Author: zentol 
Date:   2018-02-20T12:58:40Z

[FLINK-8703][tests] Port SimpleRecoveryITCaseBase to MiniClusterResource




---