Moodle
  1. Moodle
  2. MDL-27401

Unit test lib/simpletest/testpagelib_moodlepage.php fails

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0.2
    • Fix Version/s: 2.0.4
    • Component/s: Unit tests
    • Rank:
      17085

      Description

      The unit test fails because changes in other areas mean it now calls get_fast_modinfo, and the unit test has not set up enough database tables for that to work.

      This doesn't indicate a practical problem, only unit test problem.

        Issue Links

          Activity

          Hide
          Sam Marshall added a comment -

          Fixed:

          1) Added necessary db tables and data to make get_fast_modinfo work.

          2) The two tests that check errors if $cm does not have name, modname in are now unnecessary (deleted) because this function now uses the version from geT_fast_modinfo so it will always obtain these values whatever the user provided.

          Show
          Sam Marshall added a comment - Fixed: 1) Added necessary db tables and data to make get_fast_modinfo work. 2) The two tests that check errors if $cm does not have name, modname in are now unnecessary (deleted) because this function now uses the version from geT_fast_modinfo so it will always obtain these values whatever the user provided.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Uhm, the patch looks perfect in order to allow lib/simpletest/testpagelib_moodlepage.php , in fact it runs ok.

          But I've found that there are problems when running the whole lib/simpletest tests together.

          After a bit of looking, it seems that the get_fast_modinfo() has its own static cache and that causes anything using that function (explicitly or behind the scenes) to cause problems because "old" cached values are returned.

          So I'd propose to:

          1) create new function in modinfolib.php, called modinfolib_clear_all_caches_for_unit_testing() (and there perform the 'reset' call or watever is necessary to get the cache emptied.
          2) in the UnitTestCaseUsingDatabase->automatic_clean_up() add the call to that function created in 1)

          (just to follow the same approach used for accesslib and ensure all the tests using database start with clear modinfo)

          I'd happy to integrate that, TIA & ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Uhm, the patch looks perfect in order to allow lib/simpletest/testpagelib_moodlepage.php , in fact it runs ok. But I've found that there are problems when running the whole lib/simpletest tests together. After a bit of looking, it seems that the get_fast_modinfo() has its own static cache and that causes anything using that function (explicitly or behind the scenes) to cause problems because "old" cached values are returned. So I'd propose to: 1) create new function in modinfolib.php, called modinfolib_clear_all_caches_for_unit_testing() (and there perform the 'reset' call or watever is necessary to get the cache emptied. 2) in the UnitTestCaseUsingDatabase->automatic_clean_up() add the call to that function created in 1) (just to follow the same approach used for accesslib and ensure all the tests using database start with clear modinfo) I'd happy to integrate that, TIA & ciao
          Hide
          Sam Marshall added a comment -

          Thanks Eloy. I have updated this at the two branches above.

          • Re your point 1, there is already a way to reset the modinfo cache, it's not really a very nice way, but anyhow, it has been there since whenever, so I used it rather than make a new function.
          • This then revealed another couple bugs in the testpagelib (ie bits where it only worked because of old cached data) which I fixed. Also corrected a typo in a function name, woohoo.

          I ran the whole lib/simpletest tests again, they still do not all pass, but I don't see any remaining fails due to this issue (I think).

          Show
          Sam Marshall added a comment - Thanks Eloy. I have updated this at the two branches above. Re your point 1, there is already a way to reset the modinfo cache, it's not really a very nice way, but anyhow, it has been there since whenever, so I used it rather than make a new function. This then revealed another couple bugs in the testpagelib (ie bits where it only worked because of old cached data) which I fixed. Also corrected a typo in a function name, woohoo. I ran the whole lib/simpletest tests again, they still do not all pass, but I don't see any remaining fails due to this issue (I think).
          Hide
          Eloy Lafuente (stronk7) added a comment - - edited

          hehe, yeah I knew about the 'reset' way to clean it, in fact that is what I used here to check it was a caching problem. I just proposed to create the extra function to be "paired" with the way used in accesslib, that is better if something changes in the modinfolib implementation and we need to execute more stuff some day.

          But not worries, that is enough, thanks! Could you, please, for comparison purposes, paste here which tests are failing in your side?

          TIA and ciao

          Show
          Eloy Lafuente (stronk7) added a comment - - edited hehe, yeah I knew about the 'reset' way to clean it, in fact that is what I used here to check it was a caching problem. I just proposed to create the extra function to be "paired" with the way used in accesslib, that is better if something changes in the modinfolib implementation and we need to execute more stuff some day. But not worries, that is enough, thanks! Could you, please, for comparison purposes, paste here which tests are failing in your side? TIA and ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated, thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated, thanks!
          Hide
          Eloy Lafuente (stronk7) added a comment -

          This has already tested on integration by running the testpagelib_moodlepage.php tests alone and also all the /lib/simpletest ones and finally all the / tests.

          Everything went ok, both in 20_STABLE and master. So passing, thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - This has already tested on integration by running the testpagelib_moodlepage.php tests alone and also all the /lib/simpletest ones and finally all the / tests. Everything went ok, both in 20_STABLE and master. So passing, thanks!
          Hide
          Sam Marshall added a comment -

          OK, it turns out the fails went away when I set the http proxy, which wasn't set on this moodle install before for some reason. (Is it really correct that unit tests should need access to the public internet...? ah well.)

          I now get a green bar:

          52/56 test cases complete: 1037 passes, 0 fails and 0 exceptions.

          Show
          Sam Marshall added a comment - OK, it turns out the fails went away when I set the http proxy, which wasn't set on this moodle install before for some reason. (Is it really correct that unit tests should need access to the public internet...? ah well.) I now get a green bar: 52/56 test cases complete: 1037 passes, 0 fails and 0 exceptions.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Aha, nice(r), thanks for checking!

          Show
          Eloy Lafuente (stronk7) added a comment - Aha, nice(r), thanks for checking!
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Closing, because this has been sent upstream some minutes ago.

          Many thanks for your collaboration!

          Show
          Eloy Lafuente (stronk7) added a comment - Closing, because this has been sent upstream some minutes ago. Many thanks for your collaboration!

            People

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

              Dates

              • Created:
                Updated:
                Resolved: