Merge lp://staging/~gary/juju-gui/fix-charm-panel-tests into lp://staging/juju-gui/experimental

Proposed by Gary Poster
Status: Merged
Merged at revision: 735
Proposed branch: lp://staging/~gary/juju-gui/fix-charm-panel-tests
Merge into: lp://staging/juju-gui/experimental
Diff against target: 123 lines (+51/-19)
5 files modified
app/subapps/browser/templates/browser_charm.handlebars (+3/-0)
app/subapps/browser/templates/sidebar.handlebars (+3/-0)
app/templates/browser-search.handlebars (+3/-0)
app/templates/charm-token.handlebars (+3/-0)
test/test_charm_running.py (+39/-19)
To merge this branch: bzr merge lp://staging/~gary/juju-gui/fix-charm-panel-tests
Reviewer Review Type Date Requested Status
Juju GUI Hackers Pending
Review via email: mp+169502@code.staging.launchpad.net

Description of the change

To post a comment you must log in.
Revision history for this message
Gary Poster (gary) wrote :
Download full text (6.0 KiB)

Reviewers: mp+169502_code.launchpad.net,

Message:
Please take a look.

Description:
Fix CI tests

I just added some small fixes to the good work primarily done by Jeff.

https://code.launchpad.net/~gary/juju-gui/fix-charm-panel-tests/+merge/169502

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/10253055/

Affected files:
   A [revision details]
   M app/subapps/browser/templates/browser_charm.handlebars
   M app/subapps/browser/templates/sidebar.handlebars
   M app/templates/charm-token.handlebars
   M test/test_charm_running.py

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision: <email address hidden>

Index: test/test_charm_running.py
=== modified file 'test/test_charm_running.py'
--- test/test_charm_running.py 2013-06-12 14:14:36 +0000
+++ test/test_charm_running.py 2013-06-14 17:13:29 +0000
@@ -111,27 +111,47 @@

      def deploy(self, charm_name):
          """Deploy a charm."""
- def charm_panel_loaded(driver):
- """Wait for the charm panel to be ready and displayed."""
- charm_search =
driver.find_element_by_id('charm-search-trigger')
- # Click to open the charm panel.
- # Implicit wait should let this resolve.
- self.click(charm_search)
- panel = driver.find_element_by_id('juju-search-charm-panel')
- if panel.is_displayed():
- return panel
-
- charm_panel = self.wait_for(
- charm_panel_loaded, error='Unable to load charm panel.')
+ # Warning!
+ # This depends on manage.jujucharms.com being up and working
properly.
+ # For many reasons, hopefully this is not an issue :-) but if
+ # some inexplicable failure is going on here, try that possible
+ # source.
+ def get_search_box(driver):
+ # The charm browser sidebar should be open by default.
+ return driver.find_element_by_css_selector('[name=bws-search]')
+
+ def get_charm_token(driver):
+ # See
http://www.w3.org/TR/css3-selectors/#attribute-substrings .
+ return driver.find_element_by_css_selector(
+ '.yui3-charmtoken-content '
+ '.charm-token[data-charmid*={}]'.format(charm_name))
+
+ def get_add_button(driver):
+ return
driver.find_element_by_css_selector('.bws-view-data .add')
+
+ def get_deploy_button(driver):
+ return driver.find_element_by_id('charm-deploy')
+
+ # Search for the charm
+ search_box = self.wait_for(
+ get_search_box, error='Charm search box is not visible')
+ search_box.send_keys(charm_name)
+ search_box.send_keys('\n')
+
+ # Open details page
+ charm_token = self.wait_for(
+ get_charm_token, error='Charm sidebar is not visible.')
+ charm_token.click()
+
+ # Create Ghost
+ add_button = self.wait_for(
+ ...

Read more...

Revision history for this message
Richard Harding (rharding) wrote :

LGTM with small notes.

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

https://codereview.appspot.com/10253055/diff/1/test/test_charm_running.py#newcode119
test/test_charm_running.py:119: def get_search_box(driver):
is there any reason to go through search vs the default interesting data
enpoint? Just to verify that search is working?

https://codereview.appspot.com/10253055/diff/1/test/test_charm_running.py#newcode121
test/test_charm_running.py:121: return
driver.find_element_by_css_selector('[name=bws-search]')
so bws-search is another key we need to make sure to not change. I know
there was talk of re-designing the search area of the sidebar at some
point. Please update it with the same notes on breaking CI if it
changes.

https://codereview.appspot.com/10253055/

Revision history for this message
Jeff Pihach (hatch) wrote :

LGTM Thanks for taking over and fixing it up :)

https://codereview.appspot.com/10253055/diff/1/app/subapps/browser/templates/browser_charm.handlebars
File app/subapps/browser/templates/browser_charm.handlebars (right):

https://codereview.appspot.com/10253055/diff/1/app/subapps/browser/templates/browser_charm.handlebars#newcode15
app/subapps/browser/templates/browser_charm.handlebars:15: the other one
too! }}
Good idea with the comment!

https://codereview.appspot.com/10253055/

735. By Gary Poster

respond to review

Revision history for this message
Gary Poster (gary) wrote :

On 2013/06/14 17:26:32, rharding wrote:
> LGTM with small notes.

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

https://codereview.appspot.com/10253055/diff/1/test/test_charm_running.py#newcode119
> test/test_charm_running.py:119: def get_search_box(driver):
> is there any reason to go through search vs the default interesting
data
> enpoint? Just to verify that search is working?

At least in theory, this is a utility that lets you deploy arbitrary
charms. My goal was to duplicate the previous behavior--or at least the
advertised behavior :-) .

But yeah, even beyond that, this is in service of some integration
tests. I figured searching was not a bad thing in that context.

https://codereview.appspot.com/10253055/diff/1/test/test_charm_running.py#newcode121
> test/test_charm_running.py:121: return
> driver.find_element_by_css_selector('[name=bws-search]')
> so bws-search is another key we need to make sure to not change. I
know there
> was talk of re-designing the search area of the sidebar at some point.
Please
> update it with the same notes on breaking CI if it changes.

Done, thanks.

https://codereview.appspot.com/10253055/

Revision history for this message
Gary Poster (gary) wrote :

*** Submitted:

Fix CI tests

R=rharding, jeff.pihach
CC=
https://codereview.appspot.com/10253055

https://codereview.appspot.com/10253055/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches