AssertOptionSelected() does not work for dynamic HTML IDs

+++ b/core/modules/simpletest/src/AssertContentTrait.php
@@ -1160,7 +1160,7 @@ protected function assertNoFieldChecked($id, $message = '', $group = 'Browser')
   protected function assertOption($id, $option, $message = '', $group = 'Browser') {
-    $options = $this->xpath('//select[@id=:id]//option[@value=:option]', array(':id' => $id, ':option' => $option));
+    $options = $this->xpath('//select[contains(@id, :id)]//option[@value=:option]', array(':id' => $id, ':option' => $option));
     return $this->assertTrue(isset($options[0]), $message ? $message : SafeMarkup::format('Option @option for field @id exists.', array('@option' => $option, '@id' => $id)), $group);
   }

Starts with might be a better option, to avoid false positives?

Maybe even an option to explicitly make it starts-with, not sure...

Instead of using IDs we should promote assertion methods which deal which use the data-drupal-selector bits.

Can we try what happens if we just switch to data-drupal-selector?

1. +++ b/core/modules/filter/src/Tests/FilterFormTest.php
   @@ -253,18 +253,16 @@ protected function assertOptions($id, array $expected_options, $selected) {
   +    $select = $this->xpath('//select[@data-drupal-selector=:data_drupal_selector]', [':data_drupal_selector' => $drupal_selector]);
   

   Here we don't need to check if it contains 'required'?

2. +++ b/core/modules/simpletest/src/AssertContentTrait.php
   @@ -1163,8 +1163,7 @@ protected function assertNoFieldChecked($id, $message = '', $group = 'Browser')
   +    return $this->assertOptionWithDrupalSelector($id, $option, $message, $group);
   
   @@ -1213,9 +1212,9 @@ protected function assertOptionWithDrupalSelector($drupal_selector, $option, $me
   +    $options = $this->xpath('//select[@data-drupal-selector=:data_drupal_selector]//option[@value=:option]', array(':data_drupal_selector' => $id, ':option' => $option));
   +    return $this->assertFalse(isset($options[0]), $message ? $message : SafeMarkup::format('Option @option for field @data_drupal_selector exists.', array('@option' => $option, '@data_drupal_selector' => $id)), $group);
   
   @@ -1241,8 +1240,7 @@ protected function assertNoOption($id, $option, $message = '', $group = 'Browser
   +    return $this->assertOptionSelectedWithDrupalSelector($id, $option, $message, $group);
   
   @@ -1293,8 +1291,8 @@ protected function assertOptionSelectedWithDrupalSelector($drupal_selector, $opt
   +    $elements = $this->xpath('//select[@data-drupal-selector=:data_drupal_selector]//option[@value=:option]', array(':data_drupal_selector' => $id, ':option' => $option));
   +    return $this->assertFalse(isset($elements[0]) && !empty($elements[0]['selected']), $message ? $message : SafeMarkup::format('Option @option for field @data_drupal_selector is selected.', array('@option' => $option, '@data_drupal_selector' => $id)), $group);
   

   We should change the $id also here to $drupal_selector, i guess?

3. +++ b/core/modules/simpletest/src/AssertContentTrait.php
   @@ -1213,9 +1212,9 @@ protected function assertOptionWithDrupalSelector($drupal_selector, $option, $me
   +
   

   Empty line.

Looks good to me :).

Nice find!

1. +++ b/core/modules/simpletest/src/AssertContentTrait.php
   @@ -1216,15 +1215,15 @@ protected function assertOption($id, $option, $message = '', $group = 'Browser')
   -    $options = $this->xpath('//select[@data-drupal-selector=:data_drupal_selector]//option[@value=:option]', array(':data_drupal_selector' => $drupal_selector, ':option' => $option));
   -    return $this->assertTrue(isset($options[0]), $message ? $message : SafeMarkup::format('Option @option for field @data_drupal_selector exists.', array('@option' => $option, '@data_drupal_selector' => $drupal_selector)), $group);
   +    $options = $this->xpath('//select[@data-drupal-selector=:data_drupal_selector]//option[@value=:option]', [':data_drupal_selector' => $drupal_selector, ':option' => $option]);
   +    return $this->assertTrue(!empty((string) $options[0]), $message ? $message : SafeMarkup::format('Option @option for field @data_drupal_selector exists.', array('@option' => $option, '@data_drupal_selector' => $drupal_selector)), $group);
   

   Why is it necessary/correct to switch to the string cast here instead of the isset check? We didn't do that elsewhere in the patch. An inline comment would be good if there is a reason it's different.

2. +++ b/core/modules/simpletest/src/AssertContentTrait.php
   @@ -1320,9 +1318,9 @@ protected function assertOptionSelectedWithDrupalSelector($drupal_selector, $opt
   -  protected function assertNoOptionSelected($id, $option, $message = '', $group = 'Browser') {
   -    $elements = $this->xpath('//select[@id=:id]//option[@value=:option]', array(':id' => $id, ':option' => $option));
   -    return $this->assertTrue(isset($elements[0]) && empty($elements[0]['selected']), $message ? $message : SafeMarkup::format('Option @option for field @id is not selected.', array('@option' => $option, '@id' => $id)), $group);
   +  protected function assertNoOptionSelected($drupal_selector, $option, $message = '', $group = 'Browser') {
   +    $elements = $this->xpath('//select[@data-drupal-selector=:data_drupal_selector]//option[@value=:option]', array(':data_drupal_selector' => $drupal_selector, ':option' => $option));
   +    return $this->assertFalse(isset($elements[0]) && !empty($elements[0]['selected']), $message ? $message : SafeMarkup::format('Option @option for field @data_drupal_selector is selected.', array('@option' => $option, '@data_drupal_selector' => $drupal_selector)), $group);
   

   This is changing the logic of the assertion in a pretty confusing way... why? I think the result is also changing for the condition where the element is not set. Before, that would trigger a fail; now, it will trigger a pass. I think; I could have gotten confused though while drawing the truth table in my head because of the compound negatives. An inline comment would be good and I feel like we also need more test coverage for this assertion's behavior itself for all four possible combinations. (Well, all three that is; the 'selected' obviously must be empty if the parent element is not set.)

I think this would need a small change record for those using this assertion, also.

BTW, for reviewers, I highly recommending doing a word diff on this one.

Forgot the tag.

It seems we had a @todo there. I tried using the function in a contrib module but it does not work with dynamic html ids. Added the assertOptionSelectedWithName function. Should we also add something like assertOptionSelectedWithId? Also not sure if this should go to 8.2.x.

Triaging issues in simpletest.module as part of the Bug Smash Initiative to determine if they should be in the Simpletest Project or core.

This looks like it a Phpunit issue, changing component.

This looks like it was already deprecated in https://www.drupal.org/project/drupal/issues/3114617 in favor of

$this->assertSession()->optionExists() instead and check the "selected" attribute yourself.

Should this be closed as a duplicate?

I'd say outdated it is, we didn't fix the problem, we just removed the problematic method.