PHPUnit suite in trunk does not pass currently: 3 tests with extra sizes="100vw" in output

[ottok]

Current WordPress development trunk currently fails with the following 3 failures:

There were 3 failures:

1) Tests_Media::test_wp_get_attachment_image_defaults
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'<img width="150" height="150" src="http://example.org/wp-content/uploads/2016/10/test-image-large-150x150.png" class="attachment-thumbnail size-thumbnail" alt="" />'
+'<img width="150" height="150" src="http://example.org/wp-content/uploads/2016/10/test-image-large-150x150.png" class="attachment-thumbnail size-thumbnail" alt="" sizes="100vw" />'

/vagrant/www/wordpress-develop/tests/phpunit/tests/media.php:932

2) Tests_Media::test_wp_get_attachment_image_with_alt
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'<img width="150" height="150" src="http://example.org/wp-content/uploads/2016/10/test-image-large-150x150.png" class="attachment-thumbnail size-thumbnail" alt="Some very clever alt text" />'
+'<img width="150" height="150" src="http://example.org/wp-content/uploads/2016/10/test-image-large-150x150.png" class="attachment-thumbnail size-thumbnail" alt="Some very clever alt text" sizes="100vw" />'

/vagrant/www/wordpress-develop/tests/phpunit/tests/media.php:946

3) Tests_Media::test_wp_get_attachment_image_should_use_wp_get_attachment_metadata
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-<img width="999" height="999" src="http://example.org/wp-content/uploads/2016/10/test-image-testsize-999x999.png" class="attachment-testsize size-testsize" alt="" srcset="http://example.org/wp-content/uploads/2016/10/test-image-testsize-999x999.png 999w, http://example.org/wp-content/uploads/2016/10/test-image-large-150x150.png 150w" sizes="(max-width: 999px) 100vw, 999px" />
+<img width="999" height="999" src="http://example.org/wp-content/uploads/2016/10/test-image-testsize-999x999.png" class="attachment-testsize size-testsize" alt="" srcset="http://example.org/wp-content/uploads/2016/10/test-image-testsize-999x999.png 999w, http://example.org/wp-content/uploads/2016/10/test-image-large-150x150.png 150w" sizes="100vw" />

/vagrant/www/wordpress-develop/tests/phpunit/tests/media.php:1852

These are all related to the same thing. There is an extra string in the output:

sizes="100vw"

This sizes-string seem to stem from the 2017 theme. Removing that theme removes the test failures (de-activating the theme was not enough!).

Below is the code block that introduces this string. I don't know what is the proper fix, but I am happy to take a shot if somebody gives me some guidance. (I am a PHP programmer.)

/**
 * Add custom image sizes attribute to enhance responsive image functionality
 * for post thumbnails.
 *
 * @since Twenty Seventeen 1.0
 *
 * @param array $attr       Attributes for the image markup.
 * @param int   $attachment Image attachment ID.
 * @param array $size       Registered image size or flat array of height and width dimensions.
 * @return string A source size value for use in a post thumbnail 'sizes' attribute.
 */
function twentyseventeen_post_thumbnail_sizes_attr( $attr, $attachment, $size ) {
	if ( is_archive() || is_search() || is_home() ) {
		$attr['sizes'] = '(max-width: 767px) 89vw, (max-width: 1000px) 54vw, (max-width: 1071px) 543px, 580px';
	} else {
		$attr['sizes'] = '100vw';
	}

	return $attr;
}
add_filter( 'wp_get_attachment_image_attributes', 'twentyseventeen_post_thumbnail_sizes_attr', 10, 3 );

Failing tests:

	/**
	 * Tests the default output of `wp_get_attachment_image()`.
	 * @ticket 34635
	 */
	function test_wp_get_attachment_image_defaults() {
		$image = image_downsize( self::$large_id, 'thumbnail' );
		$expected = sprintf( '<img width="%1$d" height="%2$d" src="%3$s" class="attachment-thumbnail size-thumbnail" alt="" />', $image[1], $image[2], $image[0] );

		$this->assertEquals( $expected, wp_get_attachment_image( self::$large_id ) );
	}

	/**
	 * Test that `wp_get_attachment_image()` returns a proper alt value.
	 * @ticket 34635
	 */
	function test_wp_get_attachment_image_with_alt() {
		// Add test alt metadata.
		update_post_meta( self::$large_id, '_wp_attachment_image_alt', 'Some very clever alt text', true );

		$image = image_downsize( self::$large_id, 'thumbnail' );
		$expected = sprintf( '<img width="%1$d" height="%2$d" src="%3$s" class="attachment-thumbnail size-thumbnail" alt="Some very clever alt text" />', $image[1], $image[2], $image[0] );

		$this->assertEquals( $expected, wp_get_attachment_image( self::$large_id ) );

		// Cleanup.
		update_post_meta( self::$large_id, '_wp_attachment_image_alt', '', true );
	}

	/**
	 * Tests if wp_get_attachment_image() uses wp_get_attachment_metadata().
	 *
	 * In this way, the meta data can be filtered using the filter
	 * `wp_get_attachment_metadata`.
	 *
	 * The test checks if the image size that is added in the filter is
	 * used in the output of `wp_get_attachment_image()`.
	 *
	 * @ticket 36246
	 */
	function test_wp_get_attachment_image_should_use_wp_get_attachment_metadata() {
		add_filter( 'wp_get_attachment_metadata', array( $this, '_filter_36246' ), 10, 2 );

		remove_all_filters( 'wp_calculate_image_sizes' );

		$actual = wp_get_attachment_image( self::$large_id, 'testsize' );
		$year = date( 'Y' );
		$month = date( 'm' );

		$expected = '<img width="999" height="999" src="http://' . WP_TESTS_DOMAIN . '/wp-content/uploads/' . $year . '/' . $month . '/test-image-testsize-999x999.png"' .
			' class="attachment-testsize size-testsize" alt=""' .
			' srcset="http://' . WP_TESTS_DOMAIN . '/wp-content/uploads/' . $year . '/' . $month . '/test-image-testsize-999x999.png 999w,' .
				' http://' . WP_TESTS_DOMAIN . '/wp-content/uploads/' . $year . '/' . $month . '/test-image-large-150x150.png 150w"' .
				' sizes="(max-width: 999px) 100vw, 999px" />';

		remove_filter( 'wp_get_attachment_metadata', array( $this, '_filter_36246' ) );

		$this->assertSame( $expected, $actual );
	}

[ocean90]

Hello @ottok,

this shouldn't happen anymore since [38858], see also ​https://travis-ci.org/aaronjorbin/develop.wordpress/builds.

Can you make sure that you have set define( 'WP_DEFAULT_THEME', 'default' ); in your wp-tests-config.php file?

[ottok]

@ocean90 Yes, I do have. This is a fresh checkout of the svn repo in a fresh VVV environment. I shouldn't be seeing this failures but I am.

The bug you reference has not been created yet..

[ocean90]

Replying to ottok:

The bug you reference has not been created yet..

Sorry, it's a changeset: [38858]

[ottok]

@ocean90 Exactly the same commmit seems to pass on Travis-CI but fail on a pristine VVV (latest development version): ​https://travis-ci.org/ottok/WordPress-develop/branches

[peterwilsoncc]

I was having the same problems and started relying on my Travis-CI instance for tests.

inside vvv

Once before and once after phpunit

PHP Warning:  symlink(): No such file or directory in /vagrant/www/wordpress-develop/tests/phpunit/includes/functions.php on line 170

During tests, many errors similar to

Creating default object from empty value

/vagrant/www/wordpress-develop/src/wp-content/themes/twentyseventeen/inc/customizer.php:16
/vagrant/www/wordpress-develop/src/wp-includes/class-wp-hook.php:298
/vagrant/www/wordpress-develop/src/wp-includes/class-wp-hook.php:323
/vagrant/www/wordpress-develop/src/wp-includes/plugin.php:453
/vagrant/www/wordpress-develop/tests/phpunit/tests/customize/widgets.php:512

These errors are causing those described above.

$ php -v
PHP 7.0.9-1+deb.sury.org~trusty+1 (cli) ( NTS )
Copyright (c) 1997-2016 The PHP Group
Zend Engine v3.0.0, Copyright (c) 1998-2016 Zend Technologies
    with Zend OPcache v7.0.9-1+deb.sury.org~trusty+1, Copyright (c) 1999-2016, by Zend Technologies
$ phpunit --version
PHPUnit 4.8.27 by Sebastian Bergmann and contributors.

---

Outside vvv

I don't get the symlink error, I do get the followup error.

$ php -v
PHP 7.0.12 (cli) (built: Oct 14 2016 09:55:03) ( NTS )
Copyright (c) 1997-2016 The PHP Group
Zend Engine v3.0.0, Copyright (c) 1998-2016 Zend Technologies
$ phpunit --version
PHPUnit 5.4.4 by Sebastian Bergmann and contributors.

cc @pento

[pento]

I suspect it's being caused by symlink() either not finding the themes directory, or not have write permission to it. The former could be fixed, but the latter cannot, so let's try a different strategy. :-)

@ottok, @peterwilsoncc: Could I get you both to test 38457.diff​?

[peterwilsoncc]

@pento

The symlink errors have disappeared.

I'm still getting the other errors and failures both inside and outside of VVV.

I'll see what I can track down with alternative configs.

[pento]

Ugh.

Some debugging ideas that helped while I was building this patch:

• Does vagrant provision help? That'll get everything up to date within VVV.
• In phpunit/includes/bootstrap.php, after wp-settings.php is included, what's the state of $wp_theme_directories and wp_get_theme().
• Does $wp_theme_directories contain the correct path to themedir1? In VVV, it should be something like /srv/www/wordpress-develop/tests/phpunit/includes/../data/themedir1.

[ocean90]

Replying to peterwilsoncc:

I'm still getting the other errors and failures both inside and outside of VVV.

See comment:1:

Can you make sure that you have set define( 'WP_DEFAULT_THEME', 'default' ); in your wp-tests-config.php file?

[peterwilsoncc]

Replying to ocean90:

See comment:1:

Can you make sure that you have set define( 'WP_DEFAULT_THEME', 'default' ); in your wp-tests-config.php file?

You're right, I'd missed that. Thanks @ocean90, sorry Pento.

Removing the symlink in 38457.diff​ is proving more consistent across various phpunit versions.

[pento]

In 38907:

Tests: Use a minimal theme for tests.

This functionality was originally added in [38858], using symlink() to put a link to the theme in WordPress' themes directory. Unfortunately, not all installs have write access to the themes directory, causing unit tests to fail.

The new method is to add the test theme directory to $wp_theme_directories, and fix the handful of tests that don't expect $wp_theme_directories to have multiple entries.

The test install/bootstrap routines now also check that WP_DEFAULT_THEME is defined, in case the tests are being run on a system that hasn't upgraded its' wp-tests-config.php.

See #31550.
Fixes #38457.

[pento]

In 38908:

Tests: Fix a PHP notice introduced in [38907].

WP_DEFAULT_THEME was being defined in the wrong location.

Also, if WordPress has the original "default" theme installed, ensure that our test theme overrides it.

Props swissspidy for daring to dive into Themes of WordPress Past.
See #31550, #38457.

[ottok]

I confirm this is fixed and all tests on trunk pass in a fresh VVV.