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

          Attachments

            Issue Links

              Activity

              Hide
              mudrd8mz David Mudrák 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 Mudrák 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 Mudrák 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 Mudrák 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 Mudrák 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 Mudrák 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