Merge pull request #7635 from ampproject/fix/admin-bar-styles

Fix deprecated `_admin_bar_bump_cb`; Fix failing PHP unit tests
ampproject · Oct 10, 2023 · 92538fb · 92538fb
2 parents cb0feea + 82fe088
commit 92538fb
Show file tree

Hide file tree

Showing 9 changed files with 157 additions and 125 deletions.
diff --git a/.github/workflows/build-test-measure.yml b/.github/workflows/build-test-measure.yml
@@ -383,6 +383,8 @@ jobs:
     needs: pre-run
     env:
       WP_CORE_DIR: /tmp/wordpress
+      WP_TESTS_DIR: /tmp/wordpress-tests-lib
+      WP_ENVIRONMENT_TYPE: local
     services:
       mysql:
         image: mariadb:latest
@@ -544,18 +546,24 @@ jobs:
         if: needs.pre-run.outputs.changed-php-count > 0
         run: cp -r "$PWD" "$WP_CORE_DIR/src/wp-content/plugins/amp"
 
-      - name: Setup required WP constants
+      - name: Override default PHPUnit configuration
+        if: ${{ matrix.experimental == true && needs.pre-run.outputs.changed-php-count > 0 }}
         run: |
-          echo "WP_ENVIRONMENT_TYPE=local" >> $GITHUB_ENV
+          cp phpunit.xml.dist phpunit.xml
+
+          # Avoid converting deprecations, errors, notices, and warnings to exceptions in experimental mode.
+          sed -i 's/convertDeprecationsToExceptions="true"/convertDeprecationsToExceptions="false"/g' phpunit.xml
+          sed -i 's/convertErrorsToExceptions="true"/convertErrorsToExceptions="false"/g' phpunit.xml
+          sed -i 's/convertNoticesToExceptions="true"/convertNoticesToExceptions="false"/g' phpunit.xml
+          sed -i 's/convertWarningsToExceptions="true"/convertWarningsToExceptions="false"/g' phpunit.xml
+        working-directory: ${{ env.WP_CORE_DIR }}/src/wp-content/plugins/amp
 
       - name: Run tests
         if: ${{ matrix.coverage == false && needs.pre-run.outputs.changed-php-count > 0 }}
         run: |
             phpunit --version
             phpunit --verbose
         working-directory: ${{ env.WP_CORE_DIR }}/src/wp-content/plugins/amp
-        env:
-          WP_TESTS_DIR: /tmp/wordpress-tests-lib
 
       - name: Run multisite tests
         if: ${{ matrix.multisite == true && needs.pre-run.outputs.changed-php-count > 0 }}
@@ -564,7 +572,6 @@ jobs:
           phpunit --verbose
         working-directory: ${{ env.WP_CORE_DIR }}/src/wp-content/plugins/amp
         env:
-          WP_TESTS_DIR: /tmp/wordpress-tests-lib
           WP_MULTISITE: 1
 
       - name: Run tests with coverage
@@ -573,17 +580,13 @@ jobs:
           phpunit --version
           phpunit --verbose --coverage-clover ${{ env.WP_CORE_DIR }}/src/wp-content/plugins/amp/build/logs/clover.xml
         working-directory: ${{ env.WP_CORE_DIR }}/src/wp-content/plugins/amp
-        env:
-          WP_TESTS_DIR: /tmp/wordpress-tests-lib
 
       - name: Run external HTTP tests
         if: ${{ matrix.external-http == true && needs.pre-run.outputs.changed-php-count > 0 }}
         run: |
           phpunit --version
           phpunit --testsuite external-http --verbose
         working-directory: ${{ env.WP_CORE_DIR }}/src/wp-content/plugins/amp
-        env:
-          WP_TESTS_DIR: /tmp/wordpress-tests-lib
 
       - name: Upload code coverage report
         if: ${{ matrix.coverage == true && needs.pre-run.outputs.changed-php-count > 0 && github.actor != 'dependabot[bot]' }}

diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php
@@ -1225,12 +1225,21 @@ public static function init_admin_bar() {
 		} else {
 			$header_callback = '_admin_bar_bump_cb';
 		}
-		remove_action( 'wp_head', $header_callback );
+
+		// @see <https://core.trac.wordpress.org/ticket/58775>.
+		if ( ! function_exists( 'wp_enqueue_admin_bar_header_styles' ) ) {
+			remove_action( 'wp_head', $header_callback );
+		}
+
 		if ( '__return_false' !== $header_callback ) {
-			ob_start();
-			$header_callback();
-			$style = ob_get_clean();
-			$data  = trim( preg_replace( '#<style[^>]*>(.*)</style>#is', '$1', $style ) ); // See wp_add_inline_style().
+			if ( ! function_exists( 'wp_enqueue_admin_bar_header_styles' ) ) {
+				ob_start();
+				$header_callback();
+				$style = ob_get_clean();
+				$data  = trim( preg_replace( '#<style[^>]*>(.*)</style>#is', '$1', $style ) ); // See wp_add_inline_style().
+			} else {
+				$data = '';
+			}
 
 			// Override AMP's position:relative on the body for the sake of the AMP viewer, which is not relevant an an Admin Bar context.
 			if ( amp_is_dev_mode() ) {

diff --git a/src/Support/SupportData.php b/src/Support/SupportData.php
@@ -251,7 +251,10 @@ public function get_site_info() {
 			$loopback_status = ( ! empty( $loopback_status->status ) ) ? $loopback_status->status : '';
 		}
 
-		if ( function_exists( 'wp_is_https_supported' ) ) {
+		if ( function_exists( 'wp_get_https_detection_errors' ) ) {
+			$https_errors = wp_get_https_detection_errors();
+			$is_ssl       = empty( $https_errors );
+		} elseif ( function_exists( 'wp_is_https_supported' ) ) {
 			$is_ssl = wp_is_https_supported();
 		} else {
 			$is_ssl = is_ssl();

diff --git a/tests/php/src/DevTools/LikelyCulpritDetectorTest.php b/tests/php/src/DevTools/LikelyCulpritDetectorTest.php
@@ -128,13 +128,13 @@ public function single_step_trace_data() {
 			],
 
 			'parent theme' => [
-				[ get_template_directory() . '/functions.php' ],
+				[ '{{ get_template_directory }}/functions.php' ],
 				'theme',
 				'default',
 			],
 
 			'child theme'  => [
-				[ get_stylesheet_directory() . '/functions.php' ],
+				[ '{{ get_stylesheet_directory }}/functions.php' ],
 				'theme',
 				'default',
 			],
@@ -167,8 +167,8 @@ public function multi_step_trace_data() {
 						__FILE__, // AMP plugin is skipped.
 						WP_PLUGIN_DIR . '/bad-plugin/bad-plugin.php', // <== Likely culprit.
 						WP_CONTENT_DIR . '/mu-plugins/bad-mu-plugin.php',
-						get_template_directory() . '/functions.php',
-						get_stylesheet_directory() . '/functions.php',
+						'{{ get_template_directory }}/functions.php',
+						'{{ get_stylesheet_directory }}/functions.php',
 					],
 					'plugin',
 					'bad-plugin',
@@ -180,8 +180,8 @@ public function multi_step_trace_data() {
 						__FILE__, // AMP plugin is skipped.
 						WP_CONTENT_DIR . '/mu-plugins/bad-mu-plugin.php', // <== Likely culprit.
 						WP_PLUGIN_DIR . '/bad-plugin/bad-plugin.php',
-						get_template_directory() . '/functions.php',
-						get_stylesheet_directory() . '/functions.php',
+						'{{ get_template_directory }}/functions.php',
+						'{{ get_stylesheet_directory }}/functions.php',
 					],
 					'mu-plugin',
 					'bad-mu-plugin.php',
@@ -191,10 +191,10 @@ public function multi_step_trace_data() {
 					[
 						ABSPATH . '/wp-includes/some-file.php', // Core is skipped.
 						__FILE__, // AMP plugin is skipped.
-						get_template_directory() . '/functions.php', // <== Likely culprit.
+						'{{ get_template_directory }}/functions.php', // <== Likely culprit.
 						WP_PLUGIN_DIR . '/bad-plugin/bad-plugin.php',
 						WP_CONTENT_DIR . '/mu-plugins/bad-mu-plugin.php',
-						get_stylesheet_directory() . '/functions.php',
+						'{{ get_stylesheet_directory }}/functions.php',
 					],
 					'theme',
 					'default',
@@ -204,10 +204,10 @@ public function multi_step_trace_data() {
 					[
 						ABSPATH . '/wp-includes/some-file.php', // Core is skipped.
 						__FILE__, // AMP plugin is skipped.
-						get_stylesheet_directory() . '/functions.php', // <== Likely culprit.
+						'{{ get_stylesheet_directory }}/functions.php', // <== Likely culprit.
 						WP_PLUGIN_DIR . '/bad-plugin/bad-plugin.php',
 						WP_CONTENT_DIR . '/mu-plugins/bad-mu-plugin.php',
-						get_template_directory() . '/functions.php',
+						'{{ get_template_directory }}/functions.php',
 					],
 					'theme',
 					'default',
@@ -275,7 +275,12 @@ public function test_analyze_trace( $file_stack, $expected_type, $expected_name
 	private function get_trace_from_file_stack( $file_stack ) {
 		return array_map(
 			static function ( $file ) {
-				return [ 'file' => $file ];
+				$file = str_replace( '{{ get_template_directory }}', get_template_directory(), $file );
+				$file = str_replace( '{{ get_stylesheet_directory }}', get_stylesheet_directory(), $file );
+
+				return [
+					'file' => $file,
+				];
 			},
 			$file_stack
 		);