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

Behat checkbox controller "clicks" instead of "checks"

    Details

    • Testing Instructions:
      Hide

      Create a Behat test like so:

      @mdl43394
      Feature: Test MDL-43394
       
          Scenario: Why does this not work?
              Given I log in as "admin"
              And I set the following administration settings values:
              | Enable completion tracking | 1 |
              And I log out
      

      This should fail without this patch (and show aforementioned error)

      Show
      Create a Behat test like so: @mdl43394 Feature: Test MDL-43394   Scenario: Why does this not work? Given I log in as "admin" And I set the following administration settings values: | Enable completion tracking | 1 | And I log out This should fail without this patch (and show aforementioned error)
    • Affected Branches:
      MOODLE_25_STABLE, MOODLE_26_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE, MOODLE_26_STABLE
    • Pull Master Branch:

      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

          Attachments

            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