961137 - [Search] Places marionette test disabled

Status: RESOLVED FIXED

(addons.mozilla.org Graveyard :: Search, defect)

Description

[Kevin Grandon :kgrandon]

Intermittent on travis, going to disable the test for now.

Comment 1

[Kevin Grandon :kgrandon]

Disabled in: https://github.com/mozilla-b2g/gaia/commit/75601b26d52a1835a334352628f2c0d8fe6c76be

Comment 2

[Dale Harvey (:daleharvey)]

To be honest was a bit lazy of me, but the change notification is sent before the change is guaranteed to have been written, its either that or the 'press home in between searching' quite stable enough, will get this passing 100%, apologies.

Comment 3

[Kevin Grandon :kgrandon]

Disbaled other test, but lost the log in my clipboard, sorry =/

https://github.com/mozilla-b2g/gaia/commit/df3b929f3e979e5e7eef5d8394a4b800c5c94ed8

Comment 4

[Dale Harvey (:daleharvey)]

> change notification is sent before the change is guaranteed to have been written 

Is fixed by https://github.com/mozilla-b2g/gaia/commit/a1e985d3a045daa796812712dfb81486becc5a9b

I also started using a new approach to tests with the favicon thing, I now repeatedly search until we get the results I want, we had problems with things not being completed (icon fetched, place saved, store synced), this approach solved that, so I think we should be able to get these 100% now

Comment 5

[Dale Harvey (:daleharvey)]

I am gonna split this into 2 bugs, I think the original tests are fixed, looking green to me but going to do a 100 run before I do a PR

The favicon bug and issues Kevin has been seeing are a fairly serious issue with our places implementation

Comment 6

[Dale Harvey (:daleharvey)]

Attachment: https://github.com/mozilla-b2g/gaia/pull/15601

As mentioned the fix for this was in https://github.com/mozilla-b2g/gaia/commit/a1e985d3a045daa796812712dfb81486becc5a9b

Its going on 50 green runs between https://travis-ci.org/daleharvey/gaia/builds/17406566 and https://travis-ci.org/daleharvey/gaia/builds/17408429

Comment 7

[Dale Harvey (:daleharvey)]

The problem with the favicon bug is filed @ https://bugzilla.mozilla.org/show_bug.cgi?id=962554

Comment 8

[Kevin Grandon :kgrandon]

Interesting. Test run looks good, but there was one failure.

 1) Places tests Search for previously visited URL:

+ expected - actual

+http://localhost:51835/sample.html

-Sample page


I suppose what happens is that we are storing the place for the URL, before the page loads? If that's the case, it's probably not a problem with the test - but perhaps we are storing more than we want to? Let me know what you think..

Comment 9

[Dale Harvey (:daleharvey)]

It could possibly be racey due to https://bugzilla.mozilla.org/show_bug.cgi?id=962554, another test run with both fixes going now https://travis-ci.org/daleharvey/gaia/builds/17447160

Comment 10

[Kevin Grandon :kgrandon]

Comment on attachment 8363645 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/15601

Two failures on that last run, so going to clear the review flag for now. I have some ideas of what might be happening, and will try to help out once I get the rocketbar stuff into master.

Comment 11

[Dale Harvey (:daleharvey)]

Attachment: https://github.com/daleharvey/gaia/commit/d003522d5c659417881183c62a5424adad37df2c

Found the issue, code assumed the place had been written before attempting to set icons / titles, its related to https://bugzilla.mozilla.org/show_bug.cgi?id=962554

the 100 runs had one failure, https://travis-ci.org/daleharvey/gaia/builds/17477958 but it was a crash unrelated to this patch

Comment 12

[Kevin Grandon :kgrandon]

Comment on attachment 8364883 [details]
https://github.com/daleharvey/gaia/commit/d003522d5c659417881183c62a5424adad37df2c

I am a little bit worried about the failure I saw in travis, but doesn't appear to be caused by this patch. I'd say we should land this and we can investigate the other test if we see it fail again.


1) navigation "before each" hook:

Error: timeout of 20000ms exceeded

at null.<anonymous> (/home/travis/build/daleharvey/gaia/node_modules/mocha/lib/runnable.js:165:14)

at Timer.listOnTimeout [as ontimeout] (timers.js:110:15)

2) navigation "after each" hook:

Error: Not connected. To write data you must call connect first.

Comment 13

[Dale Harvey (:daleharvey)]

Yeh no failures are ok, but given the 600 or so runs I have done, having one ghost seems likely, the changes in the test are needed so at the least they can land and can redisable the tests if theres a problem, it doesnt look like there should be though

https://github.com/mozilla-b2g/gaia/commit/fa90ddf34db8057f9d95b23864902f16e049edd1