You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "michael-o (via GitHub)" <gi...@apache.org> on 2023/05/25 09:21:41 UTC

[GitHub] [maven-surefire] michael-o opened a new pull request, #645: Simplify serialization/deserialization of elapsed time

michael-o opened a new pull request, #645:
URL: https://github.com/apache/maven-surefire/pull/645

   Following this checklist to help us incorporate your 
   contribution quickly and easily:
   
    - [ ] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/SUREFIRE) filed 
          for the change (usually before you start working on it).  Trivial changes like typos do not 
          require a JIRA issue.  Your pull request should address just this issue, without 
          pulling in other changes.
    - [ ] Each commit in the pull request should have a meaningful subject line and body.
    - [ ] Format the pull request title like `[SUREFIRE-XXX] - Fixes bug in ApproximateQuantiles`,
          where you replace `SUREFIRE-XXX` with the appropriate JIRA issue. Best practice
          is to use the JIRA issue title in the pull request title and in the first line of the 
          commit message.
    - [ ] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
    - [ ] Run `mvn clean install` to make sure basic checks pass. A more thorough check will 
          be performed on your pull request automatically.
    - [ ] You have run the integration tests successfully (`mvn -Prun-its clean install`).
   
   If your pull request is about ~20 lines of code you don't need to sign an
   [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf) if you are unsure
   please ask on the developers list.
   
   To make clear that you license your contribution under 
   the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
   you have to acknowledge this by using the following check-box.
   
    - [ ] I hereby declare this contribution to be licenced under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
   
    - [ ] In any other case, please file an [Apache Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
   


-- 
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] michael-o commented on a diff in pull request #645: Simplify serialization/deserialization of elapsed time (SUREFIRE-2164 + SUREFIRE-2167)

Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #645:
URL: https://github.com/apache/maven-surefire/pull/645#discussion_r1205929141


##########
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporter.java:
##########
@@ -387,7 +390,9 @@ private void startTestElement(XMLWriter ppw, WrappedReportEntry report) throws I
             ppw.addAttribute("classname", extraEscapeAttribute(className));
         }
 
-        ppw.addAttribute("time", report.elapsedTimeAsString());
+        if (report.getElapsed() != null) {
+            ppw.addAttribute("time", String.valueOf(report.getElapsed() / ONE_SECOND));

Review Comment:
   No, it won't. That's the good point about it. It is a straight locale-neutral serialization of a float.



-- 
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] michael-o commented on a diff in pull request #645: Simplify serialization/deserialization of elapsed time (SUREFIRE-2164 + SUREFIRE-2167)

Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #645:
URL: https://github.com/apache/maven-surefire/pull/645#discussion_r1205937494


##########
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporter.java:
##########
@@ -387,7 +390,9 @@ private void startTestElement(XMLWriter ppw, WrappedReportEntry report) throws I
             ppw.addAttribute("classname", extraEscapeAttribute(className));
         }
 
-        ppw.addAttribute("time", report.elapsedTimeAsString());

Review Comment:
   Yet another problem is that it was used only in one dir, not in both which is inconsistent and non-portable. I have basically applied what Ant does: https://github.com/apache/ant/blob/6f68fbdefad521741b37304cfdfc6d8646cf1839/src/main/org/apache/tools/ant/taskdefs/optional/junit/XMLJUnitResultFormatter.java#L189-L190



-- 
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] asfgit merged pull request #645: Simplify serialization/deserialization of elapsed time (SUREFIRE-2164 + SUREFIRE-2167)

Posted by "asfgit (via GitHub)" <gi...@apache.org>.
asfgit merged PR #645:
URL: https://github.com/apache/maven-surefire/pull/645


-- 
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] michael-o commented on a diff in pull request #645: Simplify serialization/deserialization of elapsed time (SUREFIRE-2164 + SUREFIRE-2167)

Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #645:
URL: https://github.com/apache/maven-surefire/pull/645#discussion_r1205929141


##########
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporter.java:
##########
@@ -387,7 +390,9 @@ private void startTestElement(XMLWriter ppw, WrappedReportEntry report) throws I
             ppw.addAttribute("classname", extraEscapeAttribute(className));
         }
 
-        ppw.addAttribute("time", report.elapsedTimeAsString());
+        if (report.getElapsed() != null) {
+            ppw.addAttribute("time", String.valueOf(report.getElapsed() / ONE_SECOND));

Review Comment:
   No, it won't. That's the good point about it. It is a straight locale-neutral serialization if a float.



-- 
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] slawekjaranowski commented on a diff in pull request #645: Simplify serialization/deserialization of elapsed time (SUREFIRE-2164 + SUREFIRE-2167)

Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on code in PR #645:
URL: https://github.com/apache/maven-surefire/pull/645#discussion_r1206532613


##########
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporter.java:
##########
@@ -387,7 +390,9 @@ private void startTestElement(XMLWriter ppw, WrappedReportEntry report) throws I
             ppw.addAttribute("classname", extraEscapeAttribute(className));
         }
 
-        ppw.addAttribute("time", report.elapsedTimeAsString());

Review Comment:
   ok



-- 
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] michael-o commented on a diff in pull request #645: Simplify serialization/deserialization of elapsed time (SUREFIRE-2164 + SUREFIRE-2167)

Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #645:
URL: https://github.com/apache/maven-surefire/pull/645#discussion_r1205932070


##########
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporter.java:
##########
@@ -387,7 +390,9 @@ private void startTestElement(XMLWriter ppw, WrappedReportEntry report) throws I
             ppw.addAttribute("classname", extraEscapeAttribute(className));
         }
 
-        ppw.addAttribute("time", report.elapsedTimeAsString());

Review Comment:
   No, we can't. See here: https://github.com/apache/maven-surefire/pull/635/commits/9dab367d7e200022b4e88eb58cdfcf281824a124#diff-efbf1a613b0d3b854d7a732ab8f6221fa5ac34210449cdfb4eee6bb61a60c1ccR185
   In fact, this subsequent commit makes `ReporterUtils` superfluous. My object was to clearly seperate serialization/deserialization of machine data (which are never locale-specific, but neutral), rendering happens in the view layer: https://github.com/apache/maven-surefire/pull/635/commits/9dab367d7e200022b4e88eb58cdfcf281824a124#diff-efbf1a613b0d3b854d7a732ab8f6221fa5ac34210449cdfb4eee6bb61a60c1ccR59-R61



-- 
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] slawekjaranowski commented on a diff in pull request #645: Simplify serialization/deserialization of elapsed time (SUREFIRE-2164 + SUREFIRE-2167)

Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on code in PR #645:
URL: https://github.com/apache/maven-surefire/pull/645#discussion_r1205911407


##########
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporter.java:
##########
@@ -387,7 +390,9 @@ private void startTestElement(XMLWriter ppw, WrappedReportEntry report) throws I
             ppw.addAttribute("classname", extraEscapeAttribute(className));
         }
 
-        ppw.addAttribute("time", report.elapsedTimeAsString());
+        if (report.getElapsed() != null) {
+            ppw.addAttribute("time", String.valueOf(report.getElapsed() / ONE_SECOND));

Review Comment:
   This can depend on user locales like use dot or comma as separator - should we take care about it?



##########
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporter.java:
##########
@@ -387,7 +390,9 @@ private void startTestElement(XMLWriter ppw, WrappedReportEntry report) throws I
             ppw.addAttribute("classname", extraEscapeAttribute(className));
         }
 
-        ppw.addAttribute("time", report.elapsedTimeAsString());

Review Comment:
   Can we remove method `elapsedTimeAsString` and used `org.apache.maven.plugin.surefire.report.ReporterUtils`?



-- 
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