You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/02/04 11:38:12 UTC
[GitHub] [maven-surefire] sman-81 opened a new pull request #457: [SUREFIRE-1992] Do not output class name on error/failure for certain exceptions
sman-81 opened a new pull request #457:
URL: https://github.com/apache/maven-surefire/pull/457
Pull request 3/3 for this ticket. Please merge in order.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #457: [SUREFIRE-1992] Do not output class name on error/failure for certain exceptions
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #457:
URL: https://github.com/apache/maven-surefire/pull/457#discussion_r804511824
##########
File path: surefire-api/src/main/java/org/apache/maven/surefire/api/report/LegacyPojoStackTraceWriter.java
##########
@@ -81,19 +80,26 @@ public String smartTrimmedStackTrace()
result.append( "#" );
result.append( testMethod );
SafeThrowable throwable = getThrowable();
- if ( throwable.getTarget() instanceof AssertionError )
- {
- result.append( " " );
- result.append( getTruncatedMessage( throwable.getMessage(), MAX_LINE_LENGTH - result.length() ) );
- }
- else
+ Throwable target = throwable.getTarget();
+
+ if ( target != null )
{
- Throwable target = throwable.getTarget();
- if ( target != null )
+
+ final String excClassName = target.getClass().getName();
+
+ if ( ! ( AssertionError.class.isInstance( target )
+ || excClassName.endsWith( ".AssertionFailedError" )
+ || excClassName.endsWith( ".ComparisonFailure" )
+ || excClassName.startsWith( "org.opentest4j." ) ) )
+ {
+ result.append( ' ' )
+ .append( target.getClass().getSimpleName() );
+ }
+ final String msg = throwable.getMessage();
+ if ( isNotEmpty( msg ) )
{
- result.append( " " );
- result.append( target.getClass().getSimpleName() );
- result.append( getTruncatedMessage( throwable.getMessage(), MAX_LINE_LENGTH - result.length() ) );
+ result.append( ' ' )
+ .append( msg.split( "\r?\n" )[0] );
Review 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!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] sman-81 commented on a change in pull request #457: [SUREFIRE-1992] Do not output class name on error/failure for certain exceptions
Posted by GitBox <gi...@apache.org>.
sman-81 commented on a change in pull request #457:
URL: https://github.com/apache/maven-surefire/pull/457#discussion_r800143609
##########
File path: surefire-api/src/main/java/org/apache/maven/surefire/api/report/LegacyPojoStackTraceWriter.java
##########
@@ -81,19 +80,26 @@ public String smartTrimmedStackTrace()
result.append( "#" );
result.append( testMethod );
SafeThrowable throwable = getThrowable();
- if ( throwable.getTarget() instanceof AssertionError )
- {
- result.append( " " );
- result.append( getTruncatedMessage( throwable.getMessage(), MAX_LINE_LENGTH - result.length() ) );
- }
- else
+ Throwable target = throwable.getTarget();
+
+ if ( target != null )
{
- Throwable target = throwable.getTarget();
- if ( target != null )
+
+ final String excClassName = target.getClass().getName();
+
+ if ( ! ( AssertionError.class.isInstance( target )
+ || excClassName.endsWith( ".AssertionFailedError" )
+ || excClassName.endsWith( ".ComparisonFailure" )
+ || excClassName.startsWith( "org.opentest4j." ) ) )
+ {
+ result.append( ' ' )
+ .append( target.getClass().getSimpleName() );
+ }
+ final String msg = throwable.getMessage();
+ if ( isNotEmpty( msg ) )
{
- result.append( " " );
- result.append( target.getClass().getSimpleName() );
- result.append( getTruncatedMessage( throwable.getMessage(), MAX_LINE_LENGTH - result.length() ) );
+ result.append( ' ' )
+ .append( msg.split( "\r?\n" )[0] );
Review 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.
```suggestion
.append( msg.replaceAll( "\r?\n", " " ) );
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #457: [SUREFIRE-1992] Do not output class name on error/failure for certain exceptions
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #457:
URL: https://github.com/apache/maven-surefire/pull/457#discussion_r799571827
##########
File path: surefire-api/src/main/java/org/apache/maven/surefire/api/report/LegacyPojoStackTraceWriter.java
##########
@@ -81,19 +80,26 @@ public String smartTrimmedStackTrace()
result.append( "#" );
result.append( testMethod );
SafeThrowable throwable = getThrowable();
- if ( throwable.getTarget() instanceof AssertionError )
- {
- result.append( " " );
- result.append( getTruncatedMessage( throwable.getMessage(), MAX_LINE_LENGTH - result.length() ) );
- }
- else
+ Throwable target = throwable.getTarget();
+
+ if ( target != null )
{
- Throwable target = throwable.getTarget();
- if ( target != null )
+
+ final String excClassName = target.getClass().getName();
+
+ if ( ! ( AssertionError.class.isInstance( target )
+ || excClassName.endsWith( ".AssertionFailedError" )
+ || excClassName.endsWith( ".ComparisonFailure" )
+ || excClassName.startsWith( "org.opentest4j." ) ) )
+ {
+ result.append( ' ' )
+ .append( target.getClass().getSimpleName() );
+ }
+ final String msg = throwable.getMessage();
+ if ( isNotEmpty( msg ) )
{
- result.append( " " );
- result.append( target.getClass().getSimpleName() );
- result.append( getTruncatedMessage( throwable.getMessage(), MAX_LINE_LENGTH - result.length() ) );
+ result.append( ' ' )
+ .append( msg.split( "\r?\n" )[0] );
Review 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?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #457: [SUREFIRE-1992] Do not output class name on error/failure for certain exceptions
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #457:
URL: https://github.com/apache/maven-surefire/pull/457#discussion_r799575211
##########
File path: surefire-providers/common-java5/src/main/java/org/apache/maven/surefire/report/SmartStackTraceParser.java
##########
@@ -133,23 +130,19 @@ public String getString()
final String excClassName = excType.getName();
final String msg = throwable.getMessage();
- if ( target instanceof AssertionError
- || "junit.framework.AssertionFailedError".equals( excClassName )
- || "junit.framework.ComparisonFailure".equals( excClassName )
- || excClassName.startsWith( "org.opentest4j." ) )
- {
- if ( isNotEmpty( msg ) )
- {
- result.append( ' ' )
- .append( msg );
- }
- }
- else
+ if ( ! ( AssertionError.class.isInstance( target )
+ || excClassName.endsWith( ".AssertionFailedError" )
+ || excClassName.endsWith( ".ComparisonFailure" )
+ || excClassName.startsWith( "org.opentest4j." ) ) )
{
result.append( rootIsInclass() ? " " : " ยป " )
- .append( toMinimalThrowableMiniMessage( excType ) );
+ .append( toMinimalThrowableMiniMessage( excType ) );
+ }
- result.append( truncateMessage( msg, MAX_LINE_LENGTH - result.length() ) );
+ if ( isNotEmpty( msg ) )
+ {
+ result.append( ' ' )
+ .append( msg.split( "\r?\n" )[0] );
Review comment:
The same here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] sman-81 commented on a change in pull request #457: [SUREFIRE-1992] Do not output class name on error/failure for certain exceptions
Posted by GitBox <gi...@apache.org>.
sman-81 commented on a change in pull request #457:
URL: https://github.com/apache/maven-surefire/pull/457#discussion_r804537241
##########
File path: surefire-api/src/main/java/org/apache/maven/surefire/api/report/LegacyPojoStackTraceWriter.java
##########
@@ -81,19 +80,26 @@ public String smartTrimmedStackTrace()
result.append( "#" );
result.append( testMethod );
SafeThrowable throwable = getThrowable();
- if ( throwable.getTarget() instanceof AssertionError )
- {
- result.append( " " );
- result.append( getTruncatedMessage( throwable.getMessage(), MAX_LINE_LENGTH - result.length() ) );
- }
- else
+ Throwable target = throwable.getTarget();
+
+ if ( target != null )
{
- Throwable target = throwable.getTarget();
- if ( target != null )
+
+ final String excClassName = target.getClass().getName();
+
+ if ( ! ( AssertionError.class.isInstance( target )
+ || excClassName.endsWith( ".AssertionFailedError" )
+ || excClassName.endsWith( ".ComparisonFailure" )
+ || excClassName.startsWith( "org.opentest4j." ) ) )
+ {
+ result.append( ' ' )
+ .append( target.getClass().getSimpleName() );
+ }
+ final String msg = throwable.getMessage();
+ if ( isNotEmpty( msg ) )
{
- result.append( " " );
- result.append( target.getClass().getSimpleName() );
- result.append( getTruncatedMessage( throwable.getMessage(), MAX_LINE_LENGTH - result.length() ) );
+ result.append( ' ' )
+ .append( msg.split( "\r?\n" )[0] );
Review comment:
Fine with me. I'll close this pull request then?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #457: [SUREFIRE-1992] Do not output class name on error/failure for certain exceptions
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #457:
URL: https://github.com/apache/maven-surefire/pull/457#discussion_r799571827
##########
File path: surefire-api/src/main/java/org/apache/maven/surefire/api/report/LegacyPojoStackTraceWriter.java
##########
@@ -81,19 +80,26 @@ public String smartTrimmedStackTrace()
result.append( "#" );
result.append( testMethod );
SafeThrowable throwable = getThrowable();
- if ( throwable.getTarget() instanceof AssertionError )
- {
- result.append( " " );
- result.append( getTruncatedMessage( throwable.getMessage(), MAX_LINE_LENGTH - result.length() ) );
- }
- else
+ Throwable target = throwable.getTarget();
+
+ if ( target != null )
{
- Throwable target = throwable.getTarget();
- if ( target != null )
+
+ final String excClassName = target.getClass().getName();
+
+ if ( ! ( AssertionError.class.isInstance( target )
+ || excClassName.endsWith( ".AssertionFailedError" )
+ || excClassName.endsWith( ".ComparisonFailure" )
+ || excClassName.startsWith( "org.opentest4j." ) ) )
+ {
+ result.append( ' ' )
+ .append( target.getClass().getSimpleName() );
+ }
+ final String msg = throwable.getMessage();
+ if ( isNotEmpty( msg ) )
{
- result.append( " " );
- result.append( target.getClass().getSimpleName() );
- result.append( getTruncatedMessage( throwable.getMessage(), MAX_LINE_LENGTH - result.length() ) );
+ result.append( ' ' )
+ .append( msg.split( "\r?\n" )[0] );
Review 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 wahe additionalt 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?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #457: [SUREFIRE-1992] Do not output class name on error/failure for certain exceptions
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #457:
URL: https://github.com/apache/maven-surefire/pull/457#discussion_r800161922
##########
File path: surefire-api/src/main/java/org/apache/maven/surefire/api/report/LegacyPojoStackTraceWriter.java
##########
@@ -81,19 +80,26 @@ public String smartTrimmedStackTrace()
result.append( "#" );
result.append( testMethod );
SafeThrowable throwable = getThrowable();
- if ( throwable.getTarget() instanceof AssertionError )
- {
- result.append( " " );
- result.append( getTruncatedMessage( throwable.getMessage(), MAX_LINE_LENGTH - result.length() ) );
- }
- else
+ Throwable target = throwable.getTarget();
+
+ if ( target != null )
{
- Throwable target = throwable.getTarget();
- if ( target != null )
+
+ final String excClassName = target.getClass().getName();
+
+ if ( ! ( AssertionError.class.isInstance( target )
+ || excClassName.endsWith( ".AssertionFailedError" )
+ || excClassName.endsWith( ".ComparisonFailure" )
+ || excClassName.startsWith( "org.opentest4j." ) ) )
+ {
+ result.append( ' ' )
+ .append( target.getClass().getSimpleName() );
+ }
+ final String msg = throwable.getMessage();
+ if ( isNotEmpty( msg ) )
{
- result.append( " " );
- result.append( target.getClass().getSimpleName() );
- result.append( getTruncatedMessage( throwable.getMessage(), MAX_LINE_LENGTH - result.length() ) );
+ result.append( ' ' )
+ .append( msg.split( "\r?\n" )[0] );
Review 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?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] sman-81 commented on a change in pull request #457: [SUREFIRE-1992] Do not output class name on error/failure for certain exceptions
Posted by GitBox <gi...@apache.org>.
sman-81 commented on a change in pull request #457:
URL: https://github.com/apache/maven-surefire/pull/457#discussion_r800176861
##########
File path: surefire-api/src/main/java/org/apache/maven/surefire/api/report/LegacyPojoStackTraceWriter.java
##########
@@ -81,19 +80,26 @@ public String smartTrimmedStackTrace()
result.append( "#" );
result.append( testMethod );
SafeThrowable throwable = getThrowable();
- if ( throwable.getTarget() instanceof AssertionError )
- {
- result.append( " " );
- result.append( getTruncatedMessage( throwable.getMessage(), MAX_LINE_LENGTH - result.length() ) );
- }
- else
+ Throwable target = throwable.getTarget();
+
+ if ( target != null )
{
- Throwable target = throwable.getTarget();
- if ( target != null )
+
+ final String excClassName = target.getClass().getName();
+
+ if ( ! ( AssertionError.class.isInstance( target )
+ || excClassName.endsWith( ".AssertionFailedError" )
+ || excClassName.endsWith( ".ComparisonFailure" )
+ || excClassName.startsWith( "org.opentest4j." ) ) )
+ {
+ result.append( ' ' )
+ .append( target.getClass().getSimpleName() );
+ }
+ final String msg = throwable.getMessage();
+ if ( isNotEmpty( msg ) )
{
- result.append( " " );
- result.append( target.getClass().getSimpleName() );
- result.append( getTruncatedMessage( throwable.getMessage(), MAX_LINE_LENGTH - result.length() ) );
+ result.append( ' ' )
+ .append( msg.split( "\r?\n" )[0] );
Review 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org