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

          Attachments

            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