1020574 - [mozmill] modernize calendar's sharedModules

Status: RESOLVED DUPLICATE of bug 1303626

(Calendar :: General, enhancement, P1)

Description

[Markus Adrario [:Taraman]]

Attachment: mozmillSharedModules.diff

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.

Comment 1

[Philipp Kewisch [:Fallen][📱📆]]

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?

Comment 2

[Markus Adrario [:Taraman]]

> 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.

Comment 3

[Markus Adrario [:Taraman]]

Attachment: patch V2 r=philipp

updated patch per the last comments

r=philipp

Comment 4

[Markus Adrario [:Taraman]]

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.

Comment 5

[Markus Adrario [:Taraman]]

Attachment: patch V2.1

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.

Comment 6

[Philipp Kewisch [:Fallen][📱📆]]

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.

Comment 7

[Philipp Kewisch [:Fallen][📱📆]]

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

[Wayne Mery (:wsmwk)]

Does this have importance in the effort to move to ical.js?

Comment 9

[Philipp Kewisch [:Fallen][📱📆]]

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.

Comment 11

[Markus Adrario [:Taraman]]

Attachment: Patch V2.2

Debitrotted pacth after Bug 1297425

Comment 12

[[:MakeMyDay]]

I guess, this patch was intended for bug 1303626?

Comment 13

[Markus Adrario [:Taraman]]

Uh, yes.
I was too tired and too quick yesterday...