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

MDL_PERF constants should be always defined

    XMLWordPrintable

Details

    • MOODLE_402_STABLE
    • MOODLE_403_STABLE
    • Hide

      Test by setting constants in config.php:

      1. run PHPUnit with define('MDL_PERF', false)
      2. run PHPUnit with define('MDL_PERF', true)
      3. run PHPUnit with define('MDL_PERF', true) and define('MDL_PERFTOFOOT', true)
      4. run Behat with define('MDL_PERF', true) and define('MDL_PERFTOFOOT', true)
      5. test https://github.com/moodlehq/moodle-performance-comparison if still relevant
      6. test all MDL_PERF* constants - include Moodle install, Moodle upgrade and regular operation of web site

      NOTES:

      • MDL_PERFDB was removed completely because PERF->logwrites is not used after removal of legacy logging subsystem.
      • MDL_PERFTOLOG == true is not compatible with send_schedule_test PHUnit tests
      Show
      Test by setting constants in config.php: run PHPUnit with define('MDL_PERF', false) run PHPUnit with define('MDL_PERF', true) run PHPUnit with define('MDL_PERF', true) and define('MDL_PERFTOFOOT', true) run Behat with define('MDL_PERF', true) and define('MDL_PERFTOFOOT', true) test https://github.com/moodlehq/moodle-performance-comparison if still relevant test all MDL_PERF* constants - include Moodle install, Moodle upgrade and regular operation of web site NOTES: MDL_PERFDB was removed completely because PERF->logwrites is not used after removal of legacy logging subsystem. MDL_PERFTOLOG == true is not compatible with send_schedule_test PHUnit tests

    Description

      Hi team, this issue was inspired by, and is somewhat related to MDL-76506.

      If a process executes two files that have say,

      file1:

      include "$CFG->dirroot/lib/setup.php";

      file2:

      require("$CFG->dirroot/lib/setup.php"); 

      This will cause `MDL_PERF`, `MDL_PERFDB`, and `MDL_PERFTOFOOT` to be defined as `true`, even if they are not defined in the configuration.

      An example of this could be the following:

      Steps to replicate

      1. Make sure there is a line to the same effect of (or copy this one) `require("$CFG->dirroot/lib/setup.php");` in your 'config.php' file (or equivalent configuration file that you use). Put a breakpoint on this line.
      2. On https://github.com/moodle/moodle/blob/master/lib/phpunit/bootstrap.php#L217 you can see that it contains `require("$CFG->dirroot/lib/setup.php");` which is the second require of 'lib/setup.php'. Put a breakpoint on this line as well.
      3. Setup the PhpUnit environment, and listen for debugging connections with Xdebug or similar.
      4. Run any PhpUnit test, e.g. `php vendor/bin/phpunit admin/tool/mobile/tests/api_test.php --filter test_get_autologin_key`.
      5. When you trip the breakpoint from step 1, step into that line, and execute the 'lib/setup.php' file until the line that says `if (!defined('MDL_PERF_TEST')) {`. Step over. In this first pass, `MDL_PERF_TEST` is defined as false.
      6. Continue execution, when the second breakpoint is tripped from step 2, step into that line, and like in step 5, continue execution up to the line that says `if (!defined('MDL_PERF_TEST')) {`. Step over a few times. In this second pass `MDL_PERF`, `MDL_PERFDB`, and `MDL_PERFTOFOOT` are defined as `true`.

      Observed behavior

      The constants `MDL_PERF`, `MDL_PERFDB`, and `MDL_PERFTOFOOT` are defined as `true` even when they are not defined in the configuration of the site.

      Expected behavior

      The constants `MDL_PERF`, `MDL_PERFDB`, and `MDL_PERFTOFOOT` are not defined, unless defined in the configuration.

      Possible solution

      This seems like one of those cases where the `if` can be extended from:

      if (!defined('MDL_PERF_TEST')) { 

      to:

      if (!defined('MDL_PERF_TEST') || !MDL_PERF_TEST) { 

      such that when `MDL_PERF_TEST` is already defined but is `false`, the same flow of execution is followed... I tested this solution and it works, but it produces a PHP Notice (because `MDL_PERF_TEST` was already defined in the first pass), so this may not be the appropriate solution. I guess that another solution would be to enclose the `ifs` of `MDL_PERF`, `MDL_PERFDB`, and `MDL_PERFTOFOOT`, into an `if (MDL_PERF_TEST) {`. I tested that too and it worked, without producing a PHP Notice.

      Thank you for your attention to the present issue.

       

      Attachments

        1. MDL-78552-4.png
          MDL-78552-4.png
          50 kB
        2. MDL-78552-3.png
          MDL-78552-3.png
          39 kB
        3. MDL-78552-2.png
          MDL-78552-2.png
          37 kB
        4. MDL-78552-1.png
          MDL-78552-1.png
          37 kB

        Activity

          People

            skodak Petr Skoda (Inactive)
            julian.tovar Julian Tovar
            Farhan Karmali Farhan Karmali
            Andrew Lyons Andrew Lyons
            Kevin Percy Kevin Percy
            Votes:
            0 Vote for this issue
            Watchers:
            7 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Time Tracking

                Estimated:
                Original Estimate - Not Specified
                Not Specified
                Remaining:
                Remaining Estimate - 0 minutes
                0m
                Logged:
                Time Spent - 1 hour, 15 minutes
                1h 15m

                Clockify

                  Error rendering 'clockify-timesheets-time-tracking-reports:timer-sidebar'. Please contact your Jira administrators.