[jira] [Comment Edited] (FLINK-2852) Fix flaky ScalaShellITSuite

["Sachin Goel (JIRA)" <ji...@apache.org>]

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

Sachin Goel edited comment on FLINK-2852 at 10/19/15 1:04 PM:
--------------------------------------------------------------

This is proving to be a major headache, failing almost every other build. Two things:
1. We definitely need a better way to ensure that a job has finished. I propose this:
In {{JobClient}}, there is a {{if-else}} block for
{code}
if (answer instanceof JobManagerMessages.JobResultSuccess) {
    LOG.info("Job execution complete");
    ...
}
{code}
We can simply add this:
{code}
    if (sysoutLogUpdates) {
        System.out.println("Job execution complete.");
    }
{code}
and check for this string in the result.

2. Why do we actually need this check in these two tests? None of the other tests in {{ScalaShellITSuite}} verify this exact line. We can just have the usual tests which are:
{code}
output should not include "Error"
output should not include "ERROR"
output should not include "Exception"
output should not include "failed"
{code}

1 is not a very major issue, and not very important. If this needs to be fixed, it should be in a separate jira. To fix this flaky test, we can just remove the check, as if the execution ever failed, one of the four checks will fail.
Also, 
{code}
val input: String = "val els = env.fromElements(\"a\",\"b\");\nels.print\nError\n:q\n"
{code}
is somewhat poorly written [in both Scala shell integration tests]. I fail to understand why the {{Error}} is left in there. Perhaps it was for debugging purposes, but it should've been removed before committing. We should fix this also under this jira.

[~till.rohrmann], if you're busy with something else, I can push a quick fix for the 2nd point now. We can decide on 1st later.


was (Author: sachingoel0101):
This is proving to be a major headache, failing almost every other build. Two things:
1. We definitely need a better way to ensure that a job has finished. I propose this:
In {{JobClient}}, there is a {{if-else}] block for
{code}
if (answer instanceof JobManagerMessages.JobResultSuccess) {
    LOG.info("Job execution complete");
    ...
}
{code}
We can simply add this:
{code}
    if (sysoutLogUpdates) {
        System.out.println("Job execution complete.");
    }
{code}
and check for this string in the result.

2. Why do we actually need this check in these two tests? None of the other tests in {{ScalaShellITSuite}} verify this exact line. We can just have the usual tests which are:
{code}
output should not include "Error"
output should not include "ERROR"
output should not include "Exception"
output should not include "failed"
{code}

1 is not a very major issue, and not very important. If this needs to be fixed, it should be in a separate jira. To fix this flaky test, we can just remove the check, as if the execution ever failed, one of the four checks will fail.
Also, 
{code}
val input: String = "val els = env.fromElements(\"a\",\"b\");\nels.print\nError\n:q\n"
{code}
is somewhat poorly written [in both Scala shell integration tests]. I fail to understand why the {{Error}} is left in there. Perhaps it was for debugging purposes, but it should've been removed before committing. We should fix this also under this jira.

[~till.rohrmann], if you're busy with something else, I can push a quick fix for the 2nd point now. We can decide on 1st later.

> Fix flaky ScalaShellITSuite
> ---------------------------
>
>                 Key: FLINK-2852
>                 URL: https://issues.apache.org/jira/browse/FLINK-2852
>             Project: Flink
>          Issue Type: Bug
>          Components: Scala Shell
>    Affects Versions: 0.10
>            Reporter: Till Rohrmann
>            Assignee: Till Rohrmann
>            Priority: Critical
>              Labels: test-stability
>             Fix For: 0.10
>
>
> The {{ScalaShellITSuite}} checks the log output whether a job has successful completed or not. For that to happen it checks for a {{Job execution switched to status FINISHED}} string in the log output. However, if the {{FlinkClientActor}} receives first a {{JobResultSuccess}} message before receiving the {{JobStatusChanged}} message, then it will send the execution result back to the {{Client}} and terminate itself. This has the consequence that the output will never contain the above-mentioned string.
> I propose to use a different mean to check whether a job has finished successfully or not.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)