mangstadt / biweekly

biweekly is an iCalendar library written in Java.
BSD 2-Clause "Simplified" License
323 stars 44 forks source link

3 tests are failing in America/Los_Angeles timezone #74

Closed gangeli closed 4 years ago

gangeli commented 7 years ago

I suspect these are timezone errors (?), because the CI system marks the build as passing:

[INFO] Results:
[INFO] 
[ERROR] Failures: 
[ERROR]   CompoundIteratorImplTest.infiniteExclusionsAndFiniteInclusions:414 expected:<[20060411, 20060412, 20060414, 20060417, 20060418, 20060419, 20060420, 20060421, 20060424, 20060425, 20060426, 20060428, 20060501, 20060502]> but was:<[20060411, 20060412, 20060414, 20060417, 20060418, 20060419, 20060420, 20060421, 20060424, 20060425, 20060426, 20060428, 20060501, 20060502, 20060503]>
[ERROR]   CompoundIteratorImplTest.infiniteExclusionsAndFiniteInclusions_advanceTo_after_date:493 expected:<[20060424, 20060425, 20060426, 20060428, 20060501, 20060502]> but was:<[20060424, 20060425, 20060426, 20060428, 20060501, 20060502, 20060503]>
[ERROR]   CompoundIteratorImplTest.infiniteExclusionsAndFiniteInclusions_advanceTo_on_date:456 expected:<[20060418, 20060419, 20060420, 20060421, 20060424, 20060425, 20060426, 20060428, 20060501, 20060502]> but was:<[20060418, 20060419, 20060420, 20060421, 20060424, 20060425, 20060426, 20060428, 20060501, 20060502, 20060503]>
[INFO] 
[ERROR] Tests run: 1132, Failures: 3, Errors: 0, Skipped: 0
gangeli commented 7 years ago

On closer inspection, I believe it's actually the tests that are incorrect. The until value on a recurrence is meant to be inclusive (https://tools.ietf.org/html/rfc5545#section-3.3.10), which means the extra date in the "failure" is I believe actually expected behavior. Pull request inbound with the changed tests.

mangstadt commented 7 years ago

Someone else noticed problems with this unit test in the past. A pull request was submitted, but it failed for other timezones, so I rolled it back.

See: https://github.com/mangstadt/biweekly/pull/63

gangeli commented 7 years ago

Yeah, I also submitted a pull request to try to fix this, but it fails on Travis. Do you have any insights on what might be wrong? It's somewhat worrying that unit tests fail at different time zones -- and, I believe the tests I submitted are the technically correct output. That is, the until value is inclusive in the RFC spec.

mangstadt commented 7 years ago

The code related to this unit test was actually imported from another library. I've had a very hard time understanding its codebase. The code is located in the biweekly.util.com.google.ical package.

I refactored the code a lot to remove functionality that wasn't needed (notably, iCal parsing logic and various DTO classes). I could have introduced a bug somewhere inadvertently.

cloudtestsoftware commented 6 years ago

Here is the exception for American time zone. The URL is broken,

EVERE: Servlet.service() for servlet [Jersey REST Service] in context with path [/bidcrm] threw exception java.lang.IllegalArgumentException: Timezone ID not recognized. at biweekly.io.TzUrlDotOrgGenerator.notFound(TzUrlDotOrgGenerator.java:152) at biweekly.io.TzUrlDotOrgGenerator.generate(TzUrlDotOrgGenerator.java:91) at biweekly.io.TimezoneAssignment.download(TimezoneAssignment.java:100) at com.bidcrm.calendar.service.ICalendarDataService.setTimeZone(ICalendarDataService.java:247) at com.bidcrm.calendar.service.ICalendarDataService.createICalendar(ICalendarDataService.java:95) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:498) at com.sun.jersey.spi.container.JavaMethodInvokerFactory$1.invoke(JavaMethodInvokerFactory.java:60) at com.sun.jersey.server.impl.model.method.dispatch.AbstractResourceMethodDispatchProvider$ResponseOutInvoker._dispatch(AbstractResourceMethodDispatchProvider.java:205) at com.sun.jersey.server.impl.model.method.dispatch.ResourceJavaMethodDispatcher.dispatch(ResourceJavaMethodDispatcher.java:75) at com.sun.jersey.server.impl.uri.rules.HttpMethodRule.accept(HttpMethodRule.java:302) at com.sun.jersey.server.impl.uri.rules.RightHandPathRule.accept(RightHandPathRule.java:147) at com.sun.jersey.server.impl.uri.rules.ResourceClassRule.accept(ResourceClassRule.java:108) at com.sun.jersey.server.impl.uri.rules.RightHandPathRule.accept(RightHandPathRule.java:147) at com.sun.jersey.server.impl.uri.rules.RootResourceClassesRule.accept(RootResourceClassesRule.java:84) at com.sun.jersey.server.impl.application.WebApplicationImpl._handleRequest(WebApplicationImpl.java:1511) at com.sun.jersey.server.impl.application.WebApplicationImpl._handleRequest(WebApplicationImpl.java:1442) at com.sun.jersey.server.impl.application.WebApplicationImpl.handleRequest(WebApplicationImpl.java:1391) at com.sun.jersey.server.impl.application.WebApplicationImpl.handleRequest(WebApplicationImpl.java:1381) at com.sun.jersey.spi.container.servlet.WebComponent.service(WebComponent.java:416) at com.sun.jersey.spi.container.servlet.ServletContainer.service(ServletContainer.java:538) at com.sun.jersey.spi.container.servlet.ServletContainer.service(ServletContainer.java:716) at javax.servlet.http.HttpServlet.service(HttpServlet.java:741) at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:231) at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166) at org.apache.tomcat.websocket.server.WsFilter.doFilter(WsFilter.java:53) at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193) at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166) at org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:199) at org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:96) at org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:475) at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:140) at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:80) at org.apache.catalina.valves.AbstractAccessLogValve.invoke(AbstractAccessLogValve.java:651) at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:87) at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:342) at org.apache.coyote.http11.Http11Processor.service(Http11Processor.java:498) at org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:66) at org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:796) at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1374) at org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:49) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624) at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61) at java.lang.Thread.run(Thread.java:748) Caused by: java.io.FileNotFoundException: http://www.tzurl.org/zoneinfo-outlook/GMT at sun.net.www.protocol.http.HttpURLConnection.getInputStream0(HttpURLConnection.java:1872) at sun.net.www.protocol.http.HttpURLConnection.getInputStream(HttpURLConnection.java:1474) at java.net.URL.openStream(URL.java:1045) at biweekly.io.TzUrlDotOrgGenerator.getInputStream(TzUrlDotOrgGenerator.java:141) at biweekly.io.TzUrlDotOrgGenerator.generate(TzUrlDotOrgGenerator.java:88) ... 45 more

mangstadt commented 6 years ago

@cloudtestsoftware The problem you are having is unrelated to this issue.

It looks like you are trying to assigned a timezone called "GMT" to an iCalendar object. A timezone with that name cannot be found on the tzurl.org website. Biweekly formats all dates in GMT by default, so there is no need to explicitly assigned this timezone to an iCalendar object.

mangstadt commented 4 years ago

This issue should be fixed now. See: https://github.com/mangstadt/biweekly/pull/63#issuecomment-697933157