Improve testing of finish_init; add test stubs

* Clean up covers tags * Remove unnecessary is_template_support_required
ampproject · Jul 2, 2018 · 221c3f0 · 221c3f0
1 parent 1c759e5
commit 221c3f0
Show file tree

Hide file tree

Showing 5 changed files with 68 additions and 32 deletions.
diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php
@@ -309,10 +309,12 @@ public static function ensure_proper_amp_location( $exit = true ) {
 				$new_url = add_query_arg( amp_get_slug(), '', amp_remove_endpoint( $old_url ) );
 				if ( $old_url !== $new_url ) {
 					wp_safe_redirect( $new_url, 302 );
+					// @codeCoverageIgnoreStart
 					if ( $exit ) {
 						exit;
 					}
 					return true;
+					// @codeCoverageIgnoreEnd
 				}
 			}
 		}
@@ -343,10 +345,12 @@ public static function redirect_ampless_url( $exit = true ) {
 		 * occur or when a non-canonical AMP theme is used.
 		 */
 		wp_safe_redirect( $ampless_url, 302 );
+		// @codeCoverageIgnoreStart
 		if ( $exit ) {
 			exit;
 		}
 		return true;
+		// @codeCoverageIgnoreEnd
 	}
 
 	/**
@@ -394,17 +398,6 @@ public static function add_amp_template_filters() {
 		}
 	}
 
-	/**
-	 * Return whether support for all templates is required by the theme.
-	 *
-	 * @since 1.0
-	 * @return bool Whether theme requires all templates to support AMP.
-	 */
-	public static function is_template_support_required() {
-		$theme_support_args = self::get_theme_support_args( array( 'initial' => true ) );
-		return ( isset( $theme_support_args['templates_supported'] ) && 'all' === $theme_support_args['templates_supported'] );
-	}
-
 	/**
 	 * Determine template availability of AMP for the given query.
 	 *
@@ -475,7 +468,7 @@ public static function get_template_availability( $query = null ) {
 		$all_templates_supported          = (
 			$all_templates_supported_by_theme_support || AMP_Options_Manager::get_option( 'all_templates_supported' )
 		);
-		$unrecognized_templates_supported = (
+		$unrecognized_templates_supported = ( // @todo Get this from $supportable_templates.
 			isset( $theme_templates_supported['unrecognized'] ) ? true === $theme_templates_supported['unrecognized'] : AMP_Options_Manager::get_option( 'unrecognized_templates_supported' )
 		);
 

diff --git a/includes/options/class-amp-options-manager.php b/includes/options/class-amp-options-manager.php
@@ -124,7 +124,9 @@ public static function validate_options( $new_options ) {
 		$options['disable_admin_bar']   = ! empty( $new_options['disable_admin_bar'] );
 
 		$theme_support_args = AMP_Theme_Support::get_theme_support_args( array( 'initial' => true ) );
-		if ( ! AMP_Theme_Support::is_template_support_required() ) {
+
+		$is_template_support_required = ( isset( $theme_support_args['templates_supported'] ) && 'all' === $theme_support_args['templates_supported'] );
+		if ( ! $is_template_support_required ) {
 			$options['all_templates_supported'] = ! empty( $new_options['all_templates_supported'] );
 			if ( ! isset( $theme_support_args['templates_supported']['unrecognized'] ) ) {
 				$options['unrecognized_templates_supported'] = ! empty( $new_options['unrecognized_templates_supported'] );

diff --git a/includes/options/class-amp-options-menu.php b/includes/options/class-amp-options-menu.php
@@ -290,7 +290,7 @@ public function render_supported_templates() {
 
 		?>
 		<fieldset id="all_templates_supported_fieldset">
-			<?php if ( AMP_Theme_Support::is_template_support_required() ) : ?>
+			<?php if ( isset( $theme_support_args['templates_supported'] ) && 'all' === $theme_support_args['templates_supported'] ) : ?>
 				<div class="notice notice-info notice-alt inline">
 					<p>
 						<?php esc_html_e( 'The current theme requires all templates to support AMP.', 'amp' ); ?>

diff --git a/tests/test-class-amp-base-sanitizer.php b/tests/test-class-amp-base-sanitizer.php
@@ -108,7 +108,7 @@ public function get_data() {
 	 * @param array $source_attributes   Source Attrs.
 	 * @param array $expected_attributes Expected Attrs.
 	 * @param array $args                Args.
-	 * @covers AMP_Base_Sanitizer::enforce_fixed_height()
+	 * @covers AMP_Base_Sanitizer::set_layout()
 	 */
 	public function test_set_layout( $source_attributes, $expected_attributes, $args = array() ) {
 		$sanitizer           = new AMP_Test_Stub_Sanitizer( new DOMDocument(), $args );
@@ -316,7 +316,7 @@ public function test_filter_data_amp_attributes() {
 	/**
 	 * Tests set_attachment_layout_attributes.
 	 *
-	 * @covers AMP_Base_Sanitizer::set_attachment_layout_attributes()
+	 * @covers AMP_Base_Sanitizer::filter_attachment_layout_attributes()
 	 */
 	public function test_filter_attachment_layout_attributes() {
 		$sanitizer    = new AMP_Test_Stub_Sanitizer( new DOMDocument(), array() );

diff --git a/tests/test-class-amp-theme-support.php b/tests/test-class-amp-theme-support.php
@@ -57,7 +57,6 @@ public function tearDown() {
 	 * Test init.
 	 *
 	 * @covers AMP_Theme_Support::init()
-	 * @covers AMP_Theme_Support::finish_init()
 	 */
 	public function test_init() {
 		$_REQUEST['__amp_source_origin'] = 'foo';
@@ -146,38 +145,43 @@ public function test_read_theme_support_and_support_args() {
 	}
 
 	/**
-	 * Test that amphtml link is added at the right time.
+	 * Test that init finalization, including amphtml link is added at the right time.
 	 *
 	 * @covers AMP_Theme_Support::finish_init()
 	 */
-	public function test_amphtml_link() {
+	public function test_finish_init() {
 		$post_id = $this->factory()->post->create( array( 'post_title' => 'Test' ) );
 		add_theme_support( 'amp', array(
-			'template_dir'       => '...',
-			'available_callback' => function() {
-				return is_singular();
-			},
+			'mode'         => 'paired',
+			'template_dir' => 'amp',
 		) );
 
-		// Test paired mode singular.
+		// Test paired mode singular, where not on endpoint that it causes amphtml link to be added.
 		remove_action( 'wp_head', 'amp_add_amphtml_link' );
 		$this->go_to( get_permalink( $post_id ) );
+		$this->assertFalse( is_amp_endpoint() );
 		AMP_Theme_Support::finish_init();
 		$this->assertEquals( 10, has_action( 'wp_head', 'amp_add_amphtml_link' ) );
 
-		// Test paired mode homepage.
+		// Test paired mode homepage, where still not on endpoint that it causes amphtml link to be added.
 		remove_action( 'wp_head', 'amp_add_amphtml_link' );
 		$this->go_to( home_url() );
+		$this->assertFalse( is_amp_endpoint() );
 		AMP_Theme_Support::finish_init();
 		$this->assertEquals( 10, has_action( 'wp_head', 'amp_add_amphtml_link' ) );
 
-		// Test canonical.
+		// Test canonical, so amphtml link is not added and init finalizes.
 		remove_action( 'wp_head', 'amp_add_amphtml_link' );
-		remove_theme_support( 'amp' );
-		add_theme_support( 'amp' );
+		add_theme_support( 'amp', array(
+			'mode'         => 'native',
+			'template_dir' => 'amp',
+		) );
 		$this->go_to( get_permalink( $post_id ) );
+		$this->assertTrue( is_amp_endpoint() );
 		AMP_Theme_Support::finish_init();
 		$this->assertFalse( has_action( 'wp_head', 'amp_add_amphtml_link' ) );
+		$this->assertEquals( 10, has_filter( 'index_template_hierarchy', array( 'AMP_Theme_Support', 'filter_amp_template_hierarchy' ) ), 'Expected add_amp_template_filters to have been called since template_dir is not empty' );
+		$this->assertEquals( 20, has_action( 'wp_head', 'amp_add_generator_metadata' ), 'Expected add_hooks to have been called' );
 	}
 
 	/**
@@ -350,11 +354,11 @@ public function test_is_customize_preview_iframe() {
 	}
 
 	/**
-	 * Test register_paired_hooks.
+	 * Test add_amp_template_filters.
 	 *
 	 * @covers AMP_Theme_Support::add_amp_template_filters()
 	 */
-	public function test_register_paired_hooks() {
+	public function test_add_amp_template_filters() {
 		$template_types = array(
 			'paged',
 			'index',
@@ -409,6 +413,34 @@ public function test_validate_non_amp_theme() {
 		$this->assertContains( '<script type="text/javascript" src="https://cdn.ampproject.org/v0/amp-mathml-latest.js" async custom-element="amp-mathml"></script>', $sanitized_html ); // phpcs:ignore WordPress.WP.EnqueuedResources.NonEnqueuedScript
 	}
 
+	/**
+	 * Test get_template_availability.
+	 *
+	 * @covers AMP_Theme_Support::get_template_availability()
+	 */
+	public function test_get_template_availability() {
+		$this->markTestIncomplete();
+		// @todo Test nested.
+		// @todo Test callback vs ID.
+		// @todo Test with query, post, or page.
+		// @todo Test without availability of WP_Query (no_query_available).
+		// @todo Test without theme support (no_theme_support).
+		// @todo Test unrecognized by theme support arg.
+		// @todo Test unrecognized.
+	}
+
+	/**
+	 * Test get_supportable_templates.
+	 *
+	 * @covers AMP_Theme_Support::get_supportable_templates()
+	 */
+	public function test_get_supportable_templates() {
+		$this->markTestIncomplete();
+		// @todo Register custom taxonomy.
+		// @todo Set static front page
+		// @todo Add custom post types.
+	}
+
 	/**
 	 * Test add_hooks.
 	 *
@@ -812,11 +844,11 @@ public function test_amend_comment_form() {
 	}
 
 	/**
-	 * Test filter_paired_template_hierarchy.
+	 * Test filter_amp_template_hierarchy.
 	 *
 	 * @covers AMP_Theme_Support::filter_amp_template_hierarchy()
 	 */
-	public function test_filter_paired_template_hierarchy() {
+	public function test_filter_amp_template_hierarchy() {
 		$template_dir = 'amp-templates';
 		add_theme_support( 'amp', array(
 			'template_dir' => $template_dir,
@@ -950,6 +982,15 @@ public function test_filter_cancel_comment_reply_link() {
 		$this->assertContains( 'Click here to cancel reply.', $filtered_link_no_text_passed );
 	}
 
+	/**
+	 * Test init_admin_bar.
+	 *
+	 * @covers \AMP_Theme_Support::init_admin_bar()
+	 */
+	public function test_init_admin_bar() {
+		$this->markTestIncomplete();
+	}
+
 	/**
 	 * Test print_amp_styles.
 	 *