bug#34315: [PATCH] icalendar.el: DURATION fix + more robust timezone handling

[Thomas Plass]

Lars Ingebrigtsen wrote at 03:45 on October 1, 2020:
: 
: However, the code makes two tests fail:

Like with the previous patch, I hoped to get away with offloading the
testing part to the maintainer... But the failures prompted me to get
familiar with the test setup for the two culprits.

: 2 unexpected results:
:    FAILED  icalendar--convert-tz-offset
:    FAILED  icalendar--parse-vtimezone

My latest patch requires some test definitions to be modified as well
as existing commented out code to be commented in.

Background: In light of the way an abandonment of daylight saving time
is expressed in VTIMEZONE, my patch will always make sure that - when
no recurrence rules RRULE are specified - the (most recent) UTC offset
of the most recent DAYLIGHT or STANDARD is returned.  This offset is
what counts, IMHO.  Computing and using a TZ spec - even a default one
- seems inappropriate for theses cases.

: In addition, the test cases included aren't actually used?

Implicitly, they are when my patch to icalendar-tests.el (below, not a
--git, sorry) is applied.  The raw ICS data seem rather large for
incorporation into the Lisp code.

But I'd rather let Ulf have the final say about the patches and how
the data is to be tested.


----------------------------------------------------------------------
diff -Naur icalendar-tests.el.bce7de769e0c024b26729c45c2c27c7f3749c499 
icalendar-tests.el
--- icalendar-tests.el.bce7de769e0c024b26729c45c2c27c7f3749c499 2020-10-01 
12:59:40.051753100 +0200
+++ icalendar-tests.el  2020-10-01 14:04:42.106937900 +0200
@@ -248,7 +248,7 @@
     (setq result (icalendar--parse-vtimezone vtimezone))
     (should (string= "Kolkata, Chennai, Mumbai, New Delhi" (car result)))
     (message (cdr result))
-    (should (string= "STD-05:30DST-05:30,M1.1.1/00:00:00,M1.1.1/00:00:00"
+    (should (string= "STD-05:30"
                      (cdr result)))
 
     ;; FIXME: add testcase that covers changes for fix of bug#34315
@@ -557,28 +557,29 @@
                             (RRULE nil "FREQ=YEARLY;BYMONTH=3;BYDAY=-1SU"))
                           t)))
 
-          ;; testcase: dtstart is mandatory
-          (should (null (icalendar--convert-tz-offset
+          ;; testcase: dtstart is mandatory but if not set return at least the 
offset
+          (should (equal '("DST-02:00")
+                         (icalendar--convert-tz-offset
                           '((TZOFFSETFROM nil "+0100")
                             (TZOFFSETTO nil "+0200")
                             (RRULE nil "FREQ=YEARLY;BYMONTH=3;BYDAY=-1SU"))
                           t)))
 
-          ;; FIXME: rrule and rdate are NOT mandatory!  Must fix code
-          ;; before activating these testcases
-          ;; ;; testcase: no rrule and no rdate => no result
-          ;; (should (null (icalendar--convert-tz-offset
-          ;;                 '((TZOFFSETFROM nil "+0100")
-          ;;                   (TZOFFSETTO nil "+0200")
-          ;;                   (DTSTART nil "19700329T020000"))
-          ;;                 t)))
-          ;; ;; testcase: no rrule with rdate => no result
-          ;; (should (null (icalendar--convert-tz-offset
-          ;;                 '((TZOFFSETFROM nil "+0100")
-          ;;                   (TZOFFSETTO nil "+0200")
-          ;;                   (DTSTART nil "18840101T000000")
-          ;;                   (RDATE nil "18840101T000000"))
-          ;;                 t)))
+          ;; rrule and rdate are NOT mandatory: return the offset and a 
placeholder
+          (should (equal '("DST-02:00" . "19700329T020000")
+                         (icalendar--convert-tz-offset
+                          '((TZOFFSETFROM nil "+0100")
+                            (TZOFFSETTO nil "+0200")
+                            (DTSTART nil "19700329T020000"))
+                          t)))
+          ;; ;; testcase: no rrule with rdate => return offset and a default 
TZ spec
+          (should (equal '("DST-02:00" . "M1.1.1/00:00:00")
+                         (icalendar--convert-tz-offset
+                          '((TZOFFSETFROM nil "+0100")
+                            (TZOFFSETTO nil "+0200")
+                            (DTSTART nil "18840101T000000")
+                            (RDATE nil "18840101T000000"))
+                          t)))
           )
       ;; restore time-zone even if something went terribly wrong
       (setenv "TZ" tz))))