Moodle
  1. Moodle
  2. MDL-36199

Singletons in lib.pluginlib.php are not reset in unit tests

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.4
    • Fix Version/s: 2.4
    • Component/s: Unit tests
    • Labels:
    • Rank:
      44979

      Description

      The problem is that if something uses the singleton the tests for plugin_manager fail later because it can not be overridden by the test class.

        Issue Links

          Activity

          Hide
          David Mudrak added a comment -

          Oh yes, this finally explains the failure we've experienced recently. I found this pretty good overview of the issue http://sebastian-bergmann.de/archives/882-Testing-Code-That-Uses-Singletons.html

          Show
          David Mudrak added a comment - Oh yes, this finally explains the failure we've experienced recently. I found this pretty good overview of the issue http://sebastian-bergmann.de/archives/882-Testing-Code-That-Uses-Singletons.html
          Hide
          David Mudrak added a comment -

          I just realize I need to apply the similar patch to other singletons in that library. Re-assigning from Petr to myself. I will prepare a new branch and will cherry-pick Petr's current patch into it.

          Show
          David Mudrak added a comment - I just realize I need to apply the similar patch to other singletons in that library. Re-assigning from Petr to myself. I will prepare a new branch and will cherry-pick Petr's current patch into it.
          Hide
          David Mudrak added a comment -

          Ooop sorry, false alarm. The new singleton I had in my mind has not been integrated yet. So, this has my +1, requesting integration.

          Eloy: it might be useful to try this patch at the branch where we saw that weird PHPUnit fatal error.

          Show
          David Mudrak added a comment - Ooop sorry, false alarm. The new singleton I had in my mind has not been integrated yet. So, this has my +1, requesting integration. Eloy: it might be useful to try this patch at the branch where we saw that weird PHPUnit fatal error.
          Hide
          Dan Poltawski added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Dan Poltawski added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          Dan Poltawski added a comment -

          I've integrated this into master. Thanks Petr.

          Do we need this in 2.3?

          Show
          Dan Poltawski added a comment - I've integrated this into master. Thanks Petr. Do we need this in 2.3?
          Hide
          Dan Poltawski added a comment -

          Looks good. (Question about 2.3 issue remains)

          Show
          Dan Poltawski added a comment - Looks good. (Question about 2.3 issue remains)
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Amazed. Inspired. Grateful. That’s how your generosity makes me feel.

          (not really)

          Closing, thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Amazed. Inspired. Grateful. That’s how your generosity makes me feel. (not really) Closing, thanks!
          Hide
          Dan Poltawski added a comment -

          GRRR! We do need this in 2.3.

          Show
          Dan Poltawski added a comment - GRRR! We do need this in 2.3.
          Hide
          Dan Poltawski added a comment -

          I think it makes MDL-30545 go boom!

          Show
          Dan Poltawski added a comment - I think it makes MDL-30545 go boom!
          Hide
          Petr Škoda added a comment -

          Looking...

          Show
          Petr Škoda added a comment - Looking...

            People

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

              Dates

              • Created:
                Updated:
                Resolved: