Uploaded image for project: 'Maven Surefire'
  1. Maven Surefire
  2. SUREFIRE-1276

Document handling of multiline exception messages

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.20
    • Component/s: Maven Surefire Plugin
    • Labels:
      None

      Description

      I am throwing an exception whose message contains newlines for readability. When <trimStackTrace> is true (the default value) Surefire is removing all newlines.

      Expected behavior: Consider moving this functionality out of <trimStackTrace> into its own parameter. Regardless of which parameter enables this behavior, please document it at http://maven.apache.org/surefire/maven-surefire-plugin/test-mojo.html ... right now there is no mention of this behavior.

        Activity

        Hide
        tibor17 Tibor Digana added a comment -

        commit 0aeceecc3b85d1133a1a2f2335a89950689e33bb

        Show
        tibor17 Tibor Digana added a comment - commit 0aeceecc3b85d1133a1a2f2335a89950689e33bb
        Hide
        cowwoc Gili added a comment -
        Show
        cowwoc Gili added a comment - Tibor Digana Sorry for the delay. See https://github.com/apache/maven-surefire/pull/116
        Hide
        tibor17 Tibor Digana added a comment -

        Gili
        Do you have time for this?

        Show
        tibor17 Tibor Digana added a comment - Gili Do you have time for this?
        Show
        tibor17 Tibor Digana added a comment - perhaps faq and https://maven.apache.org/surefire/maven-failsafe-plugin/newerrorsummary.html https://github.com/apache/maven-surefire/blob/master/maven-surefire-plugin/src/site/fml/faq.fml https://github.com/apache/maven-surefire/blob/master/maven-surefire-plugin/src/site/markdown/newerrorsummary.md
        Hide
        cowwoc Gili added a comment -

        Tibor Digana What file do you expect me to submit a PR for this documentation change? When I click on "Release Notes" on your website, I get an empty page: https://maven.apache.org/surefire/maven-surefire-plugin/jira-report.html

        I was thinking we should add this to the release notes of 2.19 (which contained SUREFIRE-986). I'm thinking we should mention this elsewhere (FAQ maybe?), but I can't think of anywhere else that a user would look in to discover this feature. Any ideas?

        Show
        cowwoc Gili added a comment - Tibor Digana What file do you expect me to submit a PR for this documentation change? When I click on "Release Notes" on your website, I get an empty page: https://maven.apache.org/surefire/maven-surefire-plugin/jira-report.html I was thinking we should add this to the release notes of 2.19 (which contained SUREFIRE-986 ). I'm thinking we should mention this elsewhere (FAQ maybe?), but I can't think of anywhere else that a user would look in to discover this feature. Any ideas?
        Hide
        tibor17 Tibor Digana added a comment -

        String#isEmpty() is available since of Java 1.6 and we compile code with 1.5.
        The point of countNewLines == 1 && !msg.trim().endsWith( "\n" ) is to print
        java.lang.IllegalArgumentException: Message
        instead of
        {{java.lang.IllegalArgumentException:
        Message}}
        if you call throw new IllegalArgumentException("Message\n");
        The point of Groovy fix is to align expression and value.
        The code countNewLines == 1 && !msg.trim().endsWith( "\n" ) avoids situation with throw new IllegalArgumentException("Message\n"); without unnecessary call .isEmpty() and without cutting msg with tokenizer.

        Show
        tibor17 Tibor Digana added a comment - String#isEmpty() is available since of Java 1.6 and we compile code with 1.5. The point of countNewLines == 1 && !msg.trim().endsWith( "\n" ) is to print java.lang.IllegalArgumentException: Message instead of {{java.lang.IllegalArgumentException: Message}} if you call throw new IllegalArgumentException("Message\n"); The point of Groovy fix is to align expression and value. The code countNewLines == 1 && !msg.trim().endsWith( "\n" ) avoids situation with throw new IllegalArgumentException("Message\n"); without unnecessary call .isEmpty() and without cutting msg with tokenizer.
        Hide
        cowwoc Gili added a comment -

        Tibor Digana Will do. On a side-note, don't you find the conditional of https://github.com/apache/maven-surefire/commit/6e5fb5695a48ad072cc354e31b8fd82656b9d320#diff-fa9299aa9dbda1fe1c3f400814d2b25fR142 a bit confusing? I would recommend adding brackets to clarify the operator ordering. Also, its not clear when !msg.trim().endsWith( "\n" ) would ever return true. I'm guessing that replacing that with !msg.trim().isEmpty() would result in the same behavior but be more readable.

        Show
        cowwoc Gili added a comment - Tibor Digana Will do. On a side-note, don't you find the conditional of https://github.com/apache/maven-surefire/commit/6e5fb5695a48ad072cc354e31b8fd82656b9d320#diff-fa9299aa9dbda1fe1c3f400814d2b25fR142 a bit confusing? I would recommend adding brackets to clarify the operator ordering. Also, its not clear when !msg.trim().endsWith( "\n" ) would ever return true. I'm guessing that replacing that with !msg.trim().isEmpty() would result in the same behavior but be more readable.
        Hide
        tibor17 Tibor Digana added a comment -

        Feel free to add a documentation.

        Show
        tibor17 Tibor Digana added a comment - Feel free to add a documentation.
        Hide
        cowwoc Gili added a comment -

        Tibor Digana Please ignore my shortened testcase (the one with "Message"). I shortened it at the last minute and never actually testing it locally. I know that with a longer string that contains \n I see the same behavior you mentioned. So yes, I believe we are in agreement.

        Show
        cowwoc Gili added a comment - Tibor Digana Please ignore my shortened testcase (the one with "Message"). I shortened it at the last minute and never actually testing it locally. I know that with a longer string that contains \n I see the same behavior you mentioned. So yes, I believe we are in agreement.
        Hide
        tibor17 Tibor Digana added a comment - - edited

        Gili
        Something is wrong on your side because I have expected result and the newline is added only if the message has \n character:

                    result.append( t.getClass().getName() );
                    String msg = t.getMessage();
                    if ( msg != null )
                    {
                        result.append( ": " );
                        if ( isMultiLine( msg ) )
                        {
                            // SUREFIRE-986
                            result.append( '\n' );
                        }
                        result.append( msg );
                    }
        

        Check your CLI window, maybe it's cutting the lines.

        Running ATest
        Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.065 s - in ATest
        Running SomeTest
        Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.001 s <<< FAILURE! - in SomeTest
        test(SomeTest) Time elapsed: 0.001 s <<< ERROR!
        java.lang.IllegalArgumentException: Message
        at SomeTest.test(SomeTest.java:9)

        Results:

        Tests in error:
        SomeTest.test:9 IllegalArgument Message

        Tests run: 2, Failures: 0, Errors: 1, Skipped: 0

        Show
        tibor17 Tibor Digana added a comment - - edited Gili Something is wrong on your side because I have expected result and the newline is added only if the message has \n character: result.append( t.getClass().getName() ); String msg = t.getMessage(); if ( msg != null ) { result.append( ": " ); if ( isMultiLine( msg ) ) { // SUREFIRE-986 result.append( '\n' ); } result.append( msg ); } Check your CLI window, maybe it's cutting the lines. Running ATest Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.065 s - in ATest Running SomeTest Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.001 s <<< FAILURE! - in SomeTest test(SomeTest) Time elapsed: 0.001 s <<< ERROR! java.lang.IllegalArgumentException: Message at SomeTest.test(SomeTest.java:9) Results: Tests in error: SomeTest.test:9 IllegalArgument Message Tests run: 2, Failures: 0, Errors: 1, Skipped: 0
        Hide
        cowwoc Gili added a comment - - edited

        Tibor Digana If understand the code correctly, when the exception message spans multiple lines, Surefire inserts a leading newline character to make sure that the text is aligned. If so, this is a good idea, but should be documented as you mentioned. If you confirm my understanding, I will try producing a PR for this.

        Show
        cowwoc Gili added a comment - - edited Tibor Digana If understand the code correctly, when the exception message spans multiple lines, Surefire inserts a leading newline character to make sure that the text is aligned. If so, this is a good idea, but should be documented as you mentioned. If you confirm my understanding, I will try producing a PR for this.
        Hide
        tibor17 Tibor Digana added a comment -

        no need to run the POM, I see the code talks the same language as your experiences.

        Show
        tibor17 Tibor Digana added a comment - no need to run the POM, I see the code talks the same language as your experiences.
        Hide
        tibor17 Tibor Digana added a comment -

        Gili
        If we need to have improvements added to our documentation according to your expectations, feel free to open PR in GitHub.

        Show
        tibor17 Tibor Digana added a comment - Gili If we need to have improvements added to our documentation according to your expectations, feel free to open PR in GitHub.
        Hide
        tibor17 Tibor Digana added a comment -

        it's because of SUREFIRE-986

        Show
        tibor17 Tibor Digana added a comment - it's because of SUREFIRE-986
        Hide
        cowwoc Gili added a comment -

        Confirmed. Surefire 2.12.4 trims newlines in exception messages but 2.19.1 does not. So that bug is fixed.

        The second problem, which is unaffected by the value of <trimStacktrace>, can be reproduced in version 2.19.1 as follows:

        Run a unit test containing:

        	@Test
        	public void test()
        	{
        		throw new IllegalArgumentException("Message");
        	}
        

        You will get the following output:

        java.lang.IllegalArgumentException: 
        Message
        

        However, if you catch the exception inside the test method and invoke printStackTrace() then the output will be:

        java.lang.IllegalArgumentException: Message
        

        Let me know if you can reproduce this. Otherwise I will create a project as you requested.

        Show
        cowwoc Gili added a comment - Confirmed. Surefire 2.12.4 trims newlines in exception messages but 2.19.1 does not. So that bug is fixed. The second problem, which is unaffected by the value of <trimStacktrace> , can be reproduced in version 2.19.1 as follows: Run a unit test containing: @Test public void test() { throw new IllegalArgumentException( "Message" ); } You will get the following output: java.lang.IllegalArgumentException: Message However, if you catch the exception inside the test method and invoke printStackTrace() then the output will be: java.lang.IllegalArgumentException: Message Let me know if you can reproduce this. Otherwise I will create a project as you requested.
        Hide
        tibor17 Tibor Digana added a comment -

        Gili
        Trim means that stack trace entry is printed if the class is your test. It has nothing to do with exception message; otherwise the issue is not clear to me. Can you attach a project?

        Show
        tibor17 Tibor Digana added a comment - Gili Trim means that stack trace entry is printed if the class is your test. It has nothing to do with exception message; otherwise the issue is not clear to me. Can you attach a project?
        Hide
        tibor17 Tibor Digana added a comment -

        If the message contains \n new line is printed which is a fix in some of the last version(s).

        Show
        tibor17 Tibor Digana added a comment - If the message contains \n new line is printed which is a fix in some of the last version(s).
        Hide
        cowwoc Gili added a comment -

        I just noticed a second related problem. Even with <trimStackTrace> disabled, Surefire is translating:

        java.lang.IllegalArgumentException: some message
        

        into

        java.lang.IllegalArgumentException:
        some message
        

        Meaning, it is adding a newline before the exception message. Again this behavior is undocumented and there is no obvious way to disable it.

        Show
        cowwoc Gili added a comment - I just noticed a second related problem. Even with <trimStackTrace> disabled, Surefire is translating: java.lang.IllegalArgumentException: some message into java.lang.IllegalArgumentException: some message Meaning, it is adding a newline before the exception message. Again this behavior is undocumented and there is no obvious way to disable it.

          People

          • Assignee:
            tibor17 Tibor Digana
            Reporter:
            cowwoc Gili
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development