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

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

    Details

    • Type: Bug
    • Status: Closed
    • Priority: 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
            mudrd8mz 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
            mudrd8mz 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
            mudrd8mz 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
            mudrd8mz 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
            mudrd8mz 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
            mudrd8mz 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
            poltawski 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
            poltawski 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
            poltawski Dan Poltawski added a comment -

            I've integrated this into master. Thanks Petr.

            Do we need this in 2.3?

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

            Looks good. (Question about 2.3 issue remains)

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

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

            (not really)

            Closing, thanks!

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

            GRRR! We do need this in 2.3.

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

            I think it makes MDL-30545 go boom!

            Show
            poltawski Dan Poltawski added a comment - I think it makes MDL-30545 go boom!
            Hide
            skodak Petr Skoda added a comment -

            Looking...

            Show
            skodak Petr Skoda added a comment - Looking...

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  3/Dec/12