Closed
Bug 1020574
Opened 10 years ago
Closed 8 years ago
[mozmill] modernize calendar's sharedModules
Categories
(Calendar :: General, enhancement, P1)
Calendar
General
Tracking
(Not tracked)
RESOLVED
DUPLICATE
of bug 1303626
People
(Reporter: Taraman, Assigned: Taraman)
References
Details
Attachments
(4 obsolete files)
The shared modules for the calendar Mozmill-Tests use some modules, which are not available in the comm-central repository. These can be replaced by others used by Thunderbird in Mozmill-Testing thus enabling calendar tests to run as packaged tests.
Attachment #8434421 -
Flags: review?(philipp)
Comment 1•10 years ago
|
||
Comment on attachment 8434421 [details] [diff] [review] mozmillSharedModules.diff Review of attachment 8434421 [details] [diff] [review]: ----------------------------------------------------------------- r=philipp if you can present a green try-run with this applied :-) ::: calendar/test/mozmill/shared-modules/test-calendar-utils.js @@ +7,4 @@ > > +Cu.import("resource://gre/modules/Services.jsm"); > +var os = {}; Cu.import('resource://mozmill/stdlib/os.js', os); > +var frame = {}; Cu.import('resource://mozmill/modules/frame.js', frame); I believe you can also write this as: var frame = Cu.import('...', {}); @@ +59,5 @@ > + */ > +function getCalProperty(bundleName, propId) { > + var props = Services.strings.createBundle( > + "chrome://calendar/locale/" + bundleName + ".properties"); > + return props.GetStringFromName(propId); Can't you use cal.calGetString for this?
Attachment #8434421 -
Flags: review?(philipp) → review+
Assignee | ||
Comment 2•10 years ago
|
||
> I believe you can also write this as: > var frame = Cu.import('...', {}); Yes, that works. And I present to you: A working try-run: https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=56d4151e98ed The failures on linux are not in our tests... > Can't you use cal.calGetString for this? There had been problems loading the calendar-modules in the tests. Meanwhile I believe it works. For now I can leave this function out, because I will not need it until the timezone-tests are fixed. There I can try to use calGetString. With this changed, I am going to push soon.
Assignee | ||
Comment 3•10 years ago
|
||
updated patch per the last comments r=philipp
Attachment #8434421 -
Attachment is obsolete: true
Attachment #8442871 -
Flags: review+
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 4•10 years ago
|
||
Comment on attachment 8442871 [details] [diff] [review] patch V2 r=philipp There are changes to be made to this patch for mac compatibility as I just found out. Therefore this is obsolete and checkin-needed cancelled.
Attachment #8442871 -
Attachment is obsolete: true
Attachment #8442871 -
Flags: review+ → review-
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 5•10 years ago
|
||
This is a patch with updated "setData" function. I changed all the blocks containing "if (!mac)" statements to be platform independant. For that, I took out combinations of "a + ctrlKey" since the ctrlKey does not work on Mac. Instead, now all these parts contain ".select()" on the text-fields which also causes the whole text to be selected. Surely this does not imitate the user interaction exactly, but since we are not testing for the behaviour of textboxes, I think this is acceptable. I could not test the patch, because the builds are busted at the moment. Philip, please test this when you have a working building env. Unfortunately there are no tests working at the moment, which use this function, but we can make sure the module works overall.
Attachment #8443925 -
Flags: review?(philipp)
Comment 6•10 years ago
|
||
Comment on attachment 8443925 [details] [diff] [review] patch V2.1 Review of attachment 8443925 [details] [diff] [review]: ----------------------------------------------------------------- I ran our three basic tests with only this patch applied locally and it seems to work. To be sure, I've started a try run too. I hope the recent bustages don't hinder the builds, I haven't updated in a while so we might be safe. https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=562436af1f0f Code looks fine, r=philipp If the try build passes, go ahead and push.
Attachment #8443925 -
Flags: review?(philipp) → review+
Comment 7•10 years ago
|
||
New try run for this bug and bug 980515 now that c-c is green again: https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=2d130d00f10e
Comment 8•8 years ago
|
||
Does this have importance in the effort to move to ical.js?
Flags: needinfo?(philipp)
Comment 9•8 years ago
|
||
I don't think it does. This would be nice cleanup for the mozmill tests though. I'm making some updates in bug 1303626 so this patch may need some updates.
Flags: needinfo?(philipp)
Assignee | ||
Updated•8 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → DUPLICATE
Assignee | ||
Comment 11•7 years ago
|
||
Debitrotted pacth after Bug 1297425
Attachment #8443925 -
Attachment is obsolete: true
Attachment #8817340 -
Flags: review?(makemyday)
Comment 12•7 years ago
|
||
I guess, this patch was intended for bug 1303626?
Assignee | ||
Comment 13•7 years ago
|
||
Uh, yes. I was too tired and too quick yesterday...
Updated•7 years ago
|
Attachment #8817340 -
Attachment is obsolete: true
Attachment #8817340 -
Flags: review?(makemyday)
You need to log in
before you can comment on or make changes to this bug.
Description
•