959901 - [Calendar][Integration Test] "day_view_test.js" is permared

Status: RESOLVED FIXED

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

Description

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

1) day view header copy should not overflow:

AssertionError: content is bigger than container

at Context.<anonymous> (/home/travis/build/mozilla-b2g/gaia/apps/calendar/test/marionette/day_view_test.js:27:12)

at Test.Runnable.run (/home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runnable.js:211:32)

at Runner.runTest (/home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:372:10)

at /home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:448:12

at next (/home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:297:14)

at /home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:307:7

at next (/home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:245:23)

at /home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:269:7

at Hook.Runnable.run (/home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runnable.js:213:5)

at next (/home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:257:10)

at /home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:269:7

at done (/home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runnable.js:185:5)

at /home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runnable.js:197:9

at Object.executeHook (/home/travis/build/mozilla-b2g/gaia/node_modules/marionette-client/lib/marionette/client.js:370:18)

at process._tickCallback (node.js:415:13)

Comment 1

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

backed out: https://github.com/mozilla-b2g/gaia/commit/424a004947754238571efcabf153b52b7d826717

Comment 2

[Miller Medeiros [:millermedeiros]]

I'm running some tests on [this pull request](https://github.com/mozilla-b2g/gaia/pull/15335) to try to find out why it fails and if there is an easy fix.

after I replaced assertion with chai it shows up that content is 1px wider than container. I'm going to run tests a few more times to confirm behavior, and maybe I will simply replace the assertion and ignore 1-2px differences to account for rounding issues.

Comment 3

[Miller Medeiros [:millermedeiros]]

I ran the travis build 60+ times and could not figure out what is causing the issue; I was also not able to reproduce it locally (even after a `make really-clean`). The strangest thing is that any build until https://travis-ci.org/mozilla-b2g/gaia/jobs/16966928 was working just fine and after https://travis-ci.org/mozilla-b2g/gaia/jobs/16968003 started failing - no significant changes on calendar source code that could affect this test.

I spent some time comparing the logs and the build that worked installed http://ftp.mozilla.org/pub/mozilla.org/b2g/tinderbox-builds/mozilla-central-linux64_gecko/1389741932/b2g-29.0a1.multi.linux-x86_64.tar.bz2  and the one that failed used  http://ftp.mozilla.org/pub/mozilla.org/b2g/tinderbox-builds/mozilla-central-linux64_gecko/1389710917/b2g-29.0a1.multi.linux-x86_64.tar.bz2

I'm going to focus on another problems, it would be good to have someone from "platform" investigating the root cause, since it can be a Gecko problem. (don't know who to assign)

At every single time the difference was of 1px (consistently failing), so it could be some rounding issue on `element.scrollWidth`.

Comment 4

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

(In reply to Miller Medeiros [:millermedeiros] from comment #3)
> I ran the travis build 60+ times and could not figure out what is causing
> the issue; I was also not able to reproduce it locally (even after a `make
> really-clean`). The strangest thing is that any build until
> https://travis-ci.org/mozilla-b2g/gaia/jobs/16966928 was working just fine
> and after https://travis-ci.org/mozilla-b2g/gaia/jobs/16968003 started
> failing - no significant changes on calendar source code that could affect
> this test.
> 
> I spent some time comparing the logs and the build that worked installed
> http://ftp.mozilla.org/pub/mozilla.org/b2g/tinderbox-builds/mozilla-central-
> linux64_gecko/1389741932/b2g-29.0a1.multi.linux-x86_64.tar.bz2  and the one
> that failed used 
> http://ftp.mozilla.org/pub/mozilla.org/b2g/tinderbox-builds/mozilla-central-
> linux64_gecko/1389710917/b2g-29.0a1.multi.linux-x86_64.tar.bz2
> 
> I'm going to focus on another problems, it would be good to have someone
> from "platform" investigating the root cause, since it can be a Gecko
> problem. (don't know who to assign)
> 
> At every single time the difference was of 1px (consistently failing), so it
> could be some rounding issue on `element.scrollWidth`.

Sounds like a change to the platform caused this. Can we rewrite this test in another way to work around this rounding issue?

Comment 5

[Miller Medeiros [:millermedeiros]]

Attachment: Link to github pull request

changed the test logic to use `chai.assert.closeTo` but not 100% sure if we should keep a test that is theoretically hiding a platform bug (even tho it tests what it is supposed to test).

Comment 6

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

(In reply to Miller Medeiros [:millermedeiros] from comment #5)
> Created attachment 8360631 [details] [review]
> Link to github pull request
> 
> changed the test logic to use `chai.assert.closeTo` but not 100% sure if we
> should keep a test that is theoretically hiding a platform bug (even tho it
> tests what it is supposed to test).

I hear you. If it is indeed a platform issue, we should file a separate bug with minimum viable STR. It is unlikely they will look at a particular marionette-js test bug, especially given all the problems we have been having with these tests lately.

Comment 7

[Miller Medeiros [:millermedeiros]]

Comment on attachment 8360631 [details] [review]
Link to github pull request

I updated the pull request, abstracted the logic into a discrete method since this is going to be needed by other tests in the calendar.

I think it's good enough and I don't know how to create a minimum viable test case - since I can't reproduce it locally.

The proposed patch will allow me to make progress in another bug (that needs a similar test). So I think we should merge it.

Comment 8

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

Comment on attachment 8360631 [details] [review]
Link to github pull request

Gareth mentioned in IRC that he would review this, and he is a better man for the job, so....

Comment 9

[Gareth Aye [:gaye] (back from PTO)]

Comment on attachment 8360631 [details] [review]
Link to github pull request

A few nits on GH, but in general LGTM!

Comment 10

[Miller Medeiros [:millermedeiros]]

Comment on attachment 8360631 [details] [review]
Link to github pull request

Gareth, I updated the PR so now we have less code duplication - added a new method `isViewActive` to the marionette calendar and reused it "everywhere".

I think this is good to go.

Comment 11

[Miller Medeiros [:millermedeiros]]

Gareth, just do another quick review and let me know if I can merge (and/or if you will do it).

Comment 12

[Gareth Aye [:gaye] (back from PTO)]

https://github.com/mozilla-b2g/gaia/commit/72243d6ff711d1cb00403df3a7d76ebeb4d69776 we'll see if this fixes