Jorg K (CEST = GMT+2) Reporter
	Description • 6 years ago

This newly enabled test fails intermittently:
TEST-UNEXPECTED-FAIL | /Users/cltbld/tasks/task_1536922515/build/tests/mozmill/views/testTaskView.js | testTaskView.js::testTaskView

Some timing problem:
https://taskcluster-artifacts.net/DR-AotJmRgO-sygOWlTx6w/0/public/logs/live_backing.log

INFO -  SUMMARY-UNEXPECTED-FAIL | testTaskView.js | testTaskView.js::testTaskView
INFO -    EXCEPTION: waitFor: Timeout exceeded for '() => {
INFO -          countAfterDelete = taskTreeNode.mTaskArray.length;
INFO -          return countAfter - 1 == countAfterDelete;
INFO -      }'
INFO -      at: utils.js line 396
INFO -         TimeoutError utils.js:396 13
INFO -         waitFor utils.js:434 11
INFO -         MozMillController.prototype.waitFor controller.js:687 3
INFO -         testTaskView testTaskView.js:153 5

	Geoff Lankow (:darktrojan) Assignee
	Comment 1 • 6 years ago

Going to run this on Try a bunch of times, and if it passes, get review. I'm not positive this will actually fix things.

	Geoff Lankow (:darktrojan) Assignee
	Comment 2 • 6 years ago

Well, that didn't help.

	Geoff Lankow (:darktrojan) Assignee
	Comment 3 • 6 years ago

Unassigning myself in case somebody else wants to have a go.

	Magnus Melin [:mkmelin]
	Comment 4 • 6 years ago

The potential double delete looks goofy. Trying something now: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=c8693bbe868b6ab639b4a2caad9438dc56bcb7ec

	Magnus Melin [:mkmelin]
	Comment 5 • 6 years ago

Try looks green, and wasn't without it. So let's do this. Not 100% sure if it's just checking sometimes wrong numbers due to the double delete, or what. 

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

	Geoff Lankow (:darktrojan) Assignee
	Comment 6 • 6 years ago

Comment on attachment 9026368 [details] [diff] [review]
bug1491269_testTaskView.patch

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

I never understood what that was all about. Let's try this and see what happens.

	Jorg K (CEST = GMT+2) Reporter
	Comment 7 • 6 years ago

Geoff, if you check this in, let's do a better commit message, like:
Bug 1491269 - Fix test failure in testTaskView.js by avoiding double delete. r=darktrojan

I can't see you on IRC, hence NI.

	Pulsebot
	Comment 8 • 6 years ago

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/73d35ac1dec4
Fix test failure in testTaskView.js by avoiding double delete; r=darktrojan

	Jorg K (CEST = GMT+2) Reporter
	Comment 9 • 5 years ago

Failed again:
https://treeherder.mozilla.org/#/jobs?repo=comm-central&selectedJob=215335934&revision=7cdd9ac6221e517b841c4cb786f05bad32576e0c

	Jorg K (CEST = GMT+2) Reporter
	Comment 10 • 5 years ago

INFO -  SUMMARY-UNEXPECTED-FAIL | testTaskView.js | testTaskView.js::testTaskView
INFO -    EXCEPTION: Task did not delete; countBefore=1, countAfter=1
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 -         testTaskView testTaskView.js:160 5

That's around the code that was changed.

	Magnus Melin [:mkmelin]
	Comment 11 • 5 years ago

Seems like an oversight not to remember the previous count *before* clicking

	Geoff Lankow (:darktrojan) Assignee
	Comment 12 • 5 years ago

I don't think that change will make any difference, but it does read better. Why not make countBefore = taskTreeNode.mTaskArray.length instead of relying on a value from further up the code?

	Magnus Melin [:mkmelin]
	Comment 13 • 5 years ago

The difference it could make is that if the delete is really fast then the before value would be the after value -> of course no changes after that.

	Pulsebot
	Comment 14 • 5 years ago

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/0e2cd56c3a6c
Follow-up: fix testTaskView.js failure by remembering the number of tasks before clicking delete; r=darktrojan

	Jorg K (CEST = GMT+2) Reporter
	Comment 15 • 5 years ago

Still failing:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&selectedJob=215893431&revision=5b8c49b90a556e79ff6cdf0eaf929646c643adbd

EXCEPTION: Task did not delete; countBefore=1, countAfter=1

	Jorg K (CEST = GMT+2) Reporter
	Comment 16 • 5 years ago

And here:
https://treeherder.mozilla.org/#/jobs?repo=comm-central&revision=21219ad63ec84a637511fb8b76ddf34a874d51ca&selectedJob=216036124

	Pulsebot
	Comment 18 • 5 years ago

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/0ecdef42a8cb
disable failing test testTaskView.js. rs=bustage-fix

	Philipp Kewisch [:Fallen] [:📆][:🧩]
	Comment 21 • 5 years ago

Comment on attachment 9036838 [details] [diff] [review]
1491269-task-view-test-1.diff

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

Thanks for the fix!

	Pulsebot
	Comment 22 • 5 years ago

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/1c626f1c70ef
Fix failing testTaskView.js; r=Fallen