You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jena.apache.org by ales004 <gi...@git.apache.org> on 2016/05/12 05:28:58 UTC
[GitHub] jena pull request: JENA-1175: Test function for Sprintf that is Ti...
GitHub user ales004 opened a pull request:
https://github.com/apache/jena/pull/143
JENA-1175: Test function for Sprintf that is Timezone aware.
I changed the test function for Sprintf so that now it should be timezone aware. It is difficult to test if it is working correctly as Jena is using the default timezone always but I think that the current implementation should solve the problem.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ales004/jena TestFix
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/jena/pull/143.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #143
----
commit 5676aeb180619a0f7cb48bf820fd8932999d8250
Author: ales004 <ci...@hotmail.com>
Date: 2016-05-10T21:23:11Z
Test function for Sprintf that is Timezone aware.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: JENA-1175: Test function for Sprintf that is Ti...
Posted by afs <gi...@git.apache.org>.
Github user afs commented on the pull request:
https://github.com/apache/jena/pull/143#issuecomment-218739759
`test_exprSprintf_05` is ignoring it's first argument. Bother.
If NodeValue.makeDateTime is changed:
String exprStr = "afn:sprintf('%1$tm %1$te,%1$tY', +NodeValue.makeDateTime(**nodeStr**).toString()+")" ;
does it work?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: JENA-1175: Test function for Sprintf that is Ti...
Posted by afs <gi...@git.apache.org>.
Github user afs commented on the pull request:
https://github.com/apache/jena/pull/143#issuecomment-219229233
I includes the new tests and kept the temporary tests so testing happens both ways.
Tests renumbered as well.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: JENA-1175: Test function for Sprintf that is Ti...
Posted by kinow <gi...@git.apache.org>.
Github user kinow commented on a diff in the pull request:
https://github.com/apache/jena/pull/143#discussion_r63004959
--- Diff: jena-arq/src/test/java/org/apache/jena/sparql/expr/TestFunctions.java ---
@@ -81,28 +89,35 @@
// test("afn:sprintf('%1$tm %1$te,%1$tY', "+nodeStr+")",NodeValue.makeString("10 14,2005")) ;
// }
- private static void test_exprSprintf_05(String nodeStr, String... possible) {
- String exprStr = "afn:sprintf('%1$tm %1$te,%1$tY', "+NodeValue.makeDateTime("2005-10-14T13:09:43Z").toString()+")" ;
+ private static void test_exprSprintf_05(String nodeStr) {
+ String exprStr = "afn:sprintf('%1$tm %1$te,%1$tY', "+NodeValue.makeDateTime(nodeStr).toString()+")" ;
Expr expr = ExprUtils.parse(exprStr) ;
NodeValue r = expr.eval(null, FunctionEnvBase.createTest()) ;
assertTrue(r.isString()) ;
String s = r.getString() ;
- // Timezones! The locale data can be -1, 0, +1 from the Z day.
- boolean b = false ;
- for (String poss : possible ) {
- if ( poss.equals(s) )
- b = true ;
+ // Parse the date
+ String dtFormat = "yyyy-MM-dd'T'HH:mm:ssXXX";
+ SimpleDateFormat sdtFormat = new SimpleDateFormat(dtFormat);
+ Date dtDate = null;
+ try {
+ dtDate = sdtFormat.parse(nodeStr);
+ } catch (ParseException e) {
+ assertFalse("Cannot parse the input date string. Message:"+e.getMessage(),false);
}
- assertTrue(b) ;
+ // print the date based on the current timeZone.
+ SimpleDateFormat stdFormatOut = new SimpleDateFormat("MM dd,yyyy");
+ stdFormatOut.setTimeZone(TimeZone.getDefault());
+ String outDate = stdFormatOut.format(dtDate);
+ assertEquals(s,outDate);
}
// Temporary fix for JENA-1175
// Timezone -11:00 to any timezone can be a day ahead
- @Test public void exprSprintf_05a() { test_exprSprintf_05("2005-10-14T14:09:43-11:00", "10 14,2005", "10 15,2005") ; }
+ @Test public void exprSprintf_05a() { test_exprSprintf_05("2005-10-14T14:09:43-11:00") ; }
// Timezone Z to any timezone can be a day behind or a day ahead
--- End diff --
Looking at the changes, I think this PR may make that test pass. But the comment seems to indicate that we must test with Z suffix as well. Or update the comment if that makes sense.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: JENA-1175: Test function for Sprintf that is Ti...
Posted by ales004 <gi...@git.apache.org>.
Github user ales004 commented on the pull request:
https://github.com/apache/jena/pull/143#issuecomment-218751154
I had to sobstitute Z with 00:00 because the SimpleDateFormat is accepting either
yyyy-MM-dd'T'HH:mm:ssXXX
or
yyyy-MM-dd'T'HH:mm:ssZ
not both at the time.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: JENA-1175: Test function for Sprintf that is Ti...
Posted by kinow <gi...@git.apache.org>.
Github user kinow commented on the pull request:
https://github.com/apache/jena/pull/143#issuecomment-218737575
Righto, so we just update the comment later. I got an error while testing in my current time zone (Pacific/Auckland).
With the code in the master branch, test receives 2005-10-14T10:09:43+11:00 as parameter. The possible values are [10 13,2005, 10 14,2005].
Inside the test, 2005-10-14T13:09:43Z is computed as: "10 15,2005", and the test fails.
Will test the pull request to confirm whether it is failing too or not.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: JENA-1175: Test function for Sprintf that is Ti...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/jena/pull/143
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: JENA-1175: Test function for Sprintf that is Ti...
Posted by kinow <gi...@git.apache.org>.
Github user kinow commented on the pull request:
https://github.com/apache/jena/pull/143#issuecomment-218743887
FWIW, tested the pull request locally, the test seems to have passed (the build is still running, but will take a while. Arq tests passed fine).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: JENA-1175: Test function for Sprintf that is Ti...
Posted by afs <gi...@git.apache.org>.
Github user afs commented on the pull request:
https://github.com/apache/jena/pull/143#issuecomment-218735541
`Z` is the same-value-as `+00:00`, testing both is ideal.
It is commenting that if the data is Z, which it is for `exprSprintf_05b`, the outcome can be a day behind (i.e. Hawaii direction) or ahead (New Zealand direction) or the same (UTC) depending on the exact time.
Timezones range from -14:00 to +14:00.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: JENA-1175: Test function for Sprintf that is Ti...
Posted by kinow <gi...@git.apache.org>.
Github user kinow commented on the pull request:
https://github.com/apache/jena/pull/143#issuecomment-218740515
I think so. I noticed the first argument was not being used, tried that in Eclipse, and the test passed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---