UserPictureTestCase::testWithGDinvalidSize could return a picture that was smaller than the maximum size set in the test case.

given that it expects the image upload to fail because the image is bigger than the max size, i was getting failures on a clean HEAD install.

the patch makes sure that the tested file is bigger than the maxsize in the test case. also, fixes up a code style problem and updates the phpdoc to reflect the test results.

CommentFileSizeAuthor
#3 user_upload_test.patch1.91 KBAnonymous (not verified)
user_upload_test.patch2.4 KBAnonymous (not verified)

Comments

Status: Needs review » Needs work

The last submitted patch failed testing.

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs work » Needs review

woops, that was buggy, better one coming soon.

Anonymous’s picture

Anonymous (not verified) commented
StatusFileSize
new user_upload_test.patch1.91 KB

fixed the bug in my last patch.

c960657’s picture

c960657 commented

drupalGetTestFiles() has an argument for requesting files of a specific size. Perhaps that would be cleaner?

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs review » Needs work

Yes, that sounds much better - will reroll shortly

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs work » Needs review

hmm, the second param to drupalGetTestFiles requires knowing the size in bytes of the file that you want returned, which feels a bit too fragile to me, so i think i'll leave it as is.

drewish’s picture

drewish commented

subscribing.

Anonymous’s picture

Anonymous (not verified) commented

#391210: add drupalGetTestFile method to drupal_web_test_case just noting this related issue i opened.

drewish’s picture

drewish commented

i think that:

-        $image = current($this->drupalGetTestFiles('image'));
+        // Images are sorted first by size then by name. We need an image
+        // bigger than 1 KB so we'll grab the last one.
+        $image = end($this->drupalGetTestFiles('image'));

might be a much simpler way to achieve this.

c960657’s picture

c960657 commented

I added drewish's suggested fix to the patch for a related issue, #395228: Wrong sorting in drupalGetTestFiles().

c960657’s picture

c960657 commented
Status: Needs review » Fixed

The patch for #395228: Wrong sorting in drupalGetTestFiles() was just committed. I believe this also fixes this issue.

Anonymous’s picture

Anonymous (not verified) commented

fair enough, though i still think this is brittle. i don't think we should have tests rely on the sort order of the test files. if that breaks/changes, then tests can break again.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.