1215476 - [Test] task_manager_icon_test Marionette test failing locally

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::System, defect)

Description

[Alberto Pastor [:albertopq]]

Despite gaia-try doesn't seem to show it, running locally:

make test-integration TEST_FILES=apps/system/test/marionette/task_manager_icons_test.js

results in a test failing.

 1) Task Manager - Icons Firefox Apps use icon from app manifest:

  AssertionError: false == true
      at Context.<anonymous> (/Users/alberto/Projects/gaia/apps/system/test/marionette/task_manager_icons_test.js:102:7)

Comment 1

[GitHub Autolander Bot]

Attachment: [gaia] albertopq:1215476-task-manager-test > mozilla-b2g:master

Comment 2

[Alberto Pastor [:albertopq]]

Comment on attachment 8674822 [details] [review]
[gaia] albertopq:1215476-task-manager-test > mozilla-b2g:master

Hey Marcus, if I'm not wrong, the assertion in this test is incorrect. Given

https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/card.js#L59

the icon size taken will be 64 and not 32 as the test expects.
Could you please take a look? Thanks!

Comment 3

[Alberto Pastor [:albertopq]]

master: https://github.com/mozilla-b2g/gaia/commit/b86bffe236928abe156397d3582ae00d4699ac2e

Comment 4

[Michael Henretty [:mikehenrty][:mhenretty]]

reverted for causing bustage on b2g-inbound [1]:

https://github.com/mozilla-b2g/gaia/commit/9cf6b6156cc74616ffcba8956d47982b4405345e


1.) https://treeherder.mozilla.org/logviewer.html#?job_id=3076632&repo=b2g-inbound

Comment 5

[Alberto Pastor [:albertopq]]

Does this test pass locally for you, Michael? I believe this doesn't fail in gaia-try? Any idea why this would fail in b2g-inbound?

Comment 6

[Michael Henretty [:mikehenrty][:mhenretty]]

(In reply to Alberto Pastor [:albertopq] from comment #5)
> Does this test pass locally for you, Michael? I believe this doesn't fail in
> gaia-try? Any idea why this would fail in b2g-inbound?

I tried running task_manager_icons_test.js locally before I reverting, and it did indeed fail locally. When I reverted it passed locally. So I can at least reproduce It is intermittent (even on b2g-inbound), so my thinking is we should fix it locally, submit a PR, re-trigger a bunch of times in the PR, and then land and watch b2g-inbound.

Comment 7

[Alberto Pastor [:albertopq]]

Seems to be working now..

Comment 8

[Marcus Cavanaugh [:mcav]]

I think this is a devicePixelRatio-related issue; it passes on my Linux VM, fails on my Mac, with the same source tree. Taking.

Comment 9

[Marcus Cavanaugh [:mcav]]

I think this is a devicePixelRatio-related issue; it passes on my Linux VM, fails on my Mac, with the same source tree. Taking.

Comment 10

[GitHub Autolander Bot]

Attachment: [gaia] mcav:icon-size-test > mozilla-b2g:master

Comment 11

[Alberto Pastor [:albertopq]]

Comment on attachment 8676517 [details] [review]
[gaia] mcav:icon-size-test > mozilla-b2g:master

I think that code will only work if devicePixelRatio == 2. I think we need to find the closest size to 32 * devicePixelRatio in the expectedSizes list, instead.

Comment 12

[Marcus Cavanaugh [:mcav]]

Comment on attachment 8676517 [details] [review]
[gaia] mcav:icon-size-test > mozilla-b2g:master

You were right that the previous-patch devicePixelRatio test would only work for integer values. This updated patch just mocks devicePixelRatio to always equal 1, with the following rationale: The devicePixelRatio-dependent icon-selection algorithm is deep within IconsHelper, which has a bunch of unit tests already determining which icon will be selected based on dPR. I had attempted to duplicate the logic within task_manager_icons_test, but it seemed much more brittle compared to mocking dPR and letting IconsHelper be responsible for its own dPR-specific wishes.

Comment 13

[Alberto Pastor [:albertopq]]

Comment on attachment 8676517 [details] [review]
[gaia] mcav:icon-size-test > mozilla-b2g:master

Thanks!

Comment 14

[Marcus Cavanaugh [:mcav]]

master: https://github.com/mozilla-b2g/gaia/commit/352aab80c637ca5264301495872facd083b231fa