Moodle
  1. Moodle
  2. MDL-37779 unit test fixing META
  3. MDL-37782

Provide better isolation of database test cases when transactions are being used

    Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3.4, 2.4.1, 2.5
    • Fix Version/s: 2.3.7, 2.4.4
    • Component/s: Unit tests
    • Labels:
    • Testing Instructions:
      Hide

      1/ run mysql DML tests
      2/ hack test_onelevel_rollback() as described in description
      3/ run mysql DML tests again - only one more failure expected

      Show
      1/ run mysql DML tests 2/ hack test_onelevel_rollback() as described in description 3/ run mysql DML tests again - only one more failure expected
    • Affected Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      w13_MDL-37782_m25_testtrans
    • Rank:
      47516

      Description

      After detecting a lot of "new" surprise/errors in the MSSQL driver, it was detected that any (real) failure in the test_onelevel_rollback() test case was affecting to all the rest of transaction test cases in lib/dml/tests/dml_test.php

      The ultimate cause is that the 1st test case leaves open one transaction, so all the next test cases fail badly.

      To reproduce it (under any database):

      1) Edit lib/dml/tests/dml_test.php

      2) Go to test_onelevel_rollback() and addd this line:

      $this->assertTrue(false);
      

      Before any of the allow_commit() calls in the method.

      (that will cause the testcase to stop, so the allow_coomit (close transactions is never executed).

      3) run "phpunit dml_testcase lib/dml/tests/dml_test.php"

      4) You will get various failures in other test cases because 2 left the transaction open.

      While this can be easily prevented in tearDown() or friends... surely the safest way is to close any transaction as part of the standard "reset" thing, to guarantee real test case independence.

      Ciao

        Activity

        Hide
        Eloy Lafuente (stronk7) added a comment -

        Note that this was detected because MSSQL is having one real fail there (and will need to be fixed apart).

        But only one, there are 3-4 more failures that are caused by this isolation problem.

        Ciao

        Show
        Eloy Lafuente (stronk7) added a comment - Note that this was detected because MSSQL is having one real fail there (and will need to be fixed apart). But only one, there are 3-4 more failures that are caused by this isolation problem. Ciao
        Hide
        Petr Škoda added a comment -

        Thanks for the report.

        Show
        Petr Škoda added a comment - Thanks for the report.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Integrated (23, 24 & master), thanks!

        Show
        Eloy Lafuente (stronk7) added a comment - Integrated (23, 24 & master), thanks!
        Hide
        Frédéric Massart added a comment -

        Passed on MySQL (2.3, 2.4, master) and PgSQL (master). Thanks.

        Show
        Frédéric Massart added a comment - Passed on MySQL (2.3, 2.4, master) and PgSQL (master). Thanks.
        Hide
        Damyon Wiese added a comment -

        Thanks for your hard work. This issue has been integrated upstream and is now available via git (and in some hours, via mirrors and downloads).

        Show
        Damyon Wiese added a comment - Thanks for your hard work. This issue has been integrated upstream and is now available via git (and in some hours, via mirrors and downloads).

          People

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

            Dates

            • Created:
              Updated:
              Resolved: