image scaling on story's image field fails

Encountered the same error, applied the pach, error gone. RTBC since I trust catch and john to write the right code :)

Committed to CVS HEAD, but this should have been testing. Setting to 'needs work' for tests.

Here's a test.

String translate, search and validate (LocaleTranslationFunctionalTest) [Locale] 314 2 0
Message Group Filename Line Function Status
JavaScript file created: not found Other locale.test 347 LocaleTranslationFunctionalTest->testJavaScriptTranslation()
JavaScript file rebuilt: not found Other locale.test 354 LocaleTranslationFunctionalTest->testJavaScriptTranslation()

This has to be fixed o_0....
This isn't related at all with the patch...
I don't know s*** about the test bot, so i can't give suggestions

Tests are not critical. Please read and understand Priority levels of Issues, thanks.

Would be great to get this sucker in though. :)

+++ modules/image/image.test	8 Jan 2010 23:15:08 -0000
@@ -505,3 +505,36 @@ class ImageAdminStylesUnitTest extends D
+/**
+ * This class provides methods specifically for testing images in the default article content type
+ */

This breaks the 80 character wall. The first line should be a short (<77 characters) summary, which could be followed be a more in-depth explanation.

+++ modules/image/image.test	8 Jan 2010 23:15:08 -0000
@@ -505,3 +505,36 @@ class ImageAdminStylesUnitTest extends D
+   * Test image is added and displayed correctly on an Article

Nit-pick: We want these descriptions to end with a period. :)

That's really all I can point at. I think I get the test, and I don't think there's anything wrong with it - but it still needs to be tested on an install without the #1 patch (patch -R), to make sure it the test cries error (if it doesn't, the test doesn't test for the thing the patch patched). I'd be happy to RTBC with proper Doxygen and the bot's "yay" though, but having tested without the patch would be a huge bonus.

Powered by Dreditor.

Only fixing the 80 char line and the period pointed out in #9

#10: patch.patch queued for re-testing.

Fixed the PHP error (by renaming the class), optimized, and updated to test the same way file.test does.

This really just needs a review to see if the test fails without the patch (from #1), as the testing bot takes care to test it against HEAD. No one wants to do this?

+++ modules/image/image.test	5 Mar 2010 02:18:13 -0000
@@ -789,3 +789,35 @@ class ImageFieldValidateTestCase extends
+ * Provide methods for testing images in the default article content type

Should also have an trailing period. :)

+++ modules/image/image.test	5 Mar 2010 02:18:13 -0000
@@ -789,3 +789,35 @@ class ImageFieldValidateTestCase extends
+class DefaultImageFieldTestCase extends FileFieldTestCase {
+ ¶
+  public static function getInfo() {
+    return array(
+      'name' => 'Default Image field test',
+      'description' => 'Test creating an Image with the default Article image field.',
...
+ ¶
+  /**

Stray whitespaces.

148 critical left. Go review some!

Here's the fixes.

I tried to get a checkout as of 2009-11-24 for running the test w/o the patch, but could not get it to bootstrap....

I was thinking more of something like:

wget http://drupal.org/files/issues/image-effect-apply-641750-1.patch
patch -p0 -R image-effect-apply-641750-1.patch

That way, the rest of the code should also be up-to-date. Of course, if anything has been applied "on top of" that patch, hunks might fail in reversing (-R) the patch.

I believe the tests added here cover this as well. If not let me know and I can help get this sorted out. #601966: Tests for image field, specifically testImageFieldFormatters() which uploads an image to the article content type and tests the various image field formatters.

#1: image-effect-apply-641750-1.patch queued for re-testing.

#1: image-effect-apply-641750-1.patch queued for re-testing.

I have reviewed the code and the test has passed.

http://drupal.org/files/issues/image-effect-apply-641750-1.patch tested and passed review by the_paulus in #23 setting status to reviewed & tested by the community.

#1: image-effect-apply-641750-1.patch queued for re-testing.

Feel that I should add that I too reviewed this and it passed all image tests.

I'm a little bit lost and not sure what is being tested/reviewed here.

If we are still talking about the tests in #17, or #4 I don't think these are necessary anymore. We added a set of almost identical tests in #601966: Tests for image field, which not only check that the image is displayed using the default formatter but all the other formatters as well.

See function testImageFieldFormatters() in modules/image/image.test

    // Create a new node with an image attached.
    $test_image = current($this->drupalGetTestFiles('image'));
    $nid = $this->uploadNodeImage($test_image, $field_name, 'article');
    $node = node_load($nid, NULL, TRUE);

    // Test that the default formatter is being used.
    $image_uri = $node->{$field_name}[LANGUAGE_NONE][0]['uri'];
    $image_info = array(
      'path' => $image_uri,
      'getsize' => TRUE,
    );
    $default_output = theme('image', $image_info);
    $this->assertRaw($default_output, t('Default formatter displaying correctly on full node view.'));

I think it is safe to say that the testing portion of this issue is covered, and if that's the only reason this issue is still open we should close it.

#17: article_image_test_02.patch queued for re-testing.

#17: article_image_test_02.patch queued for re-testing.

Based on #27, it feels like this needs more discussion.

Based on #1, #2 and #3 and given that the test is no longer necessary because of #27 I think we can safely commit & close this.

Yeah. I agree that this issue can probably be closed. The code to fix the original issue was already committed, see comment #3, and test coverage for this was added in another issue. #601966: Tests for image field

Closing as fixed.