Skip to content
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

[SUREFIRE-1615] Sort <testcase> in the surefire-report/Test-*.xml file #208

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lanlingxueyu
Copy link

@lanlingxueyu lanlingxueyu commented Dec 12, 2018

Sort methodRunHistoryMap
Eliminate the *Test.xml report tag out of order,
Resolve differences in rebuilding *Test.xml files

Sort methodRunHistoryMap
@Tibor17
Copy link
Contributor

Tibor17 commented Dec 12, 2018

@lanlingxueyu
If you want to have a fix like this, subscribe in Jira (see the link in parent POM), create an issue as a feature, name the commit as we usually do (see the history), and write an integration test which proves the expected behavior on you desired change.

@lanlingxueyu lanlingxueyu changed the title Update StatelessXmlReporter.java [SUREFIRE-1615] Sort <testcase> in the surefire-report/Test-*.xml file Dec 13, 2018
@lanlingxueyu
Copy link
Author

ok,already edited

//map.entrySet() Convert into list
List<Map.Entry<String, List<WrappedReportEntry>>> methodRunHistoryMapCoIntoList =
new ArrayList<Map.Entry<String, List<WrappedReportEntry>>>( methodRunHistoryMap.entrySet() );
//sort
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pls do not use comments.
Can you use static import instead of this long line Collections.sort? And import Map.Entry as well. Then pls do not rephrase reference type in local variable name methodRunHistoryMapCoIntoList and use plural of methodReportEntries.
Can you please extract this part into possibly a private static method and write a unit test just for that? You should call private method with PowerMock using org.powermock.reflect.Whitebox;. As a hint see our unit tests in Surefire.


//map.entrySet() Convert into list
List<Map.Entry<String, List<WrappedReportEntry>>> methodRunHistoryMapCoIntoList =
new ArrayList<Map.Entry<String, List<WrappedReportEntry>>>( methodRunHistoryMap.entrySet() );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In Java 1.7 the constructor should use diamond instead of Generics.

@Tibor17
Copy link
Contributor

Tibor17 commented Dec 13, 2018

@lanlingxueyu
Thx and now pls write an IT. As a hint see any *IT.java in surefire-its/src/test/java. Each IT calls one project in surefire-its/src/test/resources with POM and sources, see the method
unpack( "<with directory name in surefire-its/src/test/resources>" ) in the IT.
See our code style rules https://maven.apache.org/developers/conventions/code.html

@nhojpatrick nhojpatrick mentioned this pull request Mar 13, 2022
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants