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 Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.5
    • Fix Version/s: 2.4.6, 2.5.2
    • Component/s: Unit tests
    • Labels:
      None
    • Rank:
      51172

      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.

        Issue Links

          Activity

          Hide
          Jamie Pratt added a comment -

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

          Show
          Jamie Pratt added a comment - Linking this issue to the issue about the new formslib mock_submit method.
          Hide
          Petr Škoda 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
          Petr Škoda 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
          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
          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
          Petr Škoda added a comment -

          +1

          Show
          Petr Škoda added a comment - +1
          Hide
          Jamie Pratt added a comment -

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

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

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

          Show
          Petr Škoda added a comment - +1, sending to integration, I vote for 2.5 branch + master because there is no risk of regression
          Hide
          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 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 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 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
          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
          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 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 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 Wiese added a comment -

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

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

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

          Thanks for your contribution!

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

            People

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

              Dates

              • Created:
                Updated:
                Resolved: