[SUREFIRE-1992] - Do not abbreviate test error/failure messages to 78 characters #452

spannm · 2022-01-28T08:20:20Z

Pull request for ticket [SUREFIRE-1992] which removes abbreviation of test error/failure messages to 78 characters in two classes that write test results to stdout/stderr.
The fix greatly improves usability of surefire when troubleshooting test failures.

Tibor17 · 2022-01-28T13:07:03Z

surefire-api/src/main/java/org/apache/maven/surefire/api/report/LegacyPojoStackTraceWriter.java

        if ( throwable.getTarget() instanceof AssertionError )
+        if ( throwable.getTarget() instanceof AssertionError


This if ( ... ) is a pure trojan horse.

This section of code is suited for another Jira ticket.
The way we split the fix purpose we can better separate commits per issue in the git history.

unintentional. I'll amend the PR

Feel free to open a new Jira and PR with acceptance of asserions exceptions.

@sman-81
I am not sure why the author of smartTrimmedStackTrace() ignores other exceptions. Try to open a new PR and try to turn the IF statement to:
if ( throwable.getTarget() != null )
I want to know if the old code makes strong sense to the tests.

The exception message may have multiple lines. They may produce a huge report on the console. My advice is to print only the first line if exists.

Tibor17 · 2022-01-28T13:43:45Z

surefire-api/src/main/java/org/apache/maven/surefire/api/report/LegacyPojoStackTraceWriter.java

@@ -81,10 +79,15 @@ public String smartTrimmedStackTrace()
        result.append( "#" );
        result.append( testMethod );
        SafeThrowable throwable = getThrowable();
+        final String excClassName = throwable.getTarget().getClass().getName();


This may throw NPE.

Hint: try to use Java 1.8 Optional.

surefire-api/src/main/java/org/apache/maven/surefire/api/report/LegacyPojoStackTraceWriter.java

Tibor17 · 2022-01-28T13:47:31Z

surefire-api/src/main/java/org/apache/maven/surefire/api/report/LegacyPojoStackTraceWriter.java

@@ -93,7 +96,7 @@ public String smartTrimmedStackTrace()
            {
                result.append( " " );
                result.append( target.getClass().getSimpleName() );
-                result.append( getTruncatedMessage( throwable.getMessage(), MAX_LINE_LENGTH - result.length() ) );
+                result.append( throwable.getMessage() );


Again missing the null check.

Tibor17 · 2022-01-28T13:47:47Z

...iders/common-java5/src/main/java/org/apache/maven/surefire/report/SmartStackTraceParser.java

@@ -149,7 +147,7 @@ public String getString()
            result.append( rootIsInclass() ? " " : " » " )
                    .append( toMinimalThrowableMiniMessage( excType ) );

-            result.append( truncateMessage( msg, MAX_LINE_LENGTH - result.length() ) );
+            result.append( msg );


spannm · 2022-01-28T14:12:47Z

Thanks for your feedback!
I couldn't get the project to compile in Eclipse, but should have at least built it with Maven on the command line.
The latest commit should address all the issues you raised with my pr.

Tibor17 · 2022-01-28T14:33:06Z

@sman-81
Enable the profile ide-development and install the module surefire-shared-utils and reimport the project in your IDE. Then you can run the tests in the IDE and compile.

slawekjaranowski · 2022-01-28T14:37:26Z

we should add instruction to readme ...

spannm · 2022-01-28T15:08:12Z

we should add instruction to readme ...

that would be helpful

@Tibor17 My latest commit limits error/failure messages to first line. Also added a new test.

Tibor17 · 2022-01-28T15:25:44Z

@sman-81
yes, we will add it to README.
Let's wait fo the CI.

spannm · 2022-01-28T17:11:12Z

re: IDE Setup

In Eclipse, right-click on project -> Maven -> Select Maven Profiles ..., pick ide-development

Despite this still 194 errors.

Eclipse keeps building in an endless loop. CPU usage is high.

A major source of errors is missing GroupMatcherParser which gets generated into
surefire-grouper/target/generated-sources/javacc/org/apache/maven/surefire/group/parse/GroupMatcherParser.java (in Eclipse)
Thus this source is not part of any source folder.

The POM relies on org.codehaus.mojo:javacc-maven-plugin. This plugin is ancient ... We should consider moving to org.javacc.plugin:javacc-maven-plugin and placing generated sources in a different location. Maybe build-helper-maven-plugin can help in a solution.

Anyways, any pointers how to set up in Eclipse are welcome.

Tibor17 · 2022-01-28T23:12:00Z

@sman-81
I can confirm the plugin was maintained in 2020 and 2021. The Codehaus plugin was last time released in 2009.
Feel free to open a new PR regarding the new plugin as a substitute of codehaus plugin.

spannm · 2022-01-29T08:21:15Z

@Tibor17

@sman-81 I can confirm the plugin was maintained in 2020 and 2021. The Codehaus plugin was last time released in 2009. Feel free to open a new PR regarding the new plugin as a substitute of codehaus plugin.

I will look into it. If we are lucky the successor is a drop-in replacement. Codehaus has beeen defunct for quite some time afaik.

When will the automatic checks of this pull request continue? The status has been: 1 successful, 3 cancelled, and 9 queued checks 🤔

spannm · 2022-01-29T09:43:58Z

Issue SUREFIRE-1994: Upgrade and configure javacc-maven-plugin created.

spannm · 2022-01-31T12:33:51Z

What's holding up the CI?

spannm · 2022-02-02T18:08:16Z

I'm new to this project.
After a very vibrant and produktive discussion, which ultimately led to a better, team-agreed solution, there is silence.
The CI does not continue checking this PR but remains in cancelled state.
Can the CI be triggered again? I meant to ask

spannm · 2022-02-03T05:58:23Z

@slawekjaranowski

Thanks for triggering the re-run. Test org.apache.maven.surefire.its.CheckTestNgVersionsIT crashed in Verify / macOS-latest jdk-17-temurin.
I didn't think this is related to the code change however.
What to do in this case?

Tibor17 · 2022-02-03T06:03:28Z

@sman-81
We investigated ugly issue where the CI randomly failed. This was very important and so the outcome is last few commits.

Tibor17 · 2022-02-03T06:07:39Z

@sman-81
I think you have made a great job.
I can see several improvements you have made so far and they should split in more PRs. These are namely:

length of 80 characters
printing the first line
the improvements with additional exception to catch.

Would you please fork this work in several PRs? Thx

Tibor17 · 2022-02-03T06:25:12Z

@slawekjaranowski
I have to trigger the CI for this PR as well. Why it happens?
The other PRs run automatically.

slawekjaranowski · 2022-02-03T11:26:14Z

Last build was hanging ... I rerun also ... I will not investigate why .. sometime some of build hang on GH.
For first time contributor maintainer must approve build.

spannm · 2022-02-03T14:32:05Z

@Tibor17

Would you please fork this work in several PRs? Thx

I don't know how to do this.

Tibor17 · 2022-02-04T10:46:52Z

@sman-81
It can be done easily. Since I am using IntelliJ IDEA, I know that I can create new branches and then I can go to the particular branch and select a history of the old branch sman-81-SUREFIRE-1992 and press right button and select Cherry-Pick and push it in the newly created branch, then open the PR from the branch. The same with next branch. I do not have Eclipse installed.

…haracters

spannm · 2022-02-04T11:35:40Z

Hi @Tibor17

It can be done easily. Since I am using IntelliJ IDEA, ...

I'm OK using command line. Good practice :)

1 length of 80 characters

2 printing the first line

3 the improvements with additional exception to catch.

This PR is now limited to point 1 above.

Two new pull requests are sman-81-SUREFIRE-1992-2 for point 2 and sman-81-SUREFIRE-1992-3 for point 3.

Tibor17 · 2022-02-06T12:38:42Z

@sman-81
Now this commit looks much better. Since the other branches/PRs have the same Jira id, it has confused me.
Let's concentrate on this and see the CI result...

spannm · 2022-02-08T19:50:25Z

Tibor17 · 2022-02-08T22:42:10Z

@sman-81
Thx for contributing.

spannm · 2022-02-09T10:37:26Z

@sman-81 Thx for contributing.

my pleasure, it was fun

Tibor17 reviewed Jan 28, 2022

View reviewed changes

surefire-api/src/main/java/org/apache/maven/surefire/api/report/LegacyPojoStackTraceWriter.java Outdated Show resolved Hide resolved

Tibor17 reviewed Jan 28, 2022

View reviewed changes

[SUREFIRE-1992] Do not abbreviate test error/failure messages to 78 c…

51c1eb8

…haracters

Tibor17 merged commit 89cffc2 into apache:master Feb 8, 2022

spannm deleted the sman-81-SUREFIRE-1992 branch February 16, 2022 12:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SUREFIRE-1992] - Do not abbreviate test error/failure messages to 78 characters #452

[SUREFIRE-1992] - Do not abbreviate test error/failure messages to 78 characters #452

spannm commented Jan 28, 2022

Tibor17 Jan 28, 2022

Tibor17 Jan 28, 2022

spannm Jan 28, 2022

Tibor17 Jan 28, 2022

Tibor17 Jan 28, 2022

Tibor17 Jan 28, 2022

Tibor17 Jan 28, 2022

Tibor17 Jan 28, 2022

Tibor17 Jan 28, 2022

Tibor17 Jan 28, 2022

spannm commented Jan 28, 2022

Tibor17 commented Jan 28, 2022

slawekjaranowski commented Jan 28, 2022

spannm commented Jan 28, 2022 •

edited

Tibor17 commented Jan 28, 2022

spannm commented Jan 28, 2022

Tibor17 commented Jan 28, 2022

spannm commented Jan 29, 2022

spannm commented Jan 29, 2022

spannm commented Jan 31, 2022

spannm commented Feb 2, 2022 •

edited

spannm commented Feb 3, 2022

Tibor17 commented Feb 3, 2022

Tibor17 commented Feb 3, 2022 •

edited

Tibor17 commented Feb 3, 2022 •

edited

slawekjaranowski commented Feb 3, 2022

spannm commented Feb 3, 2022

Tibor17 commented Feb 4, 2022

spannm commented Feb 4, 2022

Tibor17 commented Feb 6, 2022

spannm commented Feb 8, 2022

Tibor17 commented Feb 8, 2022

spannm commented Feb 9, 2022

		if ( throwable.getTarget() instanceof AssertionError )
		if ( throwable.getTarget() instanceof AssertionError

[SUREFIRE-1992] - Do not abbreviate test error/failure messages to 78 characters #452

[SUREFIRE-1992] - Do not abbreviate test error/failure messages to 78 characters #452

Conversation

spannm commented Jan 28, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spannm commented Jan 28, 2022

Tibor17 commented Jan 28, 2022

slawekjaranowski commented Jan 28, 2022

spannm commented Jan 28, 2022 • edited

Tibor17 commented Jan 28, 2022

spannm commented Jan 28, 2022

re: IDE Setup

Tibor17 commented Jan 28, 2022

spannm commented Jan 29, 2022

spannm commented Jan 29, 2022

spannm commented Jan 31, 2022

spannm commented Feb 2, 2022 • edited

spannm commented Feb 3, 2022

Tibor17 commented Feb 3, 2022

Tibor17 commented Feb 3, 2022 • edited

Tibor17 commented Feb 3, 2022 • edited

slawekjaranowski commented Feb 3, 2022

spannm commented Feb 3, 2022

Tibor17 commented Feb 4, 2022

spannm commented Feb 4, 2022

Tibor17 commented Feb 6, 2022

spannm commented Feb 8, 2022

Tibor17 commented Feb 8, 2022

spannm commented Feb 9, 2022

spannm commented Jan 28, 2022 •

edited

spannm commented Feb 2, 2022 •

edited

Tibor17 commented Feb 3, 2022 •

edited

Tibor17 commented Feb 3, 2022 •

edited