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

unit test framework should check for unintentional changes to _POST, _GET and _FILES super globals during each test and reset the globals if necessary

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.5
    • Fix Version/s: 2.4.6, 2.5.2
    • Component/s: Unit tests
    • Labels:
      None

      Description

      I am introducing testing for processing of submitted form contents. We need to inject the test form data through the _POST or _GET and _FILES variable. The unit test framework should handle the resetting of these global vars after each test.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            jamiesensei Jamie Pratt added a comment -

            Linking this issue to the issue about the new formslib mock_submit method.

            Show
            jamiesensei Jamie Pratt added a comment - Linking this issue to the issue about the new formslib mock_submit method.
            Hide
            skodak Petr Skoda added a comment - - edited

            oh, you have forgotten _REQUEST. Also in CLI scripts these global variables are always empty. Maybe we could just empty them without any warning.

            Show
            skodak Petr Skoda added a comment - - edited oh, you have forgotten _REQUEST. Also in CLI scripts these global variables are always empty. Maybe we could just empty them without any warning.
            Hide
            jamiesensei Jamie Pratt added a comment -

            Just do this you mean Petr : https://github.com/jamiepratt/moodle/compare/wip_MDL-40388_v2

            I agree that there is no need to keep a copy of the global vars, just setting them to an empty array as they would be before the unit test makes sense.

            Show
            jamiesensei Jamie Pratt added a comment - Just do this you mean Petr : https://github.com/jamiepratt/moodle/compare/wip_MDL-40388_v2 I agree that there is no need to keep a copy of the global vars, just setting them to an empty array as they would be before the unit test makes sense.
            Hide
            skodak Petr Skoda added a comment -

            +1

            Show
            skodak Petr Skoda added a comment - +1
            Hide
            jamiesensei Jamie Pratt added a comment -

            Updated branch and compare link to point to new branch after input from Petr.

            Show
            jamiesensei Jamie Pratt added a comment - Updated branch and compare link to point to new branch after input from Petr.
            Hide
            skodak Petr Skoda added a comment -

            +1, sending to integration, I vote for 2.5 branch + master because there is no risk of regression

            Show
            skodak Petr Skoda added a comment - +1, sending to integration, I vote for 2.5 branch + master because there is no risk of regression
            Hide
            damyon Damyon Wiese added a comment -

            Testing this now on 24, 25 and master - this is a testing framework issue so it's ok for backports.

            Show
            damyon Damyon Wiese added a comment - Testing this now on 24, 25 and master - this is a testing framework issue so it's ok for backports.
            Hide
            damyon Damyon Wiese added a comment -

            One thing I noticed is the multiple lines of whitespace. I'll clean this in the patch once the tests pass.

            Show
            damyon Damyon Wiese added a comment - One thing I noticed is the multiple lines of whitespace. I'll clean this in the patch once the tests pass.
            Hide
            jamiesensei Jamie Pratt added a comment -

            Thanks for this and for cleaning up those stray newlines Damyon. Will try to be more careful in future.

            Show
            jamiesensei Jamie Pratt added a comment - Thanks for this and for cleaning up those stray newlines Damyon. Will try to be more careful in future.
            Hide
            damyon Damyon Wiese added a comment -

            Thanks Jamie,

            Integrated to 24, 25 and master.

            I forgot to clean the blank lines before I pushed - but no big deal.

            Show
            damyon Damyon Wiese added a comment - Thanks Jamie, Integrated to 24, 25 and master. I forgot to clean the blank lines before I pushed - but no big deal.
            Hide
            damyon Damyon Wiese added a comment -

            Marking this as passed as I ran unit tests on all branches in integration.

            Show
            damyon Damyon Wiese added a comment - Marking this as passed as I ran unit tests on all branches in integration.
            Hide
            damyon Damyon Wiese added a comment -

            a single bug fix
            a drop in a waterfall
            hear the mighty roar

            Thanks for your contribution!

            Show
            damyon Damyon Wiese added a comment - a single bug fix a drop in a waterfall hear the mighty roar Thanks for your contribution!

              People

              • Assignee:
                jamiesensei Jamie Pratt
                Reporter:
                jamiesensei Jamie Pratt
                Peer reviewer:
                Petr Skoda
                Integrator:
                Damyon Wiese
                Tester:
                Damyon Wiese
                Participants:
              • Votes:
                1 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  9/Sep/13