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

timeShift function docs update. #672

Closed
wants to merge 2 commits into from
Closed

Conversation

mawasak
Copy link
Contributor

@mawasak mawasak commented Sep 19, 2021

Description

Proposition to add additonal documentation for timeShift function.

Motivation and Context

timeShift is very flexible and has not documented implicite behaviour. This change is loosely connected with https://bz.apache.org/bugzilla/show_bug.cgi?id=65217

How Has This Been Tested?

Change is in functions.xml file so gradle previewSite task was used.

Screenshots (if appropriate):

NA

Types of changes

Only documentation in site, result will be visible on html https://jmeter.apache.org/usermanual/functions.html#__timeShift

Checklist:

NA

See <a href="https://docs.oracle.com/javase/8/docs/api/java/time/format/DateTimeFormatter.html">DateTimeFormatter</a>
If omitted, the function uses milliseconds since epoch format.
If omitted, the function returns milliseconds.
Copy link
Contributor

Choose a reason for hiding this comment

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

While it returns milliseconds, those are anchored at start of the unix epoch. Hence I think that detail should not be dropped.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, You are right. I mean that when format is not given and date to shift is given (and must be Long) the code is reduced to result that don't seem explicitly connect with epoch:

Duration duration = Duration.parse("PT10S");
ZoneId systemDefaultZoneID = ZoneId.systemDefault();
ZonedDateTime zonedDateTimeToShift = ZonedDateTime.ofInstant(Instant.ofEpochMilli(Long.parseLong("10000")), systemDefaultZoneID);
zonedDateTimeToShift = zonedDateTimeToShift.plus(duration);
String result = String.valueOf(zonedDateTimeToShift.toInstant().toEpochMilli());
System.out.println(result); //just 20000ms

But, yes, when date to shift is not given (is default: now) then connection with epoch is seen better:

Duration duration = Duration.parse("PT10S");
ZoneId systemDefaultZoneID = ZoneId.systemDefault();
ZonedDateTime zonedDateTimeToShift = ZonedDateTime.now(systemDefaultZoneID);
zonedDateTimeToShift = zonedDateTimeToShift.plus(duration);
String result = String.valueOf(zonedDateTimeToShift.toInstant().toEpochMilli());
System.out.println(result); //1632157138893ms

</property>
<property name="Date to shift" required="No">
Indicate the date in the format set by the parameter <code>Format</code> to shift.
If omitted, the date is set to <em>now</em>.
If omitted, the date is set to <em>ZonedDateTime.now</em> with system zone <em>ZoneId.systemDefault()</em>.
If <code>Format</code> parameter is not set then this property (if required) must be set as number of milliseconds eg.: Format="" and "Date to shift"=10000 then if "value to shift"=PT10S then result=20000 (20sec)
Copy link
Contributor

Choose a reason for hiding this comment

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

This last line feels like an example/explaining note to me. Should this be in an extra note paragraph?
At other places in this documentation, we use the code-tag for the name of parameters, when mentioned in the docs, would not this be helpful here, too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will fix it:)

@asfgit asfgit closed this in 462aaeb Sep 21, 2021
@FSchumacher
Copy link
Contributor

Thanks for the PR

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