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

Qtype gapselect unit tests unsafe

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 3.5.4, 3.6, 3.7
    • Fix Version/s: 3.5.5, 3.6.3
    • Component/s: Questions
    • Labels:
    • Testing Instructions:
      Hide

      This needs to be done with access to the code base.

      Before patching.

      Setup unit tests and run the qtype gapselect walkthrough_test.php - it should pass.

      Now change any text in line 56. For instance change 'quick' to 'not quick'. Run the unit test again, and see how it still passes. The same applies for line 58 and all other similar lines.

      After patching.

      Check the qtype gapselect walkthrough_test.php passes.

      Change any text in line 61, for instance change 'quick' to 'not quick'. Run the test and check that it now fails. (The line number has changed because of the patch.)

      Show
      This needs to be done with access to the code base. Before patching. Setup unit tests and run the qtype gapselect walkthrough_test.php - it should pass. Now change any text in line 56. For instance change 'quick' to 'not quick'. Run the unit test again, and see how it still passes. The same applies for line 58 and all other similar lines. After patching. Check the qtype gapselect walkthrough_test.php passes. Change any text in line 61, for instance change 'quick' to 'not quick'. Run the test and check that it now fails. (The line number has changed because of the patch.)
    • Affected Branches:
      MOODLE_35_STABLE, MOODLE_36_STABLE, MOODLE_37_STABLE
    • Fixed Branches:
      MOODLE_35_STABLE, MOODLE_36_STABLE
    • Pull 3.5 Branch:
      MDL-64420-2_35
    • Pull 3.6 Branch:
      MDL-64420-2_36
    • Pull Master Branch:

      Description

      This was observed during testing of MDL-64404.

      Setup unit tests and run the qtype gapselect walkthrough_test.php - it should pass.

      Now change any text in line 56. For instance change 'quick' to 'not quick'. Run the unit test again, and see how it still passes. The same applies for line 58 and all other similar lines.

      The reason for this is that the question/engine/tests/helpers.php assert() calls the depreciated assertTag function and that function just checks for the presence of the right number of option tags within a select tag. The value of the option tag is never checked.

      This may not be very important as it could be argued that at least the gapselect selects are being checked for being present and having some options. It is just that a developer new to this area of code could easily be fooled into thinking that the option value is actually checked by the way that the test is written (well I was).

      Having found a solution for the qtype gapselect I have searched for other occurrences of similar issues and found that these qtypes and behaviours are affected:

      behaviour/adaptive

      behaviour/interactivecountback

      type/gapselect

      type/match

      type/multianswer

      type/numerical

      type/randomsamatch

      So include a fix for all these occurrences.

        Attachments

          Activity

            People

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

              Dates

              • Created:
                Updated:
                Resolved:
                Fix Release Date:
                11/Mar/19