[jira] [Commented] (FLINK-4569) JobRetrievalITCase.testJobRetrieval() does not forward exceptions to parent thread.

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-4569?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=1647#comment-1647
 ] 

ASF GitHub Bot commented on FLINK-4569:
---

Github user asfgit closed the pull request at:

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


> JobRetrievalITCase.testJobRetrieval() does not forward exceptions to parent 
> thread.
> ---
>
> Key: FLINK-4569
> URL: https://issues.apache.org/jira/browse/FLINK-4569
> Project: Flink
>  Issue Type: Bug
>  Components: Client
>Reporter: Robert Metzger
>Assignee: Chesnay Schepler
>Priority: Blocker
>  Labels: test-stability
> Fix For: 1.5.0
>
>
> The mentioned test seems to fail frequently, without being detected, because 
> the Assert.fail() is called in a separate thread which doesn't forward 
> exceptions.
> https://s3.amazonaws.com/archive.travis-ci.org/jobs/156177995/log.txt



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (FLINK-4569) JobRetrievalITCase.testJobRetrieval() does not forward exceptions to parent thread.

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-4569?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16399199#comment-16399199
 ] 

ASF GitHub Bot commented on FLINK-4569:
---

Github user zentol commented on the issue:

https://github.com/apache/flink/pull/5689
  
merging,


> JobRetrievalITCase.testJobRetrieval() does not forward exceptions to parent 
> thread.
> ---
>
> Key: FLINK-4569
> URL: https://issues.apache.org/jira/browse/FLINK-4569
> Project: Flink
>  Issue Type: Bug
>  Components: Client
>Reporter: Robert Metzger
>Assignee: Chesnay Schepler
>Priority: Blocker
>  Labels: test-stability
> Fix For: 1.5.0
>
>
> The mentioned test seems to fail frequently, without being detected, because 
> the Assert.fail() is called in a separate thread which doesn't forward 
> exceptions.
> https://s3.amazonaws.com/archive.travis-ci.org/jobs/156177995/log.txt



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (FLINK-4569) JobRetrievalITCase.testJobRetrieval() does not forward exceptions to parent thread.

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-4569?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16396968#comment-16396968
 ] 

ASF GitHub Bot commented on FLINK-4569:
---

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

https://github.com/apache/flink/pull/5689#discussion_r174140412
  
--- Diff: 
flink-tests/src/test/java/org/apache/flink/test/example/client/JobRetrievalITCase.java
 ---
@@ -119,6 +121,11 @@ public void run() {
lock.release();
 
resumingThread.join();
+
+   Throwable exception = error.get();
+   if (exception != null) {
+   throw new AssertionError(exception);
+   }
--- End diff --

assertNull throws a NPE. This throws the actual exception with stacktrace 
etc and is the most equivalent to just throwing the exception.


> JobRetrievalITCase.testJobRetrieval() does not forward exceptions to parent 
> thread.
> ---
>
> Key: FLINK-4569
> URL: https://issues.apache.org/jira/browse/FLINK-4569
> Project: Flink
>  Issue Type: Bug
>  Components: Client
>Reporter: Robert Metzger
>Assignee: Chesnay Schepler
>Priority: Blocker
>  Labels: test-stability
> Fix For: 1.5.0
>
>
> The mentioned test seems to fail frequently, without being detected, because 
> the Assert.fail() is called in a separate thread which doesn't forward 
> exceptions.
> https://s3.amazonaws.com/archive.travis-ci.org/jobs/156177995/log.txt



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (FLINK-4569) JobRetrievalITCase.testJobRetrieval() does not forward exceptions to parent thread.

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-4569?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16396953#comment-16396953
 ] 

ASF GitHub Bot commented on FLINK-4569:
---

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

https://github.com/apache/flink/pull/5689#discussion_r174135885
  
--- Diff: 
flink-tests/src/test/java/org/apache/flink/test/example/client/JobRetrievalITCase.java
 ---
@@ -119,6 +121,11 @@ public void run() {
lock.release();
 
resumingThread.join();
+
+   Throwable exception = error.get();
+   if (exception != null) {
+   throw new AssertionError(exception);
+   }
--- End diff --

Is this any different from using `Assert.assertNull(error.get())`? Not that 
it makes any difference but just to know why you picked throwing the exception 
here. Personally I think that the assertNull seems more explicit when you read 
the code and it is less verbose.


> JobRetrievalITCase.testJobRetrieval() does not forward exceptions to parent 
> thread.
> ---
>
> Key: FLINK-4569
> URL: https://issues.apache.org/jira/browse/FLINK-4569
> Project: Flink
>  Issue Type: Bug
>  Components: Client
>Reporter: Robert Metzger
>Assignee: Chesnay Schepler
>Priority: Blocker
>  Labels: test-stability
> Fix For: 1.5.0
>
>
> The mentioned test seems to fail frequently, without being detected, because 
> the Assert.fail() is called in a separate thread which doesn't forward 
> exceptions.
> https://s3.amazonaws.com/archive.travis-ci.org/jobs/156177995/log.txt



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (FLINK-4569) JobRetrievalITCase.testJobRetrieval() does not forward exceptions to parent thread.

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-4569?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16396865#comment-16396865
 ] 

ASF GitHub Bot commented on FLINK-4569:
---

GitHub user zentol opened a pull request:

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

[FLINK-4569][tests] Respect exceptions thrown in thread in 
JobRetrievalITCase

## What is the purpose of the change

This PR ensures that exceptions that occur in the submitting thread 
actually fail the test. Also contains minor fixes, like setting a thread name 
(for debugging purposes) and releasing the lock in the `SemaphoreInvokable` to 
allow multiple runs in the IDE.

## Verifying the change

One can modify the test to throw an exception after connecting 
successfully, which will fail the test.



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

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

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

https://github.com/apache/flink/pull/5689.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 #5689


commit 13c383d439550b1ebdc869981a7ffa19a6c8a151
Author: zentol 
Date:   2018-03-13T12:00:47Z

[FLINK-4569][tests] Respect exceptions thrown in thread in 
JobRetrievalITCase




> JobRetrievalITCase.testJobRetrieval() does not forward exceptions to parent 
> thread.
> ---
>
> Key: FLINK-4569
> URL: https://issues.apache.org/jira/browse/FLINK-4569
> Project: Flink
>  Issue Type: Bug
>  Components: Client
>Reporter: Robert Metzger
>Assignee: Chesnay Schepler
>Priority: Blocker
>  Labels: test-stability
> Fix For: 1.5.0
>
>
> The mentioned test seems to fail frequently, without being detected, because 
> the Assert.fail() is called in a separate thread which doesn't forward 
> exceptions.
> https://s3.amazonaws.com/archive.travis-ci.org/jobs/156177995/log.txt



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (FLINK-4569) JobRetrievalITCase.testJobRetrieval() does not forward exceptions to parent thread.

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-4569?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16395253#comment-16395253
 ] 

ASF GitHub Bot commented on FLINK-4569:
---

Github user zentol closed the pull request at:

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


> JobRetrievalITCase.testJobRetrieval() does not forward exceptions to parent 
> thread.
> ---
>
> Key: FLINK-4569
> URL: https://issues.apache.org/jira/browse/FLINK-4569
> Project: Flink
>  Issue Type: Bug
>  Components: Client
>Reporter: Robert Metzger
>Assignee: Chesnay Schepler
>Priority: Blocker
>  Labels: test-stability
> Fix For: 1.5.0
>
>
> The mentioned test seems to fail frequently, without being detected, because 
> the Assert.fail() is called in a separate thread which doesn't forward 
> exceptions.
> https://s3.amazonaws.com/archive.travis-ci.org/jobs/156177995/log.txt



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (FLINK-4569) JobRetrievalITCase.testJobRetrieval() does not forward exceptions to parent thread.

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-4569?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16395079#comment-16395079
 ] 

ASF GitHub Bot commented on FLINK-4569:
---

GitHub user zentol opened a pull request:

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

[FLINK-4569][tests] Fix JobRetrievalITCase

## What is the purpose of the change

This PR contains 2 changes to the `JobRetrievalITCase`.

1) We now store exceptions that occur in the submitting thread in a 
`AtomicReference` that we evaluate at the end of the test.

2) It could happen that the lock (preventing the job from terminating) was 
released before the client had a chance to finish the retrieval. To remedy this 
we now only release the lock if the thread has terminated, either by 
successfully connecting or failing with an error.



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

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

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

https://github.com/apache/flink/pull/5681.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 #5681


commit d45d8883365ef73c7064adfd098feea208d2ca4c
Author: zentol 
Date:   2018-03-12T11:01:57Z

[FLINK-4569][tests] Fix JobRetrievalITCase




> JobRetrievalITCase.testJobRetrieval() does not forward exceptions to parent 
> thread.
> ---
>
> Key: FLINK-4569
> URL: https://issues.apache.org/jira/browse/FLINK-4569
> Project: Flink
>  Issue Type: Bug
>  Components: Client
>Reporter: Robert Metzger
>Assignee: Chesnay Schepler
>Priority: Blocker
>  Labels: test-stability
> Fix For: 1.5.0
>
>
> The mentioned test seems to fail frequently, without being detected, because 
> the Assert.fail() is called in a separate thread which doesn't forward 
> exceptions.
> https://s3.amazonaws.com/archive.travis-ci.org/jobs/156177995/log.txt



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)