Add CI tests exercising notifications.

Add CI tests exercising notifications.

Ensure no initial errors are displayed.

https://code.launchpad.net/~frankban/juju-gui/notifications-ci-test/+merge/170036

(do not edit description out of merge proposal)

[frankban, 2013-06-18 10:27:41]

Please take a look.

[gary.poster, 2013-06-18 13:56:00]

LGTM with consideration of comments.

Thanks!

Gary

https://codereview.appspot.com/10383043/diff/1/test/test_charm_running.py
File test/test_charm_running.py (right):

https://codereview.appspot.com/10383043/diff/1/test/test_charm_running.py#new...
test/test_charm_running.py:156: def assert_deployed(self, service_name):
Nice change, though now you can't count on the service information being
available after calling deploy. not sure how important that is.

https://codereview.appspot.com/10383043/diff/1/test/test_charm_running.py#new...
test/test_charm_running.py:195: service = self.get_service_names().pop()
This is a little dangerous but fine in practice.  As you know, the service name
is arbitrary, and connected to the charm name only by default/convention.  Maybe
simply add a comment acknowledging this?

[frankban, 2013-06-18 14:35:44]

Please take a look.

https://codereview.appspot.com/10383043/diff/1/test/test_charm_running.py
File test/test_charm_running.py (right):

https://codereview.appspot.com/10383043/diff/1/test/test_charm_running.py#new...
test/test_charm_running.py:156: def assert_deployed(self, service_name):
On 2013/06/18 13:56:00, gary.poster wrote:
> Nice change, though now you can't count on the service information being
> available after calling deploy. not sure how important that is.

Added a comment.

https://codereview.appspot.com/10383043/diff/1/test/test_charm_running.py#new...
test/test_charm_running.py:195: service = self.get_service_names().pop()
On 2013/06/18 13:56:00, gary.poster wrote:
> This is a little dangerous but fine in practice.  As you know, the service
name
> is arbitrary, and connected to the charm name only by default/convention. 
Maybe
> simply add a comment acknowledging this?

Done.

[teknico, 2013-06-18 16:25:29]

LGTM, nice tests as usual (good monotony :-) ). A couple trivials.

https://codereview.appspot.com/10383043/diff/5001/test/test_charm_running.py
File test/test_charm_running.py (right):

https://codereview.appspot.com/10383043/diff/5001/test/test_charm_running.py#...
test/test_charm_running.py:115: This method only start the deployment process.
If waiting for the newly
s/start/starts/

https://codereview.appspot.com/10383043/diff/5001/test/test_charm_running.py#...
test/test_charm_running.py:123: # source.
Even if it's not changed in this branch, I'll comment anyway. :-)

"try that possible source" is rather cryptic: does it mean "source of problems"?
If so, maybe say: "you may want to investigate in that direction" instead.

[frankban, 2013-06-19 08:27:45]

*** Submitted:

Add CI tests exercising notifications.

Ensure no initial errors are displayed.

R=gary.poster, teknico
CC=
https://codereview.appspot.com/10383043

https://codereview.appspot.com/10383043/diff/5001/test/test_charm_running.py
File test/test_charm_running.py (right):

https://codereview.appspot.com/10383043/diff/5001/test/test_charm_running.py#...
test/test_charm_running.py:115: This method only start the deployment process.
If waiting for the newly
On 2013/06/18 16:25:29, teknico wrote:
> s/start/starts/

Done.

https://codereview.appspot.com/10383043/diff/5001/test/test_charm_running.py#...
test/test_charm_running.py:123: # source.
On 2013/06/18 16:25:29, teknico wrote:
> Even if it's not changed in this branch, I'll comment anyway. :-)
> 
> "try that possible source" is rather cryptic: does it mean "source of
problems"?
> If so, maybe say: "you may want to investigate in that direction" instead.

Done.

[frankban, 2013-06-19 08:32:28]

Thank you both for the reviews.