Closed Bug 530165 Opened 11 years ago Closed 11 years ago

[Mozmill] Timezone test

Categories

(Calendar :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: merike, Assigned: merike)

References

()

Details

Attachments

(5 files)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; et; rv:1.9.2b3) Gecko/20091115 Firefox/3.6b3
Build Identifier: 

Timezone test.

Reproducible: Always
Attached patch v1Splinter Review
Additionally change from bug 530159 needs to be applied.
Attachment #413683 - Flags: review?(ctalbert)
Assignee: nobody → gasell+mozilla
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Comment on attachment 413683 [details] [diff] [review]
v1

I can't get these to work on Windows or Mac. They have problems with the "verify" function in Timezone Utils.  It consistently fails to pass the check at the bottom of the file.  It doesn't seem to be going to the different dates when it is trying to do the verification.  It stays on one date (whatever is currently selected), so I'm not sure what is up with that.

Can you figure out what is going on?  If you want, I can run some instrumented code to help figure out what is going on.
Attachment #413683 - Flags: review?(ctalbert) → review-
(In reply to comment #2)
> (From update of attachment 413683 [details] [diff] [review]) 
> Can you figure out what is going on?  If you want, I can run some instrumented
> code to help figure out what is going on.

I have figured out the issue on mac.  It was the first click that you are doing on the month-header of goToDate.  By adding the label in to the first click, it starts to work.  I think without that the click misses the label and the popup is not activated.

Perhaps you can see if getting that label in place on the other platforms enables them to work as well?  I've included a set of code to show what I changed, it was small.  I figured it out by looking at the Dom inspector, and found that there is nothing to identify the active xul:toolbarbutton element in the original code.  The label allows the system to identify the proper xul:toolbarbutton element and direct the click appropriately.  Here's the code, I just made the change to include the label attribute:

  controller.window.alert("picking month now");

  // pick month

  controller.click(new elementslib.Lookup(controller.window.document, miniMonth

    + 'anon({"anonid":"minimonth-header"})/anon({"anonid":"monthheader"})/{"label":"December"}'));

  controller.sleep(500);

  controller.click(new elementslib.Lookup(controller.window.document, miniMonth

    + 'anon({"anonid":"minimonth-header"})/anon({"anonid":"minmonth-popupset"})/'

    + 'anon({"anonid":"months-popup"})/[0]/{"index":"' + (month - 1) + '"}'));

  controller.sleep(500);

  controller.window.alert("done picking month fool");

Hope that helps.  I hope you also had a good Christmas and a happy new year.
Attached patch v2Splinter Review
(In reply to comment #3)
> I have figured out the issue on mac.  It was the first click that you are doing
> on the month-header of goToDate.  By adding the label in to the first click, it
> starts to work.  I think without that the click misses the label and the popup
> is not activated.
> 
> Perhaps you can see if getting that label in place on the other platforms
> enables them to work as well?

Actually other platforms work both ways. My best guess is that on Mac you're dealing with a button that is actually rounded and thus the click on header won't work. On Linux for example that button looks rounded but is actually a rectangle. Not sure why I left out button level first place though, even Mozmill's recorder records it.

Here's what has changed:
* goToDate clicks button instead of header, doesn't use label but position though as I don't want to fetch localised month names if it can be avoided
* timezone change sets preference instead of using UI dialog, due to bug #536605
* setting timezone on an event specifies checked attributeon the menuitem, I'm not sure why it's needed (no other menuitem with checkbox needs it) but it does work

How's that running on Mac?
Attachment #419329 - Flags: review?(ctalbert)
Comment on attachment 419329 [details] [diff] [review]
v2

This is looking a lot better.  It's running really well now, but I'm looking at some inconsistencies in the timezone data.  For example, on test 3, we are in the St. John's timezone.  You created the St. John's event to be at 8AM St. John time.  However, you are verifying it at 4:30AM in timezone 3, date Jan 1, 2009.

So, I have two things for you to double check here: 
1. When you do the double click to create your events, be sure that you are consistently creating the events at the same times on each OS.  I'll attach an exported ICS file showing when the events were created on mac.  We may need to go with forcing the time in test 2 if they are different across OS's.

1a. The way the event dialog works is that it will treat the "start/end" time on the dialog as local timezone time.  So once you change the timezone, those times will automatically change.  So if you start in Europe/London and you click on 8AM, then inside the dialog you will default to 8AM.  Once you change the timezone to Europe/Paris, the start/end time will change to start at 9am and end at 10AM to reflect your original time selection in your new timezone.  That may be another reason the data is being thrown out of sync.

2. I am seeing a bug in the calendar on mac that it isn't updating the UI properly when the timezone is changed.  I'll file that as a follow on. For this review, I'll just ensure that the test is attempting to verify the right events at the right time and I won't care so much about whether it passes or fails the UI check due to this issue.
Attachment #419329 - Flags: review?(ctalbert) → review-
This is an ICS export of the timezone events that are created by this test on mac.  To read this: the first 123 lines are timezone definitions.  The first event (America/St_Johns) starts at line 124.  Check out the DTSTART attribute: DTSTART;TZID=America/St_Johns:20090101T080000, shows that the event begins in the America/St_Johns timezone at 8AM on Jan 1 2009.

However, when we attempt to verify that the event is correct in test3.js we are looking for it at 4:30AM instead of 8AM.
This test keeps getting weirder. I'm aware of event dialog converting time on timezone change and it's weird still.

When I export created calendar after test2 I get DTSTART;TZID=America/St_Johns:20090101T043000 out of it. Given that this event is first opened as 8am event and there's a timezone change of 3.5 hours it should end up 4:30am event local time. I'm not sure why you get it at 8am local time. The idea is to create it at 4:30 local time and not at 8:00.

I could adjust test2 so that it enters start time after changing timezone and you would get the same events that I get, but that really shouldn't be necessary as event dialog *should* get you 4:30 after changing timezone any way. Could you check visually whether after changing first event's timezone start time is 8:00 or 4:30? For me it's 4:30 and given the way event dialog works it should be 4:30 on Mac too. Is it not?

View update issue could be bug 454181 and this behaviour is why these are restart tests and not regular mozmill tests. This way it first changes app's timezone, restarts and then checks events in that timezone and therefore it shouldn't be affected by view issues after timezone change without restart.
(In reply to comment #7)
> This test keeps getting weirder. I'm aware of event dialog converting time on
> timezone change and it's weird still.
> 
Welcome to timezones? :( Yeah, getting this monster to run is a bear.

> When I export created calendar after test2 I get
> DTSTART;TZID=America/St_Johns:20090101T043000 out of it. Given that this event
> is first opened as 8am event and there's a timezone change of 3.5 hours it
> should end up 4:30am event local time. I'm not sure why you get it at 8am local
> time. The idea is to create it at 4:30 local time and not at 8:00.
> 
> I could adjust test2 so that it enters start time after changing timezone and
> you would get the same events that I get, but that really shouldn't be
> necessary as event dialog *should* get you 4:30 after changing timezone any
> way. Could you check visually whether after changing first event's timezone
> start time is 8:00 or 4:30? For me it's 4:30 and given the way event dialog
> works it should be 4:30 on Mac too. Is it not?
> 
You know, I seem to recall that there were a bunch of default settings as to where the day view is scrolled based on the "current" time you run the test.  Is that still the case?  I wonder if my day view is consistently scrolled later than yours because my events come in much later than your events do.  It might be best to force the event dialog to use your current times after changing to the target timezone.  (or perhaps changing if the time does not match what you expect it to be).  

> View update issue could be bug 454181 and this behavior is why these are
> restart tests and not regular mozmill tests. This way it first changes app's
> timezone, restarts and then checks events in that timezone and therefore it
> shouldn't be affected by view issues after timezone change without restart.
That is probably what I'm hitting, because I'm running these from inside the IDE itself.  I mistakenly thought we'd fixed that so that you don't have to do the restart anymore.  :(  I always hated that we had to do the restart.
(In reply to comment #8)
> You know, I seem to recall that there were a bunch of default settings as to
> where the day view is scrolled based on the "current" time you run the test. 
> Is that still the case?  I wonder if my day view is consistently scrolled later
> than yours because my events come in much later than your events do.  It might
> be best to force the event dialog to use your current times after changing to
> the target timezone.  (or perhaps changing if the time does not match what you
> expect it to be).  

View scroll shouldn't interfere here. All the boxes are there no matter which ones are visible so index is relative to midnight not to the first box in view. Unless it's different on Mac of course. In fact sending a doubleclick to a box that is not visible doesn't trigger event dialog even.

Anyhow, if you only get these events at wrong time in Mozmill test and not when you do the same manually then I agree it's best to set times explicitly. I'll make a modified version so that we can see if this was the only thing going wrong.
Attachment #421931 - Flags: review?(ctalbert)
I ran this on windows and it passed beautifully.  Running it on mac os X has issues, and I'm not exactly sure why.  I've attached the log from the run, and I may not have time to dig into these until tomorrow.  In both runs, I didn't touch the mouse or keyboard during the run so I don't think it is focus related.  I imagine that the verification of the hour offsets is working differently on mac OS due to the slightly different mac theme.  

Does this pass on linux too?  Because if it does, I'm almost inclined to take it as a windows/linux test and leave mac out of the picture.  The timezone code is mostly JS these days and so I'm not worried about mac-only timezone bugs. 

Philipp, Martin: thoughts?
Passes fine on linux for me, I'm also fine with making this win/linux only. Please add a comment explaining why though.
Comment on attachment 421931 [details] [diff] [review]
v3 Setting starttime explicitly

Merike, I have figured out the issue on mac.  The problem was that I had a an old version of timezone definitions on my test profile.  Once I corrected that, it all started working.  I just ran all the tests on mac OS and they all passed beautifully.

Congratulations Merike on a job very well done.  Boo on me for not figuring out that issue sooner.
Attachment #421931 - Flags: review?(ctalbert) → review+
Pushed as d41e7d54aef3 
--> FIXED (whoop!)

Time for some "timezone juice"
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.