You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jmeter.apache.org by "FSchumacher (via GitHub)" <gi...@apache.org> on 2023/03/26 10:50:01 UTC

[GitHub] [jmeter] FSchumacher opened a new pull request, #5796: Warn about time format change only once

FSchumacher opened a new pull request, #5796:
URL: https://github.com/apache/jmeter/pull/5796

   ## Description
   
   Warn only once, when `u` is found in format string of the `__time` function.
   
   ## Motivation
   
   There could be other reasons, why `u` is contained in the format string. So use the format with a known date and check the result, if `u` might have been used.
   
   The warning will be issued only once.
   
   This implementation has a few loopholes/problems:
    * `u` could still be used for something different and the year could be coming from a `YYYY` format string
    * we add a volatile variable, which adds overhead
   
   I start to believe that it is probably better to revert to the old logic and live with the IDE warnings.
   
   Part of #5694
   
   ## How Has This Been Tested?
   
   Used two different format strings in the GUI and watched the logs.
   
   ## Types of changes
   <!--- What types of changes does your code introduce? Delete as appropriate -->
   - Bug fix (non-breaking change which fixes an issue)
   
   ## Checklist:
   <!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
   <!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->
   - [x] My code follows the [code style][style-guide] of this project.
   - [ ] I have updated the documentation accordingly.
   
   [style-guide]: https://wiki.apache.org/jmeter/CodeStyleGuidelines
   


-- 
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: dev-unsubscribe@jmeter.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [jmeter] vlsi commented on pull request #5796: Warn about time format change only once

Posted by "vlsi (via GitHub)" <gi...@apache.org>.
vlsi commented on PR #5796:
URL: https://github.com/apache/jmeter/pull/5796#issuecomment-1554451913

   Reworked the fix in https://github.com/apache/jmeter/pull/5934


-- 
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: dev-unsubscribe@jmeter.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [jmeter] vlsi commented on a diff in pull request #5796: Warn about time format change only once

Posted by "vlsi (via GitHub)" <gi...@apache.org>.
vlsi commented on code in PR #5796:
URL: https://github.com/apache/jmeter/pull/5796#discussion_r1148826181


##########
src/functions/src/main/java/org/apache/jmeter/functions/TimeFunction.java:
##########
@@ -96,13 +99,17 @@ public String execute(SampleResult previousResult, Sampler currentSampler) throw
                 long div = Long.parseLong(fmt.substring(1)); // should never case NFE
                 datetime = Long.toString(System.currentTimeMillis() / div);
             } else {
-                if (fmt.contains("u")) {
+                DateTimeFormatter df = DateTimeFormatter
+                        .ofPattern(fmt);

Review Comment:
   We could probably move the initialization of `DateTimeFormatter` to something like "once for a given pattern" since `DateTimeFormatter` is thread-safe.
   
   That might be a task for another PR though.



-- 
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: dev-unsubscribe@jmeter.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [jmeter] vlsi closed pull request #5796: Warn about time format change only once

Posted by "vlsi (via GitHub)" <gi...@apache.org>.
vlsi closed pull request #5796: Warn about time format change only once
URL: https://github.com/apache/jmeter/pull/5796


-- 
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: dev-unsubscribe@jmeter.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [jmeter] vlsi commented on a diff in pull request #5796: Warn about time format change only once

Posted by "vlsi (via GitHub)" <gi...@apache.org>.
vlsi commented on code in PR #5796:
URL: https://github.com/apache/jmeter/pull/5796#discussion_r1148534522


##########
src/functions/src/main/java/org/apache/jmeter/functions/TimeFunction.java:
##########
@@ -96,13 +99,17 @@ public String execute(SampleResult previousResult, Sampler currentSampler) throw
                 long div = Long.parseLong(fmt.substring(1)); // should never case NFE
                 datetime = Long.toString(System.currentTimeMillis() / div);
             } else {
-                if (fmt.contains("u")) {
+                DateTimeFormatter df = DateTimeFormatter // Not synchronised, so can't be shared
+                        .ofPattern(fmt);

Review Comment:
   Is it really non-thread-safe?
   
   The javadoc says otherwise: https://docs.oracle.com/javase/8/docs/api/java/time/format/DateTimeFormatter.html
   
   > A formatter created from a pattern can be used as many times as necessary, it is immutable and is thread-safe.



-- 
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: dev-unsubscribe@jmeter.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org