[mozmill] fix and enhance shared-module calendar-utils "setData()"-Function

RESOLVED FIXED in 6.7

Status

enhancement
RESOLVED FIXED
11 months ago
10 months ago

People

(Reporter: Taraman, Assigned: Taraman)

Tracking

unspecified
Dependency tree / graph

Details

Attachments

(1 attachment, 3 obsolete attachments)

As a consequence of bug 1484636, my work on this for bug 984044 was somewhat obsoleted.

Since I completely fixed the setData Function and enhanced it to be able to fill all parts of the event-editing dialog/iframe, I like to still contribute this part.

Also I centralized some more of the lookup-paths so they can be used by other modules.
In bug 984044 I said:
>Working on the eventDialog tests, it shows that we have some helper-functions in test-calendar-utils.js, that deal with the >event-Dialog.
>
>I favour putting these in a seperate helper file called test-event-dialog-helpers.js.
>What is your opinion on this?

I still like to continue this way.
Meanwhile I think it could be event-editing-helpers, since we are not solely working with the event-dialogm but also do "in-Tab-editing"

Any objections?
Blocked by bug 1492095, because the EventDialog Test checks for appearance of the Alarm-Window.
Depends on: 1492095
Posted patch setData.diff (obsolete) — Splinter Review
This is a WIP-Patch with all tests passing locally.

Tryserver Build ist scheduled:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=311ebb00b6633948e9a497a4cbe4196b5034c239
Attachment #9013024 - Flags: feedback?(philipp)
Attachment #9013024 - Flags: feedback?(geoff)
Comment on attachment 9013024 [details] [diff] [review]
setData.diff

Review of attachment 9013024 [details] [diff] [review]:
-----------------------------------------------------------------

A good start. Thank you for doing some sorely-needed refactoring in here.

::: calendar/test/mozmill/shared-modules/test-calendar-utils.js
@@ +37,5 @@
>  `;
> +var DAYVIEW = `${VIEWDECK}/id("day-view")`;
> +var WEEKVIEW = `${VIEWDECK}/id("week-view")`;
> +var MULTIWEEKVIEW = `${VIEWDECK}/id("multiweek-view")`;
> +var MONTHVIEW = `${VIEWDECK}/id("month-view")`;

DAY_VIEW, WEEK_VIEW, etc.?

::: calendar/test/mozmill/shared-modules/test-item-editing-helpers.js
@@ +124,5 @@
> +function getInnerFramePath(iframe) {
> +    let type = cal.isEvent(iframe.window.calendarItem) ? "event" : "task";
> +    return `/id("calendar-${type}-dialog-inner")/id("event-grid")/
> +            id("event-grid-rows")`;
> +}

Maybe this should just be handled in helpersForController, so we don't have to keep repeating ourselves?

@@ +289,5 @@
> +
> +    // recurrence
> +    if (data.repeat != undefined) {
> +        menulistSelect(iframeid("item-repeat"), data.repeat, dialog);
> +    }

Could you add "repeat until"? We don't have anything that uses it yet, but we should.

@@ +418,5 @@
> + * @param controller      Mozmill controller of item-Iframe
> + * @param menulist        the reminder menulist node
> + * @param index           0-based position of the desired item (without separators)
> + */
> +function setReminderMenulist(controller, menulist, index) {

This would be better if it used a string matching the menuitem id. They're all in the form "reminder-something-menuitem", so passing "something" should be fine.
Attachment #9013024 - Flags: feedback?(geoff) → feedback+
(In reply to Geoff Lankow (:darktrojan) from comment #4)
> ::: calendar/test/mozmill/shared-modules/test-item-editing-helpers.js
> @@ +124,5 @@
> > +function getInnerFramePath(iframe) {
> > +    let type = cal.isEvent(iframe.window.calendarItem) ? "event" : "task";
> > +    return `/id("calendar-${type}-dialog-inner")/id("event-grid")/
> > +            id("event-grid-rows")`;
> > +}
> 
> Maybe this should just be handled in helpersForController, so we don't have
> to keep repeating ourselves?
I'm not sure what you mean.
I put this in the item-editing-helpers, because it only refers to the item-iframe. So mostly, we will have the item-editing-helpers needed and imported anyway when we need this.

The other 3 comments will be incorporated.
Thanks!
After reading and looking at the code, I now see what you mean. I check if I find a way to do this better.
Posted patch setData.diff V2 (obsolete) — Splinter Review
This incorporates the comments from the WIP-patch.

lots of lookup-paths are centralized now and other cleanup.
E.g.:
- made use of 100 character/line limit ISO 80
- tweaks to make tests more stable

New try run with the final patch:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=1a1d5455041ef9e074b764f2e57cf1129fb30909
Attachment #9013024 - Attachment is obsolete: true
Attachment #9013024 - Flags: feedback?(philipp)
Attachment #9013738 - Flags: review?(geoff)
Oh sure, as soon as I post a patch that is ready, all the try builds fail...
I made another Try push off an earlier m-c revision without some known problems. It seems testEventDialog is broken, and it might be causing the others in that directory to fail sometimes too.

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=253d19fd977afa0ae8d75dc91da9b7ce272b85a1&group_state=expanded
Yeah, it fails waiting for the alarm dialog.

For some reason the alarm is not set, when the alarm-menulist is clicked by the test.
When I try manually, it works.

I don't have the slightest idea why. Tried every combination of controller.click and menulist/menuitem.click and nothing works.
Comment on attachment 9013738 [details] [diff] [review]
setData.diff V2

Review of attachment 9013738 [details] [diff] [review]:
-----------------------------------------------------------------

I'm going to give this r+, but there's still stuff to do.

Since you're changing a lot of comments, sentences have capital letters and full stops. I get told off for this, so I'd better pass it on. :-)

Make sure you run ESLint on this too.

::: calendar/test/mozmill/cal-recurrence/testWeeklyNRecurrence.js
@@ +112,4 @@
>  }
>  
>  function setRecurrence(recurrence) {
> +    let { sleep: recsleep, lookup: reclookup, eid: recid } = helpersForController(recurrence);

This should go with the other stuff in test-item-editing-helpers.js.

::: calendar/test/mozmill/cal-recurrence/testWeeklyWithExceptionRecurrence.js
@@ +78,3 @@
>  
>      // move 5th January occurrence to 6th January
>      eventBox = lookupEventBox("day", EVENT_BOX, null, 1, HOUR, EVENTPATH);

The hour argument does nothing with EVENT_BOX, should be set to null.
(And elsewhere.)

@@ +96,4 @@
>  
>          event.waitForElement(eventid("item-repeat"));
>          plan_for_modal_dialog("Calendar:EventDialog:Recurrence", changeRecurrence);
> +        event.click(iframelookup(`${REPEAT_DETAILS}`));

No `${}` needed here.

@@ +116,3 @@
>          anon({"anonid":"daybox"})/[0]/anon({"anonid":"boxstack"})/
>          anon({"anonid":"topbox"})/{"flex":"1"}/{"flex":"1"}/[eventIndex]
>      `;

Can this be eliminated? Might need to modify getEventBoxPath to do so.
(And again below.)

::: calendar/test/mozmill/eventDialog/testEventDialog.js
@@ +89,1 @@
>          `);

I think this string and its 3 friends could be stored too. Seems like something we'd use a lot.

@@ +165,3 @@
>  
>      // 31st of January is Saturday so there's four more full rows to check
> +    date = 4;

date will already be 4.

::: calendar/test/mozmill/eventDialog/testEventDialogModificationPrompt.js
@@ +69,5 @@
> +        // pref can not be retrieved earlier, because it is not in mozmillprofile
> +        // and will be created only if the first event is created
> +        let categories = Services.prefs.getCharPref("calendar.categories.names").split(",");
> +        data[0].categories.push(categories[0]);
> +        data[1].categories.push(categories[1], categories[2]);

In testEventDialog you get these from the L10n strings. I think it's better to do it that way.

@@ +79,4 @@
>          event.click(eventid("button-saveandclose"));
>      });
>  
> +    eventbox = lookupEventBox("day", EVENT_BOX, null, 1, 8, EVENTPATH);

This lookup happens 6 times in this file, can we make it 1?

@@ +89,5 @@
> +        try {
> +            wait_for_modal_dialog("commonDialog", TIMEOUT_COMMON_DIALOG);
> +        } catch (e) {
> +            failPoints.first = "";
> +        }

This feels wrong. Can you put mark_failure inside the try block and get rid of failPoints?

@@ +183,2 @@
>              description: "description1",
> +            categories: [], // categories fail the test.

This comment doesn't describe what's happening accurately.

@@ +195,5 @@
>              freebusy: "busy",
>              timezone: true,
> +            attachment: { add: "http://mozilla.org" },
> +            // test fails when changing attendees, therefore leaving out for now
> +            // attendees: { add: "foo@bar.de,foo@bar.com" }

Has this been fixed?

::: calendar/test/mozmill/recurrenceRotated/testDailyRecurrence.js
@@ +41,1 @@
>      createCalendar(controller, CALENDARNAME);

Looks like you missed moving the rotation up to here.

::: calendar/test/mozmill/shared-modules/test-calendar-utils.js
@@ +40,5 @@
> +var WEEK_VIEW = `${VIEWDECK}/id("week-view")`;
> +// multiday-view-day-box of day and week view
> +var DAYBOX = `
> +    anon({"anonid":"mainbox"})/anon({"anonid":"scrollbox"})/anon({"anonid":"daybox"})
> +    `;

Stray whitespace.

@@ +91,5 @@
>      ({ open_pref_tab } = collector.getModule("pref-window-helpers"));
>      collector.getModule("pref-window-helpers").setupModule();
> +
> +    // For our tests, we assume that Sunday is start of week.
> +    Preferences.set("calendar.week.start", 0);

Oh good, this beats remembering to set environment variables when running tests. I'd prefer you to use Services.prefs.setIntPref though.

::: calendar/test/mozmill/shared-modules/test-item-editing-helpers.js
@@ +219,5 @@
> +    if (data.timezone != undefined) {
> +        let menuitem = eid("options-timezones-menuitem");
> +        menuitem.getNode().setAttribute("checked", data.timezone);
> +        dialog.click(menuitem);
> +    }

I think setting data.timezone to a timezone name should set the timezone. You can take my implementation from testTimezones.js for this. Also, having the timezone not displayed doesn't make it local.

@@ +388,5 @@
> +
> +/**
> + * select the n-th item in the reminder menulist.
> + * since the alarm-menulist does not have values, we can't use menulistSelect here.
> + * Custom reminders not supported

This comment is out of date.

@@ +415,5 @@
> + */
> +function setCategories(dialog, iframe, categories) {
> +    let { itemEditLookup: iframelookup, eid: iframeid } = helpersForController(iframe);
> +    let categoryMenulist = iframeid("item-categories");
> +    let categoryList = iframelookup(`${CATEGORY_LIST}`);

iframelookup(CATEGORY_LIST)

@@ +418,5 @@
> +    let categoryMenulist = iframeid("item-categories");
> +    let categoryList = iframelookup(`${CATEGORY_LIST}`);
> +    dialog.click(categoryMenulist);
> +    dialog.waitFor(() => categoryMenulist.getNode().open);
> +    if (categoryMenulist.itemCount > -1 && categoryMenulist.itemCount < data.categories.length) {

getNode() required here?

@@ +428,5 @@
> +            let set = false;
> +            for (let category of categories) {
> +                if (item.label == category) {
> +                    set = true;
> +                    break;

You can check categories.includes(item.label) here. Also, some of listItems (the first two) won't be menuitems.

::: calendar/test/mozmill/views/testMonthView.js
@@ +3,3 @@
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
>  var RELATIVE_ROOT = "../shared-modules";

You forgot MODULE_NAME here and in other files in this folder.
Attachment #9013738 - Flags: review?(geoff) → review+
OK, I found it. I introduced an until-date for the daily recurrence to cope with infrequent failures due to modal dialogs not opening in time on debug builds.
The idea was to improve performance by limiting the number of occurrences. Since we jump back to 2009, this leads to the events being too far back to trigger alarms.

I see if I can convert the test to use recurrences always starting "yesterday" without too much changes. Else we have to live with the failures on debug builds for a while and I will look into this later.
Yes, ideally the lower half of that test should be elsewhere anyway, and then it doesn't matter what year or month we make the event. Actually the alarm dialog test should be elsewhere too, but let's not get ahead of ourselves. :)
I also already thought about relocating this part - will create another bug for that in the future.
(In reply to Geoff Lankow (:darktrojan) from comment #11)
> Review of attachment 9013738 [details] [diff] [review]:
> -----------------------------------------------------------------

> Make sure you run ESLint on this too.
ESLint runs without errors on this version already.

> ::: calendar/test/mozmill/cal-recurrence/testWeeklyNRecurrence.js
> @@ +112,4 @@
> >  }
> >  
> >  function setRecurrence(recurrence) {
> > +    let { sleep: recsleep, lookup: reclookup, eid: recid } = helpersForController(recurrence);
> 
> This should go with the other stuff in test-item-editing-helpers.js.
Since the setRecurrence functions are unique in the different recurrence tests, I would have to create a general function for dealing with the recurrence dialog.
Since this has a lot of options and thus this would mean a lot of work, I like to keep this for a seperate bug. -> Bug 1496605

> @@ +116,3 @@
> >          anon({"anonid":"daybox"})/[0]/anon({"anonid":"boxstack"})/
> >          anon({"anonid":"topbox"})/{"flex":"1"}/{"flex":"1"}/[eventIndex]
> >      `;
> 
> Can this be eliminated? Might need to modify getEventBoxPath to do so.
> (And again below.)
I think yes. getEventBoxPath should be able to cope with multiple event boxes on one day.
Nevertheless this needs some research how the views handle this exactly and I see this in a sperate bug. -> Bug 1496604


> ::: calendar/test/mozmill/eventDialog/testEventDialog.js
> @@ +89,1 @@
> >          `);
> 
> I think this string and its 3 friends could be stored too. Seems like
> something we'd use a lot.
I created helpersForEditUI to deal with this. Also put the iframeLookup in there.

> 
> @@ +165,3 @@
> >  
> >      // 31st of January is Saturday so there's four more full rows to check
> > +    date = 4;
> 
> date will already be 4.
Just thought this would make the code more clear. But I will take it out for the final patch.

> @@ +89,5 @@
> > +        try {
> > +            wait_for_modal_dialog("commonDialog", TIMEOUT_COMMON_DIALOG);
> > +        } catch (e) {
> > +            failPoints.first = "";
> > +        }
> 
> This feels wrong. Can you put mark_failure inside the try block and get rid
> of failPoints?
I agree, this is a dirty hack, but we can't use mark_failure earlier, because then the test would stop. Since we test for different cases here, we want the other tests to proceed. Nevertheless I wanted to have a failure message that tells us at what point the test fails.
Since I can not pass an argument to the callback for the Modal Dialog, I can only do something in the catch()-Block. Since the catch in this case means the test passed, I can only start with a full failure message and in the test delete the passed parts.
A solution would be to rewrite the test using seperate testXXX-Functions. But that would be stuff for a different bug.

> @@ +195,5 @@
> >              freebusy: "busy",
> >              timezone: true,
> > +            attachment: { add: "http://mozilla.org" },
> > +            // test fails when changing attendees, therefore leaving out for now
> > +            // attendees: { add: "foo@bar.de,foo@bar.com" }
> 
> Has this been fixed?
Unfortunately not. I believe this fails, because we don't use the context menu to delete the Attendees and thus do not update the dialog properly. But I could not get that context-menu working.

> ::: calendar/test/mozmill/shared-modules/test-item-editing-helpers.js
> @@ +219,5 @@
> > +    if (data.timezone != undefined) {
> > +        let menuitem = eid("options-timezones-menuitem");
> > +        menuitem.getNode().setAttribute("checked", data.timezone);
> > +        dialog.click(menuitem);
> > +    }
> 
> I think setting data.timezone to a timezone name should set the timezone.
> You can take my implementation from testTimezones.js for this. Also, having
> the timezone not displayed doesn't make it local.
the value name "data.timezone" is deceiving here. It should better be named timezonedisplay, since it just switches the display of the timezone on or off. Also there was a bug in this code. Changed both.
Setting the timezone was done in the timezone-tests themselfes. But it is worth another bug to move this to setData. -> Bug 1496602

> @@ +418,5 @@
> > +    let categoryMenulist = iframeid("item-categories");
> > +    let categoryList = iframelookup(`${CATEGORY_LIST}`);
> > +    dialog.click(categoryMenulist);
> > +    dialog.waitFor(() => categoryMenulist.getNode().open);
> > +    if (categoryMenulist.itemCount > -1 && categoryMenulist.itemCount < data.categories.length) {
> 
> getNode() required here?
For some reason yes. This menulist gave me a hard time I can tell you.
 
> @@ +428,5 @@
> > +            let set = false;
> > +            for (let category of categories) {
> > +                if (item.label == category) {
> > +                    set = true;
> > +                    break;
> 
> You can check categories.includes(item.label) here. Also, some of listItems (the first two) won't be menuitems.
listItems = categoryList.getNode().childNodes; seems to only return listItems, omitting the first does not work.


I will post the patch as soon as i have a good try-run.
Posted patch setData.diff V3 (obsolete) — Splinter Review
Found some more optimizations, rest as per previous comment.
Comments are all correct sentences now.

Debug builds are failing at the moment on try, but the rest looks good:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=58967297bae7dde7ff65ee196bf39fa3e9c42145
Attachment #9013738 - Attachment is obsolete: true
Attachment #9014713 - Flags: review?(geoff)
(In reply to Markus Adrario [:Taraman] from comment #15)
> (In reply to Geoff Lankow (:darktrojan) from comment #11)
> > @@ +418,5 @@
> > > +    let categoryMenulist = iframeid("item-categories");
> > > +    let categoryList = iframelookup(`${CATEGORY_LIST}`);
> > > +    dialog.click(categoryMenulist);
> > > +    dialog.waitFor(() => categoryMenulist.getNode().open);
> > > +    if (categoryMenulist.itemCount > -1 && categoryMenulist.itemCount < data.categories.length) {
> > 
> > getNode() required here?
> For some reason yes. This menulist gave me a hard time I can tell you.

Sorry, that was ambiguous. In the last line, I think it should be categoryMenuList.getNode().itemCount.
Comment on attachment 9014713 [details] [diff] [review]
setData.diff V3

Review of attachment 9014713 [details] [diff] [review]:
-----------------------------------------------------------------

Nice work. Thanks!

::: calendar/test/mozmill/cal-recurrence/testLastDayOfMonthRecurrence.js
@@ +104,4 @@
>          );
>      }
>  
> +    // D.

Oops? :-)

::: calendar/test/mozmill/eventDialog/testEventDialogModificationPrompt.js
@@ +67,5 @@
> +        // Pref can not be retrieved earlier, because it is not in mozmillprofile
> +        // and will be created only if the first event is created.
> +        let categories = cal.calGetString("categories", "categories2").split(",");
> +        data[0].categories.push(categories[0]);
> +        data[1].categories.push(categories[1], categories[2]);

This comment doesn't apply any more, and now you can put this code with the data definition lower down.
Attachment #9014713 - Flags: review?(geoff) → review+
Pushed by Mozilla@Adrario.de:
https://hg.mozilla.org/comm-central/rev/20ab2d2a45b1
[mozmill] fix and enhance shared-module calendar-utils "setData()"-Function. r=darktrojan
Status: ASSIGNED → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
With the changes from comment #18 pushed to comm-central.

try-build with final patch:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=a649f4525f25e1fb76e103f4738e36529dfeaed7
Not sure why you do a try run after you pushed this to C-C. In any case, I'll back it out since we now have two perma-red test failures on Mac, which BTW, were already visible in the try run in comment #16 :-(
Backout:
https://hg.mozilla.org/comm-central/rev/78ca133d04d61a08ddc78412ca75037011dcb03b
Status: RESOLVED → REOPENED
Flags: needinfo?(Mozilla)
Resolution: FIXED → ---
TEST-UNEXPECTED-FAIL | /Users/cltbld/tasks/task_1539452524/build/tests/mozmill/eventDialog/testEventDialogSize.js | testEventDialogSize.js::testEventDialog
TEST-UNEXPECTED-FAIL | /Users/cltbld/tasks/task_1539452524/build/tests/mozmill/eventDialog/testEventDialogSize.js | testEventDialogSize.js::testTaskDialog
TEST-UNEXPECTED-FAIL | /Users/cltbld/tasks/task_1539452524/build/tests/mozmill/eventDialog/testUTF8.js | testUTF8.js::testUTF8

testEventDialogSize.js is perma-red on Mac.
The failure of testEventDialogSize is well known to me.
It was activated on comm-centralon Thu, 27 Sep 2018 [1] with bug 1480338
I also commented on that. [2]

So I got the impression this is a known failure and geoff is working on it. I didn't check the comm-central builds for that however, else I would have spotted that the test passes there.

The test does not use any of the code I changed and I did not change any logic in that test, so I don't have an idea how my patch causes this test to fail - but I'll look into it.

The try run was started and finished before the push. I wanted to post it with the push message, since last time  I pushed a patch the automatic comment from pulsebot was not implemented.
So I put the comment in afterwards.

Hope that explains a little, how that happened. Sorry for the hassle.


[1]: https://hg.mozilla.org/comm-central/rev/e6adb6f71893
[2]: https://bugzilla.mozilla.org/show_bug.cgi?id=1480338#c33
Flags: needinfo?(Mozilla)
It's less of a hassle than a disappointment to the developer to see their stuff backed out.

In general, unlike in the olden days, I try to keep opt runs completely green, apart from those pesky intermittent failures which are recorded here: https://mzl.la/2gS72WO. That's necessary since we have a few people working now and they all want to know whether their stuff caused failures or not. That's also why the backout policies are stricter now.

The Mac failure is indeed weird, but it's perma-red everywhere you look and the log is also quite clear:
https://taskcluster-artifacts.net/WPy37ERrSaavPacvmoWpvQ/0/public/logs/live_backing.log

INFO -  SUMMARY-UNEXPECTED-FAIL | testEventDialogSize.js | testEventDialogSize.js::testEventDialog
INFO -    EXCEPTION: waitFor: Timeout exceeded for '() => {
INFO -          return (iframeNode.clientWidth + 1 >= scrollWidth) &&
INFO -              (iframeNode.clientHeight + 1 >= scrollHeight);
INFO -      }'
INFO -      at: utils.js line 396
INFO -         TimeoutError utils.js:396 13
INFO -         waitFor utils.js:452 11
INFO -         MozMillController.prototype.waitFor controller.js:687 3
INFO -         checkLargeEnough testEventDialogSize.js:139 5
INFO -         testEventDialog/< testEventDialogSize.js:60 9
INFO -         invokeEventDialog test-calendar-utils.js:340 5
INFO -         testEventDialog testEventDialogSize.js:49 5
The test passes on try with my patch, if I don't run the other tests in the directory.
So it seems, that something in another test running before it changes the size of the dialog.

I investigate further...
Running locally I can't see where the window size changes in the previously running tests.
Anyhow the size test is quite flaky - it fails on both of my testing VMs, so I can't even find out if any changes I make in the other tests will cause it to pass.

So my suggestion ist to put this test in a seperate folder and run it alone.
This patch moves the eventDialogSize Test to an own directory, where it runs fine:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&selectedJob=207218337&revision=449b7b64bda58ffdb2afc75f97cc1a2bcca83e5f

I know this is not the most favourable solution since the test belongs to the event-dialog, but I have no idea how to fix this otherwise.
Another possibility would be to try to give it a name which causes it to run first on all platforms (Is it correct that files in a directory are run sorted by filename?)
Attachment #9014713 - Attachment is obsolete: true
Attachment #9019316 - Flags: review?(geoff)
Apologies for the delay. I'm doing some further investigating as I'm convinced what we landed in this bug isn't at fault. This work is being hindered by lots of bustage.
I guess the "problem" is that with the patch some more fields are set in "testEventDialog" and one of them changes the size of the dialog.

Try deactivating TimeZoneDisplay, Attendees, attachments or Description.
Timezone display being enabled does explain why the window is 16px wider with the patch than without, which did confuse me for a long time. (BTW, the timezone display can't be disabled, because false == undefined.)

It doesn't explain why the window's being resized to 4px too short in that one place, which is what's failing the test. My code must be buggy but I don't know why. I think I'll just increase the tolerance on the test and forget about it. The last 4 rows of pixels are all padding anyway.
Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/a0437b33ea6c
[mozmill] fix and enhance shared-module calendar-utils "setData()"-Function. r=darktrojan
Status: REOPENED → RESOLVED
Closed: 10 months ago10 months ago
Resolution: --- → FIXED
Target Milestone: --- → 6.6
Comment on attachment 9019316 [details] [diff] [review]
setData.diff V3.1

I've pushed with my changes to the test instead. Both ways are a bit ugly but it isn't worth spending any more time on.
Attachment #9019316 - Flags: review?(geoff)
Target Milestone: 6.6 → 6.7
You need to log in before you can comment on or make changes to this bug.