1352148 - TEST-UNEXPECTED-FAIL | xpcshell-libical.ini:calendar/test/unit/test_alarmservice.js

Status: RESOLVED FIXED

(Calendar :: General, defect)

Description

[Jorg K (CEST = GMT+2)]

TEST-UNEXPECTED-FAIL | xpcshell-libical.ini:calendar/test/unit/test_alarmservice.js | xpcshell return code: 0

TEST-UNEXPECTED-FAIL | xpcshell-libical.ini:calendar/test/unit/test_alarmservice.js | test_addItems - [test_addItems : 12] addTestItems Test 7; check fired - false == true

First seen:
https://treeherder.mozilla.org/#/jobs?repo=comm-central&revision=9b821ab693d55ac96b1e6e544fc26a6345c6c600&selectedJob=87533384

Daily build of 2017-03-30.

Since nothing was landed on C-C this must have come from some M-C change:

Last good:
M-C: 60d7a0496a3673450ddbc37ec387525148
First bad:
M-C: 60d7a0496a3673450ddbc37ec387525148

Hey, that's the same M-C change set, C-C also unchanged, so where do these failures suddenly come from?

Weird. Markus, can you shed some light onto this?

Comment 1

[Markus Adrario [:Taraman]]

Next week I'm away, so it will take a while...

Comment 2

[Decathlon]

The test that fails is the same that caused the bug on the first day of the month (bug 1244818) and again the cause seems the same.
The test creates a monthly recurring event 2 months and 1 day before the 30th of March i.e. the 29th of January, so the sequence of occurrences skip February (no leap year this year) and the second occurrence is the 29th of March.
The expected sequence of status for the alarms, for all the days different than the first, wants two occurrences without alarm before the occurrence with alarm so it's wrong in this case:
https://dxr.mozilla.org/comm-centaral/source/calendar/test/unit/test_alarmservice.js#243

Comment 3

[Jorg K (CEST = GMT+2)]

Funny link you posted there: comm-centaral ;-)

Many thanks for looking at it, so this will go away by itself.

Comment 4

[Decathlon]

Funny, many problems with my machine these days (and connection too), but a Ctrl+C + Ctrl+V not working properly seems too much.
https://dxr.mozilla.org/comm-central/source/calendar/test/unit/test_alarmservice.js#243

Comment 5

[Jorg K (CEST = GMT+2)]

(In reply to Jorg K (GMT+1) from comment #3)
> Many thanks for looking at it, so this will go away by itself.
But when? On April 2nd? Remember there are failures at the 1st of the month.

Comment 6

[Decathlon]

The failures at the first day of the month should be fixed with bug 1261688. It remains bug 1244818 only for ical.js but there's already a patch waiting for review for that.

Comment 7

[Jorg K (CEST = GMT+2)]

Failures have gone away.

Comment 8

[Decathlon]

Attachment: patch-v1

As far as I can see this bug occurs only on the 30th and 31st of March and on 30th of April and is caused, as mentioned in comment 2, by the missing days of February compared to the others months.
This patch takes into account these particular cases along with the first day of the month already fixed in bug 1261688.

Comment 9

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

Comment on attachment 8854177 [details] [diff] [review]
patch-v1

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

Code looks fine to me, r=philipp I guess we can't really do a try run here because we'd have to run it on one of the affected days? I guess we could do a try run now to make sure it doesn't break the current tests, and hope for the best on the mentioned days. If you haven't already, can you start one?

Comment 10

[Markus Adrario [:Taraman]]

One could test it locally by setting his system date to one of the failing dates 
I'm on vacation and don't have my VMs at hand,  so unfortunately can't do it for all platforms.

Comment 11

[Decathlon]

(In reply to Philipp Kewisch [:Fallen] from comment #9)

> Code looks fine to me, r=philipp I guess we can't really do a try run here
> because we'd have to run it on one of the affected days? I guess we could do
> a try run now to make sure it doesn't break the current tests, and hope for
> the best on the mentioned days. If you haven't already, can you start one?

Already done:

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=53bd7cadfa41cff91f8a589676b7d43d9599b714

the test "test_alarmservice" passes.
We can wait for the 30th of April to see if there is the failure and see if a run on the try server fixes and in the meantime I can do a run every now and again.

(In reply to Markus Adrario [:Taraman] from comment #10)
> One could test it locally by setting his system date to one of the failing
> dates 

Yes, this is the way I've tested, though strange things happen to the machine when you move the system date too far in the past or in the future while testing the dates.

Comment 12

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

(In reply to Decathlon from comment #11)
> We can wait for the 30th of April to see if there is the failure and see if
> a run on the try server fixes and in the meantime I can do a run every now
> and again.
I think we should either land now and wait for the next failure, being ready to do additional patches on those days, or not land until those days and do a try run. Personally I'd go with the first option. I'll leave the decision to Jörg since he is managing the tree.

Comment 13

[Jorg K (CEST = GMT+2)]

Let's land it now and trust the author and reviewer ;-) After all, that test failure is not dramatic in any way since it shows a problem with the test and not a functional problem in our software.

Let me land this when I next need a patch for pushing since I do two pushes a day right after the M-C merges around midday and midnight European time.

Comment 14

[Jorg K (CEST = GMT+2)]

https://hg.mozilla.org/comm-central/rev/b08dc613a3d275711f76b42cf0ceba1119d82f02

Thanks for the quick fix and review!

Comment 15

[Decathlon]

OK, I've just verified that, without the patch, the test fails on the 30th of April:

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=1a704770661a642a50a4dd2dad452e8570f1bae5

and there is no failure with the patch:

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=199b71120c145185900ff8f5c294ad985e23e08e