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

Should all qtype get_question_options() call parent ?

    XMLWordPrintable

    Details

    • Testing Instructions:
      Hide

      1. CI passing should be enough to verify this is ok.
      2. Bonus: Can run mod_quiz_testsuite and core_question_testsuite with php 7.4 and verify that there isn't any "Creating default object from empty value" problem within questions anymore.

      Show
      1. CI passing should be enough to verify this is ok. 2. Bonus: Can run mod_quiz_testsuite and core_question_testsuite with php 7.4 and verify that there isn't any "Creating default object from empty value" problem within questions anymore.
    • Affected Branches:
      MOODLE_36_STABLE, MOODLE_37_STABLE, MOODLE_38_STABLE, MOODLE_39_STABLE
    • Fixed Branches:
      MOODLE_38_STABLE
    • Pull Master Branch:

      Description

      Doing some preliminary tests with php74, there are some warnings leading to phpunit failures like this:

      mod_quiz_attempt_walkthrough_from_csv_testcase::test_walkthrough_from_csv with data set #0 (array('00', 'deferredfeedback'), array(PHPUnit\DbUnit\DataSet\DefaultTable Object (...), PHPUnit\DbUnit\DataSet\DefaultTable Object (...), PHPUnit\DbUnit\DataSet\DefaultTable Object (...)))
      Creating default object from empty value
       
      /var/www/html/question/type/multianswer/questiontype.php:67
      /var/www/html/lib/questionlib.php:939
      /var/www/html/lib/questionlib.php:1008
      /var/www/html/mod/quiz/attemptlib.php:164
      /var/www/html/mod/quiz/locallib.php:169
      /var/www/html/mod/quiz/tests/attempt_walkthrough_from_csv_test.php:271
      /var/www/html/mod/quiz/tests/attempt_walkthrough_from_csv_test.php:152
      /var/www/html/mod/quiz/tests/attempt_walkthrough_from_csv_test.php:69
      /var/www/html/lib/phpunit/classes/advanced_testcase.php:80
      

      php74 is more picky about situations like that (and that's a good thing, I think).

      So, looking over all qtypes... I've observed that a number of them, having the get_question_options() method do use to call to parent one (that is in charge of initialising the question->options object:

      $ ag 'on get_question_options\(' -l | xargs ag 'parent::get_question_options' -l
      randomsamatch/questiontype.php
      gapselect/questiontypebase.php
      numerical/questiontype.php
      match/questiontype.php
      ddimageortext/questiontypebase.php
      multichoice/questiontype.php
      essay/questiontype.php
      

      But other qtypes aren't calling parent ever (like multianswer above in the failure):

      $ ag 'on get_question_options\(' -l | xargs ag 'parent::get_question_options' -L
      multianswer/questiontype.php
      truefalse/questiontype.php
      calculated/questiontype.php
      random/questiontype.php
      questiontypebase.php
      

      So, this is about to decide one of:

      1) Each qtype is responsible of initialising options its own 100%.
      2) Each qtype should be always calling to parent, apart from any other own and particular init needed.

      In any of the cases the outcome is clear, to have everything available before trying to access to it or php74 will cry (well done).

      (for some reason, ensuring that parent is always called, aka 2, sounds better here, but I don't know the internals enough)

      Ciao

      PS: Feel free to consider this an improvement, i've initially classified it as bug.

        Attachments

          Issue Links

            Activity

              People

              Assignee:
              stronk7 Eloy Lafuente (stronk7)
              Reporter:
              stronk7 Eloy Lafuente (stronk7)
              Peer reviewer:
              Tim Hunt Tim Hunt
              Integrator:
              Andrew Lyons Andrew Lyons
              Tester:
              CiBoT CiBoT
              Participants:
              Component watchers:
              Tim Hunt, Andrew Lyons, Dongsheng Cai, Huong Nguyen, Jun Pataleta, Michael Hawkins, Shamim Rezaie, Simey Lameze
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:
                Fix Release Date:
                11/May/20

                  Time Tracking

                  Estimated:
                  Original Estimate - 0 minutes
                  0m
                  Remaining:
                  Remaining Estimate - 0 minutes
                  0m
                  Logged:
                  Time Spent - 5 hours, 4 minutes
                  5h 4m