Code review comment for lp:~bjornt/launchpad/bug-415336

On Wed, Aug 19, 2009 at 09:09:58AM -0000, Gavin Panella wrote:
> Review: Approve
> It's good to have these fixed :)
>
> One small ateration and it's all good.
>
> Gavin.
>
>
> > === modified file 'lib/lp/bugs/windmill/tests/test_bugs/test_filebug_dupe_finder.py'
> > --- lib/lp/bugs/windmill/tests/test_bugs/test_filebug_dupe_finder.py 2009-06-25 00:40:31 +0000
> > +++ lib/lp/bugs/windmill/tests/test_bugs/test_filebug_dupe_finder.py 2009-08-19 08:53:28 +0000
> > @@ -8,7 +8,7 @@
> > WAIT_PAGELOAD = u'20000'
> > WAIT_ELEMENT_COMPLETE = u'20000'
> > WAIT_CHECK_CHANGE = u'1000'
> > -FILEBUG_URL = 'http://launchpad.dev:8085/firefox/+filebug'
> > +FILEBUG_URL = 'http://bugs.launchpad.dev:8085/firefox/+filebug'
> >
> > FORM_OVERLAY = u'//div[@id="duplicate-overlay-bug-4"]/table'
> > FORM_OVERLAY_CANCEL = (
> > @@ -24,6 +24,9 @@
> > FORM_VISIBLE = (
> > u'element.className.search("yui-lazr-formoverlay-hidden") == -1')
> >
> > +BUG_INFO_HIDDEN = 'style.height|0px'
> > +BUG_INFO_SHOWN_JS = 'element.style.height != "0px"'
> > +
> >
> > def test_duplicate_finder():
> > """Test the +filebug duplicate finder.
> > @@ -47,10 +50,9 @@
> > client.type(text=u'problem', id=u'field.title')
> > client.click(xpath=u'//input[@id="field.actions.search"]')
> > client.waits.forPageLoad(timeout=WAIT_PAGELOAD)
> > -
> > # The details div for the duplicate bug should not be shown.
> > client.asserts.assertProperty(
> > - id='details-for-bug-4', validator='style.display|none')
> > + id='details-for-bug-4', validator=BUG_INFO_HIDDEN)
> >
> > # The expander for the duplicate should be collapsed.
> > client.asserts.assertProperty(
> > @@ -64,8 +66,8 @@
> > client.waits.sleep(milliseconds=WAIT_CHECK_CHANGE)
> > client.asserts.assertProperty(
> > id='bug-details-expander-bug-4', validator='src|/@@/treeExpanded')
> > - client.asserts.assertProperty(
> > - id='details-for-bug-4', validator='style.display|block')
> > + client.asserts.assertElemJS(
> > + id='details-for-bug-4', js=BUG_INFO_SHOWN_JS)
> >
> > # Clicking the expander again will hide the details div and collapse
> > # the expander.
> > @@ -74,15 +76,15 @@
> > client.asserts.assertProperty(
> > id='bug-details-expander-bug-4', validator='src|/@@/treeCollapsed')
> > client.asserts.assertProperty(
> > - id='details-for-bug-4', validator='style.display|none')
> > + id='details-for-bug-4', validator=BUG_INFO_HIDDEN)
> >
> > # Clicking it yet again will reopen it.
> > client.click(id='bug-details-expander-bug-4')
> > client.waits.sleep(milliseconds=WAIT_CHECK_CHANGE)
> > client.asserts.assertProperty(
> > id='bug-details-expander-bug-4', validator='src|/@@/treeExpanded')
> > - client.asserts.assertProperty(
> > - id='details-for-bug-4', validator='style.display|block')
> > + client.asserts.assertElemJS(
> > + id='details-for-bug-4', js='element.style.height != "0px"')
>
> I guess this should be: js=BUG_INFO_SHOWN_JS

Right, good catch.

--
Björn Tillenius | https://launchpad.net/~bjornt