1149024 - [v2.2] wait_for_notification_toaster_displayed needs 'message' as its argument

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::UI Tests, defect)

Description

[Hermes Cheng (inactive after July 27, 2015)]

When performed "test_sms_notification.py", it failed when calling system.wait_for_notification_toaster_displayed(...).

The failed reason is putting a incorrect parameter "message" to wait_for_notification_toaster_displayed(...).

It is only 

* Error:
TEST-UNEXPECTED-ERROR | test_sms_notification.py TestSmsNotification.test_sms_notification | TypeError: wait_for_notification_toaster_displayed() got an unexpected keyword argument 'message'

Traceback (most recent call last):
  File "/home/hermesc/workspace/test-space/gaia-2.2/gaia/2.2/local/lib/python2.7/site-packages/marionette_client-0.8.6-py2.7.egg/marionette/marionette_test.py", line 268, in run
    testMethod()
  File "/home/hermesc/workspace/test-space/gaia-2.2/gaia/tests/python/gaia-ui-tests/gaiatest/tests/functional/messages/test_sms_notification.py", line 26, in test_sms_notification
    message="Notification did not appear. SMS database dump: %s " % self.data_layer.get_all_sms())
TEST-INFO took 15861ms

Comment 1

[Hermes Cheng (inactive after July 27, 2015)]

Shing, you have a fixed for wait_for_notification_toaster_displayed(...) in the latest PR. Could you check it?

https://github.com/mozilla-b2g/gaia/blob/v2.2/tests/python/gaia-ui-tests/gaiatest/apps/system/app.py

Comment 2

[Hermes Cheng (inactive after July 27, 2015)]

Attachment: non-smoketest report.html

Please see the error in "test_sms_notification.TestSmsNotification"

Comment 3

[Shing Lyu [:shinglyu]]

Oh yes, the "message" parameter was removed during the review, because it was not passed down and the reviewer thought it was useless. I'll create another PR to add it back and maybe pass it down.

Comment 4

[GitHub Autolander Bot]

Attachment: [gaia] shinglyu:bug1149024 > mozilla-b2g:v2.2

Comment 5

[Shing Lyu [:shinglyu]]

Comment on attachment 8585836 [details] [review]
[gaia] shinglyu:bug1149024 > mozilla-b2g:v2.2

Hi Geo, th "message" argument was removed in Bug 1132479. Now other tests need it, so I added it back and passed along. Thanks.

Comment 6

[Hermes Cheng (inactive after July 27, 2015)]

Comment on attachment 8585836 [details] [review]
[gaia] shinglyu:bug1149024 > mozilla-b2g:v2.2

It looks good. Thanks!

Comment 7

[Martijn Wargers (dead)]

The pull request from bug 1132479 caused this bug, so marking blocking that bug.

Comment 8

[Martijn Wargers (dead)]

Comment on attachment 8585836 [details] [review]
[gaia] shinglyu:bug1149024 > mozilla-b2g:v2.2

I'm fine with the chang, it looks good and I assume it makes the test pass.

But I'm concerned this change with introduced in the first place. I understand the reasoning in comment 3, but then you should try to get that change back into master too.
I'd prefer to keep the most minimum difference between the tests between v2.2 and master, unless absolutely necessary.

Comment 9

[Shing Lyu [:shinglyu]]

Thanks Martjin, I'll send another pull request on master to make the two branches in sync.

Could you help me merge the pull request? I don't have the permission for merging. Thanks!

Comment 10

[Martijn Wargers (dead)]

Yes, I can do that. Unfortunately, the Treeherder Try server seems to be very unstable, the results are orange. It's probably not from this pull request, but afaik, I can only merge the pull request with a succesful Tryserver run.
I have the same issue in bug 1149886.

Comment 11

[Martijn Wargers (dead)]

Shing, something with Treeherder try and v2.2 is very wrong, where a retrigger doesn't give new Gip results. I talked with James Lal about this, but I don't really know what needs to be done to get the v2.2 pull request in. I suggest you chat with James Lal about this.
There are more v2.2 pull requests waiting on this v2.2 Treeherder try bustage to be fixed.

Comment 12

[Shing Lyu [:shinglyu]]

@Martijn: Thanks for your effort. I'll keep an eye on this until it landed.

Comment 13

[Martijn Wargers (dead)]

Shing, it looks like v2.2 Treeherder try is green now, so this can be checked in?

Comment 14

[Shing Lyu [:shinglyu]]

I've rebased the branch, rerunning the tests now. Thanks!

Comment 15

[Shing Lyu [:shinglyu]]

Looks like it's all green, can you help me check this in, Martijn? Thank you!

Comment 16

[Martijn Wargers (dead)]

Treeheder Try was green, checked into v2.2: https://github.com/mozilla-b2g/gaia/commit/a69f8eb5c3815289afac45d2a55d868547b17f00

Comment 17

[Martijn Wargers (dead)]

Shing, does the thing in comment 9 still need to happen (sync the code in v2.2 and master)?

Comment 18

[Shing Lyu [:shinglyu]]

(In reply to Martijn Wargers [:mwargers] (QA) from comment #17)
The master branch now uses "Wait()" directly, without going through the "self.wait_for_element_displayed()" helper. I'm not sure how we are going to make them in sync.


 38 <<<<<<< HEAD                                                                     
 39     # A lot of tests, like mail or call received, need a longer timeout here     
 40     def wait_for_notification_toaster_displayed(self, timeout=30, message=None): 
 41     ¦   Wait(self.marionette, timeout).until(                                    
 42     ¦   ¦   expected.element_displayed(*self._notification_toaster_locator), message=message)
 43 =======                                                                          
 44     def wait_for_notification_toaster_displayed(self, timeout=10, message=None): 
 45     ¦   self.wait_for_element_displayed(*self._notification_toaster_locator,     
 46     ¦   ¦   ¦   ¦   ¦   ¦   ¦   ¦   ¦   timeout=timeout, message=message)        
 47 >>>>>>> bd81b49... Bug 1149024 - wait_for_notification_toaster_displayed needs 'message' as its argument

Comment 19

[Martijn Wargers (dead)]

Ok, just leave it as it is then, thanks.