Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-43394

Behat checkbox controller "clicks" instead of "checks"

    Details

      Description

      I discovered this while writing Behat tests for MDL-40241.

      No matter what I did I could not get the following to work

       And I set the following administration settings values:
          | Enable completion tracking | 1 |
       And I log out
      

      I'd always get the error:

            Exception thrown by (//html//*[self::input | self::textarea | self::select][not(./@type = 'submit' or ./@type = 'image' or ./@type = 'hidden')][@id=//label[contains(normalize-space(.), 'Enable completion tracking')]/@for])[1]
            Unable to submit on a "input" tag.
      

      Digging around in the code I think I found the correct fix

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            dmonllao David Monllaó added a comment -

            Hi,

            Thanks for the bug report Aaron you are becoming the bug reporter number one In fact we already have features setting this administration setting (completion/tests/behat/restrict_section_availability.feature), the difference is that their scenarios are using @javascript, so they run in selenium / phantomjs rather than in a headless browser that can only follow links and submit buttons; the test you provided is without Javascript and that's why it fails, as Goutte (the non-JS driver used by behat) can not "click" on inputs.

            The problem with the provided patch is that we need to "click" checkboxes when JS is running because otherwise the onclick listeners attached to the form elements will not notice about the change; to allow a different behaviour depending on whether we are running a test using JS or not we can use $this->running_javascript()

            lib/behat/form_field/behat_form_checkbox.php::set_value()
             
                public function set_value($value) { 
                  
                    if (!$this->running_javascript()) {
                        $this->field->check();
                        return;
                    }
                
                    if (!empty($value) && !$this->field->isChecked()) {
                        // Check it if it should be checked and it is not.
                        $this->field->click();
                
                    } else if (empty($value) && $this->field->isChecked()) {
                        // Uncheck if it is checked and shouldn't.
                        $this->field->click();
                    } 
                }
             

            With this patch all scenarios should work as expected, feel free to pick this code and update your pull branch. Would be good to backport this patch as it is a bug.

            Show
            dmonllao David Monllaó added a comment - Hi, Thanks for the bug report Aaron you are becoming the bug reporter number one In fact we already have features setting this administration setting (completion/tests/behat/restrict_section_availability.feature), the difference is that their scenarios are using @javascript, so they run in selenium / phantomjs rather than in a headless browser that can only follow links and submit buttons; the test you provided is without Javascript and that's why it fails, as Goutte (the non-JS driver used by behat) can not "click" on inputs. The problem with the provided patch is that we need to "click" checkboxes when JS is running because otherwise the onclick listeners attached to the form elements will not notice about the change; to allow a different behaviour depending on whether we are running a test using JS or not we can use $this->running_javascript() lib/behat/form_field/behat_form_checkbox.php::set_value()   public function set_value($value) { if (!$this->running_javascript()) { $this->field->check(); return; } if (!empty($value) && !$this->field->isChecked()) { // Check it if it should be checked and it is not. $this->field->click(); } else if (empty($value) && $this->field->isChecked()) { // Uncheck if it is checked and shouldn't. $this->field->click(); } }   With this patch all scenarios should work as expected, feel free to pick this code and update your pull branch. Would be good to backport this patch as it is a bug.
            Hide
            sry_not4sale Aaron Barnes added a comment -

            Updated patch, thanks David - that all makes complete sense

            Show
            sry_not4sale Aaron Barnes added a comment - Updated patch, thanks David - that all makes complete sense
            Hide
            poltawski Dan Poltawski added a comment -

            Thanks Aaron - integrated to master, 26 and 25

            Show
            poltawski Dan Poltawski added a comment - Thanks Aaron - integrated to master, 26 and 25
            Hide
            salvetore Michael de Raadt added a comment -

            Test result: Success!

            Tested in 2.5, 2.6 and master.

            1 scenario (1 passed)
            7 steps (7 passed)
            0m12.476s
            

            Show
            salvetore Michael de Raadt added a comment - Test result: Success! Tested in 2.5, 2.6 and master. 1 scenario (1 passed) 7 steps (7 passed) 0m12.476s
            Hide
            damyon Damyon Wiese added a comment -

            David built a framework for behat
            At first just to test this and that
            10000+ steps written
            Sounds like we're all smitten
            And David should be smiling at that

            Thanks for reporting, patching, and testing this issue. It has been released upstream along with 64 others today.

            Show
            damyon Damyon Wiese added a comment - David built a framework for behat At first just to test this and that 10000+ steps written Sounds like we're all smitten And David should be smiling at that Thanks for reporting, patching, and testing this issue. It has been released upstream along with 64 others today.

              People

              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  13/Jan/14