[Java] Fix deselecting options in <select>

1. Fix inconsistency between java doc and implementation:
   deselectByVisibleText, deselectByIndex and deselectByValue
   did not throw NoSuchElementException, while they should have to.

2. Remove check !isMultiple() while selecting/deselecting by index,
   since no need as index is unique.

3. Add appropriate tests to SelectElementTest

4. Include SelectElementTest in LargeTests suite, since it was
   not a part of any.

Add exception when deselecting non-multiselect

Signed-off-by: Luke Inman-Semerau <[email protected]>

Author: glibas

java/client/src/org/openqa/selenium/support/ui/Select.java

@@ -116,7 +116,7 @@ public void selectByVisibleText(String text) {
 
     boolean matched = false;
     for (WebElement option : options) {
-      setSelected(option);
+      setSelected(option, true);
       if (!isMultiple()) {
         return;
       }
@@ -137,7 +137,7 @@ public void selectByVisibleText(String text) {
       }
       for (WebElement option : candidates) {
         if (text.equals(option.getText())) {
-          setSelected(option);
+          setSelected(option, true);
           if (!isMultiple()) {
             return;
           }
@@ -173,19 +173,13 @@ private String getLongestSubstringWithoutSpace(String s) {
   public void selectByIndex(int index) {
     String match = String.valueOf(index);
 
-    boolean matched = false;
     for (WebElement option : getOptions()) {
       if (match.equals(option.getAttribute("index"))) {
-        setSelected(option);
-        if (!isMultiple()) {
-          return;
-        }
-        matched = true;
+        setSelected(option, true);
+        return;
       }
     }
-    if (!matched) {
-      throw new NoSuchElementException("Cannot locate option with index: " + index);
-    }
+    throw new NoSuchElementException("Cannot locate option with index: " + index);
   }
 
   /**
@@ -203,7 +197,7 @@ public void selectByValue(String value) {
 
     boolean matched = false;
     for (WebElement option : options) {
-      setSelected(option);
+      setSelected(option, true);
       if (!isMultiple()) {
         return;
       }
@@ -227,9 +221,7 @@ public void deselectAll() {
     }
 
     for (WebElement option : getOptions()) {
-      if (option.isSelected()) {
-        option.click();
-      }
+      setSelected(option, false);
     }
   }
 
@@ -241,15 +233,23 @@ public void deselectAll() {
    *
    * @param value The value to match against
    * @throws NoSuchElementException If no matching option elements are found
+   * @throws UnsupportedOperationException If the SELECT does not support multiple selections
    */
   public void deselectByValue(String value) {
+    if (!isMultiple()) {
+      throw new UnsupportedOperationException(
+          "You may only deselect options of a multi-select");
+    }
+    
     List<WebElement> options = element.findElements(By.xpath(
         ".//option[@value = " + Quotes.escape(value) + "]"));
-
+    boolean matched = false;
     for (WebElement option : options) {
-      if (option.isSelected()) {
-        option.click();
-      }
+      setSelected(option, false);
+      matched = true;
+    }
+    if (!matched) {
+      throw new NoSuchElementException("Cannot locate option with value: " + value);
     }
   }
 
@@ -259,15 +259,23 @@ public void deselectByValue(String value) {
    *
    * @param index The option at this index will be deselected
    * @throws NoSuchElementException If no matching option elements are found
+   * @throws UnsupportedOperationException If the SELECT does not support multiple selections
    */
   public void deselectByIndex(int index) {
+    if (!isMultiple()) {
+      throw new UnsupportedOperationException(
+          "You may only deselect options of a multi-select");
+    }
+    
     String match = String.valueOf(index);
-
+    
     for (WebElement option : getOptions()) {
-      if (match.equals(option.getAttribute("index")) && option.isSelected()) {
-        option.click();
+      if (match.equals(option.getAttribute("index"))) {
+        setSelected(option, false);
+        return;
       }
     }
+    throw new NoSuchElementException("Cannot locate option with index: " + index);
   }
 
   /**
@@ -278,20 +286,40 @@ public void deselectByIndex(int index) {
    *
    * @param text The visible text to match against
    * @throws NoSuchElementException If no matching option elements are found
+   * @throws UnsupportedOperationException If the SELECT does not support multiple selections
    */
   public void deselectByVisibleText(String text) {
+    if (!isMultiple()) {
+      throw new UnsupportedOperationException(
+          "You may only deselect options of a multi-select");
+    }
+    
     List<WebElement> options = element.findElements(By.xpath(
         ".//option[normalize-space(.) = " + Quotes.escape(text) + "]"));
 
+    boolean matched = false;
     for (WebElement option : options) {
-      if (option.isSelected()) {
-        option.click();
-      }
+      setSelected(option, false);
+      matched = true;
+    }
+
+    if (!matched) {
+      throw new NoSuchElementException("Cannot locate element with text: " + text);
     }
   }
 
-  private void setSelected(WebElement option) {
-    if (!option.isSelected()) {
+  /**
+   * Select or deselect specified option
+   *
+   * @param option
+   *          The option which state needs to be changed
+   * @param select
+   *          Indicates whether the option needs to be selected (true) or
+   *          deselected (false)
+   */
+  private void setSelected(WebElement option, boolean select) {
+    boolean isSelected=option.isSelected();
+    if ((!isSelected && select) || (isSelected && !select)) {
       option.click();
     }
   }

java/client/test/org/openqa/selenium/support/LargeTests.java

@@ -20,13 +20,15 @@
 import org.junit.runner.RunWith;
 import org.junit.runners.Suite;
 import org.openqa.selenium.support.pagefactory.UsingPageFactoryTest;
+import org.openqa.selenium.support.ui.SelectElementTest;
 import org.openqa.selenium.support.ui.SelectLargeTest;
 import org.openqa.selenium.testing.JUnit4TestBase;
 
 @RunWith(Suite.class)
 @Suite.SuiteClasses({
     SelectLargeTest.class,
-    UsingPageFactoryTest.class
+    UsingPageFactoryTest.class,
+    SelectElementTest.class
 })
 public class LargeTests {
 

java/client/test/org/openqa/selenium/support/ui/SelectElementTest.java

@@ -270,4 +270,46 @@ public void shouldAllowOptionsToBeSelectedFromTheSelectElementThatIsNarrowerThan
 
     assertEquals(1,returnedOptions.size());
   }
+  
+  @Test(expected = NoSuchElementException.class)
+  public void shouldThrowExceptionOnDeselectByReturnedValueIfOptionDoesNotExist() {
+    WebElement selectElement = driver.findElement(By.name("select_empty_multiple"));
+    Select select = new Select(selectElement);
+    select.deselectByValue("not there");
+  }
+  
+  @Test(expected = NoSuchElementException.class)
+  public void shouldThrowExceptionOnDeselectByVisibleTextIfOptionDoesNotExist() {
+    WebElement selectElement = driver.findElement(By.name("select_empty_multiple"));
+    Select select = new Select(selectElement);
+    select.deselectByVisibleText("not there");
+  }
+  
+  @Test(expected = NoSuchElementException.class)
+  public void shouldThrowExceptionOnDeselectByIndexIfOptionDoesNotExist() {
+    WebElement selectElement = driver.findElement(By.name("select_empty_multiple"));
+    Select select = new Select(selectElement);
+    select.deselectByIndex(10);
+  }
+  
+  @Test(expected = UnsupportedOperationException.class)
+  public void shouldNotAllowUserToDeselectByIndexWhenSelectDoesNotSupportMultipleSelections() {
+    WebElement selectElement = driver.findElement(By.name("selectomatic"));
+    Select select = new Select(selectElement);
+    select.deselectByIndex(0);
+  }
+  
+  @Test(expected = UnsupportedOperationException.class)
+  public void shouldNotAllowUserToDeselectByValueWhenSelectDoesNotSupportMultipleSelections() {
+    WebElement selectElement = driver.findElement(By.name("selectomatic"));
+    Select select = new Select(selectElement);
+    select.deselectByValue("two");
+  }
+  
+  @Test(expected = UnsupportedOperationException.class)
+  public void shouldNotAllowUserToDeselectByVisibleTextWhenSelectDoesNotSupportMultipleSelections() {
+    WebElement selectElement = driver.findElement(By.name("selectomatic"));
+    Select select = new Select(selectElement);
+    select.deselectByVisibleText("Four");
+  }
 }