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

[JUnit] Introduce the property of trimStackTrace to show error stacktrace in mvn-test #126

Merged
merged 3 commits into from
Aug 5, 2022

Conversation

zuston
Copy link
Member

@zuston zuston commented Aug 4, 2022

What changes were proposed in this pull request?

[JUnit] Introduce the property of trimStackTrace to show error stacktrace in mvn-test

Why are the changes needed?

When acting mvn test in current codebase, it will not throw detailed stacktrace about failed UTs and make hard to debug. So introduce this property of trimStackTrace to allow user to show the complete stacktrace.

Does this PR introduce any user-facing change?

NO

How was this patch tested?

CI Before this PR

  1. https://github.com/zuston/incubator-uniffle/actions/runs/2800975455#artifacts
    image

After

  1. https://github.com/apache/incubator-uniffle/runs/7684126515?check_suite_focus=true
    image

@jerqi
Copy link
Contributor

jerqi commented Aug 4, 2022

Could you show me the effect of this plugin by you own Github action? like #122

@jerqi
Copy link
Contributor

jerqi commented Aug 4, 2022

If you want to see the log, you can download https://github.com/apache/incubator-uniffle/actions/runs/2779258464
企业微信截图_ddffa6a0-c008-4150-bfa7-3fa21a08d420
Click the link which I give you, and Search Artifacts

@codecov-commenter
Copy link

codecov-commenter commented Aug 4, 2022

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 57.17%. Comparing base (fd8ccdd) to head (dc58fc1).
Report is 914 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##             master     #126   +/-   ##
=========================================
  Coverage     57.17%   57.17%           
  Complexity     1201     1201           
=========================================
  Files           150      150           
  Lines          8178     8178           
  Branches        773      773           
=========================================
  Hits           4676     4676           
  Misses         3256     3256           
  Partials        246      246           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@zuston
Copy link
Member Author

zuston commented Aug 4, 2022

The failed test cases stracktrace will be shown in artifacts? I didn’t find.

And i think it’s meaningful to allow user to specify the config to control the stacktrace output.

@jerqi
Copy link
Contributor

jerqi commented Aug 4, 2022

The failed test cases stracktrace will be shown in artifacts? I didn’t find.

Yes, Click the link which I give you, and Search Artifacts, you can find it.

@zuston
Copy link
Member Author

zuston commented Aug 5, 2022

Recheck the log in artifact, still not found the junit test case error detailed stacktrace.

Refer: https://stackoverflow.com/questions/2928548/make-mavens-surefire-show-stacktrace-in-console

@jerqi
Copy link
Contributor

jerqi commented Aug 5, 2022

Could you show me the effect of this plugin by you own Github action? like #122

Could you give me an example?

@zuston
Copy link
Member Author

zuston commented Aug 5, 2022

The default value of trimStackTrace is true, which will not output the error stackstrace. But when we execute the command of mvn clean install -Pspark3 -DskipTests -DtrimStackTrace=true, it will be valid.

Could you give me an example?

Ok, it will change the default value to false.

@zuston
Copy link
Member Author

zuston commented Aug 5, 2022

@jerqi
Copy link
Contributor

jerqi commented Aug 5, 2022

The artifact log which I give you have these logs, you can find it in *.out.

@zuston
Copy link
Member Author

zuston commented Aug 5, 2022

Due to this PR, so the detailed log can be shown in the console and output log.

@jerqi
Copy link
Contributor

jerqi commented Aug 5, 2022

Due to this PR, so the detailed log can be shown in the console and output log.

I don't use your pr.

@zuston
Copy link
Member Author

zuston commented Aug 5, 2022

Updated @jerqi

@@ -144,6 +144,7 @@
<useFile>${test.redirectToFile}</useFile>
<argLine>-ea -Xmx3g</argLine>
<failIfNoTests>false</failIfNoTests>
<trimStackTrace>${trimStackTrace}</trimStackTrace>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need this? Is it enough to change parent pom file? And why do we only modify common, mr, spark-common? How about spark2 and spark3?

Copy link
Member Author

Choose a reason for hiding this comment

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

Tested. The trimStackTrace in parent pom is enough.

@zuston zuston requested a review from jerqi August 5, 2022 05:19
Copy link
Contributor

@jerqi jerqi left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @zuston

@jerqi jerqi merged commit 3f8826a into apache:master Aug 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants