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:

      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.

        Gliffy Diagrams

          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 Skoda added a comment -

            Looking...

            Show
            Petr Skoda added a comment - Looking...

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: