Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-32253][INFRA] Show errors only for the sbt tests of github actions #29133

Closed
wants to merge 5 commits into from

Conversation

gengliangwang
Copy link
Member

@gengliangwang gengliangwang commented Jul 16, 2020

What changes were proposed in this pull request?

Make the test result log of github action more readable by showing errors from SBT only.

  1. Add "--error" flag to sbt in github action to set the log level as "ERROR"
  2. Show only failed test cases in stderr output of github action. According to https://www.scalatest.org/user_guide/using_the_runner, with SBT option -eNCXEHLOPQMDF we can drop all the following events:
N - drop TestStarting events
C - drop TestSucceeded events
X - drop TestIgnored events
E - drop TestPending events
H - drop SuiteStarting events
L - drop SuiteCompleted events
O - drop InfoProvided events
P - drop ScopeOpened events
Q - drop ScopeClosed events
R - drop ScopePending events
M - drop MarkupProvided events

and enable the following two mode:

D - show all durations
F - show full stack traces

Why are the changes needed?

Currently, the output of github action is very long and we have to scroll down to find the failed test cases. Even more, the log may be truncated. In such a case, we will have to wait until all the jobs are completed and then download all the raw logs.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Before changes, all the warnings in compiling are shown:
image

as well as all the passed and ignored test cases:
image

After changes, sbt test only shows the summary for a successful job:
image

image

If there is a test failure, a full stack track is shown as well as a test failure summary at the end of test log:

image

image

@SparkQA
Copy link

SparkQA commented Jul 16, 2020

Test build #125982 has finished for PR 29133 at commit 3bcfbb1.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jul 16, 2020

Test build #125983 has finished for PR 29133 at commit 60db1af.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@gengliangwang gengliangwang changed the title [SPARK-32253][INFRA] Make readability better in the test result logs [WIP][SPARK-32253][INFRA] Make readability better in the test result logs Jul 16, 2020
// Show only the failed test cases in github action to make the log more readable.
testOptions in Test += Tests.Argument(TestFrameworks.ScalaTest,
sys.env.get("GITHUB_ACTIONS").map { _ =>
Seq("-eNCXEHLOPQMDSF")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that you explicitly enabled all available standard error options except W and U. If then, could you describe the reason why you choose some and exclude those two, please?

W - without color
U - unformatted mode

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, could you add the link into the comment at below line 1030 because this is non-trivial?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we drop all the following events:

N - drop TestStarting events
C - drop TestSucceeded events
X - drop TestIgnored events
E - drop TestPending events
H - drop SuiteStarting events
L - drop SuiteCompleted events
O - drop InfoProvided events
P - drop ScopeOpened events
Q - drop ScopeClosed events
R - drop ScopePending events
M - drop MarkupProvided events

and enable the following two mode:

D - show all durations
F - show full stack traces

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The following two modes will make the readability worse

W - without color
U - unformatted mode

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you put that explanation into the PR description, please? PR description will become a permanent commit log . So, it will help other people to understand your decision. Thanks, @gengliangwang .

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dongjoon-hyun Sure, I have updated the pr description.

@gengliangwang gengliangwang changed the title [WIP][SPARK-32253][INFRA] Make readability better in the test result logs [WIP][SPARK-32253][INFRA] Show errors only for the sbt tests of github actions Jul 16, 2020
@SparkQA
Copy link

SparkQA commented Jul 16, 2020

Test build #125989 has finished for PR 29133 at commit 663e5a7.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jul 16, 2020

Test build #125991 has finished for PR 29133 at commit 3590262.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@HyukjinKwon
Copy link
Member

@gengliangwang, it would be easier to review if we have before/after examples of the logs :-) cc @viirya as well

@gengliangwang gengliangwang changed the title [WIP][SPARK-32253][INFRA] Show errors only for the sbt tests of github actions [SPARK-32253][INFRA] Show errors only for the sbt tests of github actions Jul 17, 2020
@gengliangwang
Copy link
Member Author

gengliangwang commented Jul 17, 2020

@HyukjinKwon I have updated the PR description.
Meanwhile, I created a PR on my repo to see what the test failure log will look like: gengliangwang#6

Here is an example of failed log output: https://github.com/gengliangwang/spark/pull/6/checks?check_run_id=880362871

@SparkQA
Copy link

SparkQA commented Jul 17, 2020

Test build #126044 has finished for PR 29133 at commit 2d39b8e.

  • This patch fails PySpark pip packaging tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@dongjoon-hyun
Copy link
Member

"This patch fails PySpark pip packaging tests." flakiness happens frequently in these day although I don't know the root cause.

@viirya
Copy link
Member

viirya commented Jul 18, 2020

I saw the screenshot of after this change. Can we also add one screenshot of before this change too?

@gengliangwang
Copy link
Member Author

@viirya I have updated the PR description.

@HyukjinKwon
Copy link
Member

@dongjoon-hyun, I am debugging the flakiness at #29117 to make a complete fix. I think it was caused by my recent fixes and Jenkins environment differences vs Github Actions. I will be able to fix at least in the next week.

@HyukjinKwon
Copy link
Member

I am okay to try this out for now until we have a way to report the test results but I'll leave it to @dongjoon-hyun and @viirya.

@viirya
Copy link
Member

viirya commented Jul 19, 2020

Looks okay to me. We can keep improving this.

Copy link
Member

@HyukjinKwon HyukjinKwon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay.. LGTM

@HyukjinKwon
Copy link
Member

I filed new JIRA for test reporter (SPARK-32357).. let's see how it goes.

Merged to master.

@HyukjinKwon
Copy link
Member

@gengliangwang, seems this hides the test results from JUnit tests. Can you take a look and see if it shows JUnit test failures as well? If you're busy for something else, we can revert it for now as well.

@gengliangwang
Copy link
Member Author

@HyukjinKwon thanks for the reminder! I will see if there is any quick fix. If not, I will revert this very soon.

@HyukjinKwon
Copy link
Member

Thank you @gengliangwang for investigating this.

gengliangwang added a commit that referenced this pull request Jul 24, 2020
…thub actions"

### What changes were proposed in this pull request?

This reverts commit 026b0b9.

### Why are the changes needed?

As HyukjinKwon pointed out in #29133 (comment), there is no JUnit test report after #29133. Let's revert #29133 for now and find a better solution to improve the log output later.

### Does this PR introduce _any_ user-facing change?

No, dev-only.

### How was this patch tested?

GitHub Actions build

Closes #29219 from gengliangwang/revertErrorOnly.

Authored-by: Gengliang Wang <gengliang.wang@databricks.com>
Signed-off-by: Gengliang Wang <gengliang.wang@databricks.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants