Moodle
  1. Moodle
  2. MDL-36447

test_get_file_preview depends on GD, but GD is not a moodle requirement

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3.2
    • Fix Version/s: 2.3.4, 2.4.1
    • Component/s: Unit tests
    • Labels:
      None
    • Rank:
      45278

      Description

      When running unit tests without the gd extension, you'll get this failure:

      1) filestoragelib_testcase::test_get_file_preview
      Failed asserting that false is an instance of class "stored_file".

      This is because generate_image_thumbnail will return false if gd is not installed.

      We should either:

      • Make GD a requirement or
      • Make the test skip when GD is not installed

        Activity

        Hide
        Dan Poltawski added a comment -

        Assigning to David as I believe he introduced the test.

        Show
        Dan Poltawski added a comment - Assigning to David as I believe he introduced the test.
        Hide
        Dan Poltawski added a comment -

        Assing Mark as a watcher as he experienced this problem.

        Show
        Dan Poltawski added a comment - Assing Mark as a watcher as he experienced this problem.
        Hide
        Mark Nelson added a comment -

        I don't really like the sound of 'assing' me as anything tbh.

        Show
        Mark Nelson added a comment - I don't really like the sound of 'assing' me as anything tbh.
        Hide
        Dan Poltawski added a comment -

        lol.

        Show
        Dan Poltawski added a comment - lol.
        Hide
        Petr Škoda added a comment -

        Thanks for the report.

        Show
        Petr Škoda added a comment - Thanks for the report.
        Hide
        Dan Poltawski added a comment -

        Hmm, theoretically test_get_file_preview_nonimage() could be argued to be skipped too.

        Show
        Dan Poltawski added a comment - Hmm, theoretically test_get_file_preview_nonimage() could be argued to be skipped too.
        Hide
        Petr Škoda added a comment -

        Hmm, but it did not seem to cause any problems, so why skip it?

        Show
        Petr Škoda added a comment - Hmm, but it did not seem to cause any problems, so why skip it?
        Hide
        Dan Poltawski added a comment -

        Cos the test is giving a false sense of security, i'm not too bothered either way, it just does not seem correct to be stesting something which is not testing what the test looks to be testing.

        Show
        Dan Poltawski added a comment - Cos the test is giving a false sense of security, i'm not too bothered either way, it just does not seem correct to be stesting something which is not testing what the test looks to be testing.
        Hide
        Dan Poltawski added a comment -

        I've integrated it as it is, since I don't think its a major issue, if even an issue at all!

        Show
        Dan Poltawski added a comment - I've integrated it as it is, since I don't think its a major issue, if even an issue at all!
        Hide
        Dan Poltawski added a comment -

        It works (I almost forgot to reset the phpunit env)

        Show
        Dan Poltawski added a comment - It works (I almost forgot to reset the phpunit env)
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Many thanks for your effort, the whole Moodle Community will be enjoying your great solutions starting now!

        Closing, ciao

        Show
        Eloy Lafuente (stronk7) added a comment - Many thanks for your effort, the whole Moodle Community will be enjoying your great solutions starting now! Closing, ciao

          People

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

            Dates

            • Created:
              Updated:
              Resolved: