You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by azagrebin <gi...@git.apache.org> on 2018/04/25 12:07:55 UTC
[GitHub] flink pull request #5912: [FLINK-9041] Refactor StreamTaskTest to not use sc...
GitHub user azagrebin opened a pull request:
https://github.com/apache/flink/pull/5912
[FLINK-9041] Refactor StreamTaskTest to not use scala and akka
## What is the purpose of the change
Get rid of scala/akka dependency in StreamTaskTest
## Brief change log
`StreamTaskTest.testEarlyCanceling()` and `StreamTaskTest.TestingExecutionStateListener` are refactored to use java 8 `CompletableFuture` instead of scala `Promise`
## Verifying this change
This change is already covered by existing tests, as it just refactors them.
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): (yes: javassist 3.18.2-GA to 3.19.0-GA - [reason](https://github.com/powermock/powermock/issues/729))
- The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (no)
- The serializers: (no)
- The runtime per-record code paths (performance sensitive): (no)
- Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (no)
- The S3 file system connector: (no)
## Documentation
- Does this pull request introduce a new feature? (no)
- If yes, how is the feature documented? (not applicable)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/azagrebin/flink FLINK-9041
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/5912.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 #5912
----
commit 0b119c931fd3e0c83eaaa4d858b775b57cc41d3b
Author: Andrey Zagrebin <an...@...>
Date: 2018-04-25T09:02:09Z
[FLINK-9041] Refactor StreamTaskTest to use java 8 CompletableFuture instead of scala/akka Promise
----
---
[GitHub] flink issue #5912: [FLINK-9041] Refactor StreamTaskTest to not use scala and...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:
https://github.com/apache/flink/pull/5912
alrighty, merging this.
---
[GitHub] flink pull request #5912: [FLINK-9041] Refactor StreamTaskTest to not use sc...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/5912#discussion_r184048468
--- Diff: pom.xml ---
@@ -452,7 +452,7 @@ under the License.
<dependency>
<groupId>org.javassist</groupId>
<artifactId>javassist</artifactId>
- <version>3.18.2-GA</version>
+ <version>3.19.0-GA</version>
--- End diff --
This is probably caused by the usage of `Comparator.comparingInt` along with mocking.
We probably would run into this in the future anyway at some point, might as well fix it now...
---
[GitHub] flink pull request #5912: [FLINK-9041] Refactor StreamTaskTest to not use sc...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/5912#discussion_r184034886
--- Diff: flink-streaming-java/src/test/java/org/apache/flink/streaming/runtime/tasks/StreamTaskTest.java ---
@@ -858,25 +856,17 @@ public void close() throws Exception {
private ExecutionState executionState = null;
- private final PriorityQueue<Tuple2<ExecutionState, Promise<ExecutionState>>> priorityQueue = new PriorityQueue<>(
- 1,
- new Comparator<Tuple2<ExecutionState, Promise<ExecutionState>>>() {
- @Override
- public int compare(Tuple2<ExecutionState, Promise<ExecutionState>> o1, Tuple2<ExecutionState, Promise<ExecutionState>> o2) {
- return o1.f0.ordinal() - o2.f0.ordinal();
- }
- });
+ private final PriorityQueue<Tuple2<ExecutionState, CompletableFuture<ExecutionState>>> priorityQueue =
+ new PriorityQueue<>(1, Comparator.comparingInt(o -> o.f0.ordinal()));
--- End diff --
please avoid formatting changes
---
[GitHub] flink pull request #5912: [FLINK-9041] Refactor StreamTaskTest to not use sc...
Posted by azagrebin <gi...@git.apache.org>.
Github user azagrebin commented on a diff in the pull request:
https://github.com/apache/flink/pull/5912#discussion_r184043414
--- Diff: flink-streaming-java/src/test/java/org/apache/flink/streaming/runtime/tasks/StreamTaskTest.java ---
@@ -178,7 +172,7 @@
*/
@Test
public void testEarlyCanceling() throws Exception {
- Deadline deadline = new FiniteDuration(2, TimeUnit.MINUTES).fromNow();
--- End diff --
thanks, changed
---
[GitHub] flink pull request #5912: [FLINK-9041] Refactor StreamTaskTest to not use sc...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/5912#discussion_r184034760
--- Diff: pom.xml ---
@@ -452,7 +452,7 @@ under the License.
<dependency>
<groupId>org.javassist</groupId>
<artifactId>javassist</artifactId>
- <version>3.18.2-GA</version>
+ <version>3.19.0-GA</version>
--- End diff --
is this change necessary?
---
[GitHub] flink pull request #5912: [FLINK-9041] Refactor StreamTaskTest to not use sc...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/5912
---
[GitHub] flink pull request #5912: [FLINK-9041] Refactor StreamTaskTest to not use sc...
Posted by azagrebin <gi...@git.apache.org>.
Github user azagrebin commented on a diff in the pull request:
https://github.com/apache/flink/pull/5912#discussion_r184042695
--- Diff: pom.xml ---
@@ -452,7 +452,7 @@ under the License.
<dependency>
<groupId>org.javassist</groupId>
<artifactId>javassist</artifactId>
- <version>3.18.2-GA</version>
+ <version>3.19.0-GA</version>
--- End diff --
I have a problem with 18 version:
<details><summary>Stack trace</summary>
<p>
java.lang.IllegalStateException: Failed to transform class with name org.apache.flink.streaming.runtime.tasks.StreamTaskTest$TestingExecutionStateListener. Reason: javassist.bytecode.InterfaceMethodrefInfo cannot be cast to javassist.bytecode.MethodrefInfo
at org.powermock.core.classloader.MockClassLoader.loadMockClass(MockClassLoader.java:283)
at org.powermock.core.classloader.MockClassLoader.loadModifiedClass(MockClassLoader.java:192)
at org.powermock.core.classloader.DeferSupportingClassLoader.loadClass(DeferSupportingClassLoader.java:71)
at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
at java.lang.Class.forName0(Native Method)
at java.lang.Class.forName(Class.java:264)
at javassist.runtime.Desc.getClassObject(Desc.java:43)
at javassist.runtime.Desc.getClassType(Desc.java:152)
at javassist.runtime.Desc.getType(Desc.java:122)
at javassist.runtime.Desc.getType(Desc.java:78)
at org.apache.flink.streaming.runtime.tasks.StreamTaskTest.testEarlyCanceling(StreamTaskTest.java:185)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at org.junit.internal.runners.TestMethod.invoke(TestMethod.java:68)
at org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl$PowerMockJUnit44MethodRunner.runTestMethod(PowerMockJUnit44RunnerDelegateImpl.java:316)
at org.junit.internal.runners.MethodRoadie$2.run(MethodRoadie.java:89)
at org.junit.internal.runners.MethodRoadie.runBeforesThenTestThenAfters(MethodRoadie.java:97)
at org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl$PowerMockJUnit44MethodRunner.executeTest(PowerMockJUnit44RunnerDelegateImpl.java:300)
at org.powermock.modules.junit4.internal.impl.PowerMockJUnit47RunnerDelegateImpl$PowerMockJUnit47MethodRunner.executeTestInSuper(PowerMockJUnit47RunnerDelegateImpl.java:131)
at org.powermock.modules.junit4.internal.impl.PowerMockJUnit47RunnerDelegateImpl$PowerMockJUnit47MethodRunner.access$100(PowerMockJUnit47RunnerDelegateImpl.java:59)
at org.powermock.modules.junit4.internal.impl.PowerMockJUnit47RunnerDelegateImpl$PowerMockJUnit47MethodRunner$TestExecutorStatement.evaluate(PowerMockJUnit47RunnerDelegateImpl.java:147)
at org.junit.rules.TestWatcher$1.evaluate(TestWatcher.java:55)
at org.powermock.modules.junit4.internal.impl.PowerMockJUnit47RunnerDelegateImpl$PowerMockJUnit47MethodRunner.evaluateStatement(PowerMockJUnit47RunnerDelegateImpl.java:107)
at org.powermock.modules.junit4.internal.impl.PowerMockJUnit47RunnerDelegateImpl$PowerMockJUnit47MethodRunner.executeTest(PowerMockJUnit47RunnerDelegateImpl.java:82)
at org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl$PowerMockJUnit44MethodRunner.runBeforesThenTestThenAfters(PowerMockJUnit44RunnerDelegateImpl.java:288)
at org.junit.internal.runners.MethodRoadie.runTest(MethodRoadie.java:87)
at org.junit.internal.runners.MethodRoadie.run(MethodRoadie.java:50)
at org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl.invokeTestMethod(PowerMockJUnit44RunnerDelegateImpl.java:208)
at org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl.runMethods(PowerMockJUnit44RunnerDelegateImpl.java:147)
at org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl$1.run(PowerMockJUnit44RunnerDelegateImpl.java:121)
at org.junit.internal.runners.ClassRoadie.runUnprotected(ClassRoadie.java:34)
at org.junit.internal.runners.ClassRoadie.runProtected(ClassRoadie.java:44)
at org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl.run(PowerMockJUnit44RunnerDelegateImpl.java:123)
at org.powermock.modules.junit4.common.internal.impl.JUnit4TestSuiteChunkerImpl.run(JUnit4TestSuiteChunkerImpl.java:121)
at org.powermock.modules.junit4.common.internal.impl.AbstractCommonPowerMockRunner.run(AbstractCommonPowerMockRunner.java:53)
at org.powermock.modules.junit4.PowerMockRunner.run(PowerMockRunner.java:59)
at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:68)
at com.intellij.rt.execution.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:47)
at com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:242)
at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:70)
Caused by: java.lang.ClassCastException: javassist.bytecode.InterfaceMethodrefInfo cannot be cast to javassist.bytecode.MethodrefInfo
at javassist.bytecode.ConstPool.getMethodrefClassName(ConstPool.java:404)
at javassist.expr.MethodCall.getClassName(MethodCall.java:94)
at javassist.expr.MethodCall.getCtClass(MethodCall.java:76)
at javassist.expr.MethodCall.getMethod(MethodCall.java:115)
at org.powermock.core.transformers.impl.AbstractMainMockTransformer$PowerMockExpressionEditor.edit(AbstractMainMockTransformer.java:293)
at javassist.expr.ExprEditor.loopBody(ExprEditor.java:192)
at javassist.expr.ExprEditor.doit(ExprEditor.java:91)
at javassist.CtClassType.instrument(CtClassType.java:1398)
at org.powermock.core.transformers.impl.ClassMockTransformer.transformMockClass(ClassMockTransformer.java:65)
at org.powermock.core.transformers.impl.AbstractMainMockTransformer.transform(AbstractMainMockTransformer.java:247)
at org.powermock.core.classloader.MockClassLoader.loadMockClass(MockClassLoader.java:264)
... 42 more
</p>
</details>
I am not sure how my changes caused this problem but it seems to be a [known issue in javassist](https://github.com/powermock/powermock/issues/729).
---
[GitHub] flink pull request #5912: [FLINK-9041] Refactor StreamTaskTest to not use sc...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/5912#discussion_r184034674
--- Diff: flink-streaming-java/src/test/java/org/apache/flink/streaming/runtime/tasks/StreamTaskTest.java ---
@@ -178,7 +172,7 @@
*/
@Test
public void testEarlyCanceling() throws Exception {
- Deadline deadline = new FiniteDuration(2, TimeUnit.MINUTES).fromNow();
--- End diff --
We have our own `Deadline` implementation that is pretty much a drop-in replacement for the scala version.
---