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

Unit tests contain hard coded file hash

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3
    • Fix Version/s: 2.3
    • Component/s: Unit tests
    • Labels:
    • Testing Instructions:
      Hide

      Replace the image backup/converter/moodle1/simpletest/files/icon.gif with another file of the same name. Run unit tests (to make them run faster limit to the path backup/converter/)

      Expect:

      • tests still pass

      What happens:

      • tests fails
      Show
      Replace the image backup/converter/moodle1/simpletest/files/icon.gif with another file of the same name. Run unit tests (to make them run faster limit to the path backup/converter/) Expect: tests still pass What happens: tests fails
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull Master Branch:

      Description

      In theory this could break tests in the future, I propose calculating the hash dynamically.

        Gliffy Diagrams

          Activity

          Hide
          salvetore Michael de Raadt added a comment -

          Thanks for contributing this, Aaron, but it should go through peer review before going to integration.

          Show
          salvetore Michael de Raadt added a comment - Thanks for contributing this, Aaron, but it should go through peer review before going to integration.
          Hide
          salvetore Michael de Raadt added a comment -

          Actually, I found it was not possible to go from the status "Re-opened" to "Request peer review". I've adjusted the workflow so this is now possible and it should appear in the list of issues to peer review.

          Show
          salvetore Michael de Raadt added a comment - Actually, I found it was not possible to go from the status "Re-opened" to "Request peer review". I've adjusted the workflow so this is now possible and it should appear in the list of issues to peer review.
          Hide
          salvetore Michael de Raadt added a comment -

          Setting Aaron as assignee as he has done he work.

          Show
          salvetore Michael de Raadt added a comment - Setting Aaron as assignee as he has done he work.
          Hide
          nebgor Aparup Banerjee added a comment -

          This looks good to me!
          Thanks for the patch!

          ------

          ps: i know its in the code above but i might be tempted to put the

          "$CFG->tempdir/backup/$this->tempdir/moddata/unittest/4/icon.gif" 

          as

          $CFG->tempdir . '/backup/' . $this->tempdir . '/moddata/unittest/4/icon.gif'

          just for clarity's sake (imo)

          Show
          nebgor Aparup Banerjee added a comment - This looks good to me! Thanks for the patch! ------ ps: i know its in the code above but i might be tempted to put the "$CFG->tempdir/backup/$this->tempdir/moddata/unittest/4/icon.gif" as $CFG->tempdir . '/backup/' . $this->tempdir . '/moddata/unittest/4/icon.gif' just for clarity's sake (imo)
          Hide
          sry_not4sale Aaron Barnes added a comment -

          Hi Aprarup,

          Is that preferred in Moodle over
          "{$CFG->tempdir}/backup/{$this->tempdir}/moddata/unittest/4/icon.gif"
          ?

          Cheers,
          Aaron

          Show
          sry_not4sale Aaron Barnes added a comment - Hi Aprarup, Is that preferred in Moodle over "{$CFG->tempdir}/backup/{$this->tempdir}/moddata/unittest/4/icon.gif" ? Cheers, Aaron
          Hide
          nebgor Aparup Banerjee added a comment -

          personally i like the dots - it helps clarity for me and we can always put long strings on multiple lines, but i don't think theres anything stopping curly brace uses

          The coding style says http://docs.moodle.org/dev/Coding_style#String_concatenation

          ps: (a little above there is a use of the {$name} which is a little confusing too)

          Show
          nebgor Aparup Banerjee added a comment - personally i like the dots - it helps clarity for me and we can always put long strings on multiple lines, but i don't think theres anything stopping curly brace uses The coding style says http://docs.moodle.org/dev/Coding_style#String_concatenation ps: (a little above there is a use of the {$name} which is a little confusing too)
          Hide
          sry_not4sale Aaron Barnes added a comment -

          Updated patch

          Show
          sry_not4sale Aaron Barnes added a comment - Updated patch
          Hide
          nebgor Aparup Banerjee added a comment -

          looked good, still does! sending it up for integration.

          Show
          nebgor Aparup Banerjee added a comment - looked good, still does! sending it up for integration.
          Hide
          samhemelryk Sam Hemelryk added a comment -

          Thanks guys, this has been integrated now

          Show
          samhemelryk Sam Hemelryk added a comment - Thanks guys, this has been integrated now
          Hide
          rajeshtaneja Rajesh Taneja added a comment -

          Test pass after replacing icon.gif

          Thanks for fixing this, Aaron.

          Show
          rajeshtaneja Rajesh Taneja added a comment - Test pass after replacing icon.gif Thanks for fixing this, Aaron.
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          U P S T R E A M I Z E D !

          Many thanks for the hard work, closing this as fixed.

          Ciao

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - U P S T R E A M I Z E D ! Many thanks for the hard work, closing this as fixed. Ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved:
                Fix Release Date:
                25/Jun/12