-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
Trim stack trace. #1028
Trim stack trace. #1028
Conversation
81fe75d
to
11b0e34
Compare
*/ | ||
public String getFilteredTrace() { | ||
Throwable exception = getException(); | ||
String fullTrace = getTrace(exception); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The above two lines can be replaced with a call to getTrace()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
|
@kcooney I like the general idea! |
44e12ce
to
4f7c311
Compare
I finally found time to work on this. I added lots of tests. Please take another look. @marcphilipp I'm not sure what you meant by "BaseTestRunner.getFilteredTrace() would have to be removed then, right?" |
ea663e2
to
7b174b5
Compare
7b174b5
to
4930a1e
Compare
@kcooney I don't recall what I meant by that either, sorry. Never mind, then. LGTM so far, needs a bit of refactoring IMHO since it contains a few long, complex methods. |
@marcphilipp the only really long method is |
static String trimStackTrace(String extracedExceptionMessage, String fullTrace) { | ||
StringBuilder trimmedTrace = new StringBuilder(extracedExceptionMessage); | ||
BufferedReader reader = new BufferedReader( | ||
new StringReader(fullTrace.substring(extracedExceptionMessage.length()))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extraced -> extracted - typo
I extracted a method, and added more test cases. PTAL If it looks good, I want to squash commits before merging this pull. |
BTW, this pull won't affect the functionality of Maven (I checked), and won't likely affect Gradle. It only affects users of |
I've made some more changes
PTAL In parallel, I'm experimenting with changing the actual stack trace of the thrown exception. If that works, that solution would cause the stack traces to be trimmed in Maven and Gradle |
Before trimming the stacktrace for Maven/Gradle, I think we should ask someone from each project if that's something they'd appreciate, shouldn't we? |
@marcphilipp I would be more interested in the feedback of Maven or Gradle users that also use JUnit. I don't think we need to reach out to the Maven or Gradle teams unless we think we would break them. If we change the actual stack trace itself, I would be worried about breaking IDEs that already do this trimming. The change as-is won't help many users |
b6f240b
to
3c6be5a
Compare
I was reviewing old pool requests, and took another look at this. Maven Surefire already has an option (enabled, by default) to trim JUnit stack traces. I think I didn't realize this before since often the stack traces I see from running JUnit tests are from JUnit tests that run other JUnit tests. In any case, I'd like to merge this. I've merged from master, and the tests pass locally. I also moved @marcphilipp any objections to merging this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Update TextListener to trim stack traces. Fixes junit-team#669
… classic/non-orchestrator modes. This change should clean up test failure reporting by: - Remove test runner framework related stack frames - Truncate stack traces to a 64KB size when running under orchestrator to attempt to avoid binder transaction limits. This limit is already enforced when running in classic/non-orchestrator mode JUnit 4.13 has a really nice getTrimmedStackTrace feature, but androidx.test is fixed to 4.12 for the time being. So as a temporary workaround, copy the relevant JUnit change junit-team/junit4#1028 into this project. Fixes #729, and hopefully #269 PiperOrigin-RevId: 329797783
… classic/non-orchestrator modes. This change should clean up test failure reporting by: - Remove test runner framework related stack frames - Truncate stack traces to a 64KB size when running under orchestrator to attempt to avoid binder transaction limits. This limit is already enforced when running in classic/non-orchestrator mode JUnit 4.13 has a really nice getTrimmedStackTrace feature, but androidx.test is fixed to 4.12 for the time being. So as a temporary workaround, copy the relevant JUnit change junit-team/junit4#1028 into this project. Fixes #729, and hopefully #269 PiperOrigin-RevId: 329797783
… classic/non-orchestrator modes. This change should clean up test failure reporting by: - Remove test runner framework related stack frames - Truncate stack traces to a 64KB size when running under orchestrator to attempt to avoid binder transaction limits. This limit is already enforced when running in classic/non-orchestrator mode JUnit 4.13 has a really nice getTrimmedStackTrace feature, but androidx.test is fixed to 4.12 for the time being. So as a temporary workaround, copy the relevant JUnit change junit-team/junit4#1028 into this project. Fixes #729, and hopefully #269 PiperOrigin-RevId: 329797783
… classic/non-orchestrator modes. This change should clean up test failure reporting by: - Remove test runner framework related stack frames - Truncate stack traces to a 64KB size when running under orchestrator to attempt to avoid binder transaction limits. This limit is already enforced when running in classic/non-orchestrator mode JUnit 4.13 has a really nice getTrimmedStackTrace feature, but androidx.test is fixed to 4.12 for the time being. So as a temporary workaround, copy the relevant JUnit change junit-team/junit4#1028 into this project. Fixes #729, and hopefully #269 PiperOrigin-RevId: 332051125
… classic/non-orchestrator modes. This change should clean up test failure reporting by: - Remove test runner framework related stack frames - Truncate stack traces to a 64KB size when running under orchestrator to attempt to avoid binder transaction limits. This limit is already enforced when running in classic/non-orchestrator mode JUnit 4.13 has a really nice getTrimmedStackTrace feature, but androidx.test is fixed to 4.12 for the time being. So as a temporary workaround, copy the relevant JUnit change junit-team/junit4#1028 into this project. Fixes #729, and hopefully #269 PiperOrigin-RevId: 332051125
Update TextListener to trim stack traces. Fixes junit-team#669
This is just some ideas I had for fixing #669. I haven't written tests yet; just wanted early feedback.