[SUREFIRE-1992] Do not output class name on error/failure for certain exceptions #457
Conversation
| result.append( target.getClass().getSimpleName() ); | ||
| result.append( getTruncatedMessage( throwable.getMessage(), MAX_LINE_LENGTH - result.length() ) ); | ||
| result.append( ' ' ) | ||
| .append( msg.split( "\r?\n" )[0] ); |
There was a problem hiding this comment.
I know that the .split( "\r?\n" )[0] was my request but this part of code will break the nackwards compatibility. Pls use only .append( msg );, thx
And I have an additional question. Is the code between the lines 90 and 103 the same in the class SmartStackTraceParser - see the lines 133 - 146? Is it possible to share this code between classes?
Has the method toMinimalThrowableMiniMessage() the same code in both classes?
There was a problem hiding this comment.
Hi @Tibor17
I know that the
.split( "\r?\n" )[0]was my request but this part of code will break the nackwards compatibility.
In what way would this break backward compatibility?
Surefire has always kept each failure on one line on the output. This behaviour is still true.
If your concern is that important info is contained on lines 2..n of the error/failure message, we could replace line separators by space and keep the complete message.
| .append( msg.split( "\r?\n" )[0] ); | |
| .append( msg.replaceAll( "\r?\n", " " ) ); |
There was a problem hiding this comment.
The mesage was always printed completely.
If you compare witht he old code, you will see only msg.
The point of the issue is to remove the function which takes first 77 chars, right?
There was a problem hiding this comment.
The point of the issue is to remove the function which takes first 77 chars, right?
Correct.
Or to put in more general terms: output enough of the error message that it is always clear and eases troubleshooting.
Your suggestion to print only the first line is a good compromise I think.
To have multi-line messages in exception is unusual (if not an anti-pattern). I cannot think of many examples other than jdbc drivers maybe that tend to be quite verbose in the exception message.
There was a problem hiding this comment.
The Spark/Groovy is having such assertion statements which have excellent benefit with multiline message for user.
I would not like to break the users since they rely on complete message.
Thx for your work and all your efforts!
There was a problem hiding this comment.
Fine with me. I'll close this pull request then?
| if ( isNotEmpty( msg ) ) | ||
| { | ||
| result.append( ' ' ) | ||
| .append( msg.split( "\r?\n" )[0] ); |
Pull request 3/3 for this ticket. Please merge in order.