Fix the UI label for verifying flash plugin status.

functional/about_plugins_ui.py

 #!/usr/bin/env python
 # Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 import os
 import re
 
 import pyauto_functional  # Must be imported before pyauto
 import pyauto
@@ skipping 112 matching lines @@
       internal_flash_elem = pyauto_utils.WaitForDomElement(self, driver,
           './/*[contains(text(), "gcswf32.dll")]')
 
       # Disable internal Flash plugin.
       internal_flash_disable_link = internal_flash_elem.find_element_by_xpath(
           './/ancestor::*[@class="plugin-details"]//a[text()="Disable"]')
       internal_flash_disable_link.click()
 
       # Verify internal flash is disabled.
       pyauto_utils.WaitForDomElement(self, internal_flash_elem,
-          './/ancestor::*[@class="plugin-file-disabled"]')
+          './/ancestor::*[@class="plugin-disabled"]')
       self.assertTrue(self.WaitUntil(lambda: not
           self._IsEnabled('Shockwave Flash')))
 
       # Enable internal Flash plugin.
       internal_flash_enable_link = internal_flash_elem.find_element_by_xpath(
           './/ancestor::*[@class="plugin-details"]//a[text()="Enable"]')
       internal_flash_enable_link.click()
       self.assertTrue(self.WaitUntil(lambda: len(
           internal_flash_elem.find_elements_by_xpath(
-          './/ancestor::*[@class="plugin-file-enabled"]')) == 1),
+          './/ancestor::*[@class="plugin-enabled"]')) >= 1),

[Nirnimesh, 2012/04/26 18:45:05]

are there many of these 'plugin-enabled' elements? If so, how exactly are you
verifying that a new one got created?

[vivianz, 2012/04/26 18:50:58]

 there used to be 2 set plugin-enabled and plugin-file-enabled, when you enable
flash plugin both will be enabled, the dev just did a combine of the 2 since it
does not make sense to have two sets, it's redundant. for my test I just need to
verify the class label flipped to plugin-enabled. used to have 1 called
plugin-file-enabled , now we have 2 both plugin-enabled. but they don't affect
this test.

[Nirnimesh, 2012/04/26 19:06:14]

I understand what you're trying to do, but I don't see from the test how you're
verifying that the flip happened?

Is the count 0 before the click on line 140?
Why did you change line 143 to >=1?

[dyu1, 2012/04/26 19:14:52]

Do they both need to be enabled? Are the plugins exclusive or inclusive of each
other? Trying to figure out why you changed from '==' to '=>'. Does it mean that
at any given time one plugin can be enabled while the other is disabled or both
can be enabled? 

On 2012/04/26 18:50:58, vivianz wrote:

[vivianz, 2012/04/26 22:08:58]

yes, before the click on line 140, the class name is plugin-disabled

On 2012/04/26 19:06:14, Nirnimesh wrote:

[vivianz, 2012/04/26 22:08:58]

because now after developer make plugin-file-[something] folder deprecated. now
both named plugin-[something], so i have 2 now, that is the reason i change it
to >=.
On 2012/04/26 19:14:52, dyu1 wrote:

[Nirnimesh, 2012/04/26 22:19:58]

then why not match to 2? Why >= 1?

[vivianz, 2012/04/26 22:30:51]

I could, I make it >=1 in case dev make another one or get rid of one of them in
the future, the test will be more robust this way, since it only need to make
sure 
 one class label is flipped, how many of them them exist is irrelevant here. 

On 2012/04/26 22:19:58, Nirnimesh wrote:

[Nirnimesh, 2012/04/26 22:33:39]

Before the click happens, what's the count? 1?
If so, you should match for 2 here, or else this check is a no-op.

Or did I totally misinterpret what's going on here?

[vivianz, 2012/04/26 22:42:56]

before the click , the class name is plugin-disabled, after the click the class
name is plugin-enabled . so if click the enable button and nothing happen ( bug
scenario) , the class name will not be flipped , so checking here will be count
0, therefore fail the test. so the correct way is before click, this one count
0, after click this one count >=0.
On 2012/04/26 22:33:39, Nirnimesh wrote:

[Nirnimesh, 2012/04/26 23:00:07]

I think you should do a strict match. ie == 1 here.

[vivianz, 2012/04/26 23:15:04]

I can make it ==2, but I just don't see the point of doing so, it only make the
test more flaky. can you tell me what is the benefit to doing so? I don't want
to have to come back fix it again if few days later dev decide the other folder
is redundant after all and removed it.
On 2012/04/26 23:00:07, Nirnimesh wrote:

[Nirnimesh, 2012/04/26 23:21:33]

I want to ensure that reality is coded in here.
Options:
  if before click, count of plugin-enabled is 0, check for >0 after click

I don't understand where ==2 comes from, maybe that's leading to the confusion.

Also, line 142 is looking for elements only in the flash dom element. why would
you expect there to be more than 1 plugin-enabled items?

[vivianz, 2012/04/26 23:37:23]

got it, I will change it to >0 tomorrow, that make sense, thanks for the point!
On 2012/04/26 23:21:33, Nirnimesh wrote:

[vivianz, 2012/04/27 18:00:32]

Done.

           msg='Failed to enable internal Flash plugin')
       self.assertTrue(self.WaitUntil(lambda:
           self._IsEnabled('Shockwave Flash')))
 
       # Disable all flash plugins.
       flash_disable_all_path = """
       .//ancestor::*[@class="plugin-text"]//following-sibling::*
       [@class="plugin-actions"]//a[text()="Disable"]
       """
       flash_disable_all_link = internal_flash_elem.find_element_by_xpath(
@@ skipping 14 matching lines @@
       # Re-enable Flash plugin from flash detail info.
       flash_enable_link = flash_plugins_elem.find_element_by_xpath(
           './/a[text()="Enable"]')
       flash_enable_link.click()
       self.assertTrue(self.WaitUntil(lambda:
           self._IsEnabled('Shockwave Flash')))
 
 
 if __name__ == '__main__':
   pyauto_functional.Main()