-
Notifications
You must be signed in to change notification settings - Fork 382
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Test AMP compatibility of entire site #1183
Changes from 6 commits
305c28b
9e9df92
fd50993
1324044
3c4db61
bbfafa4
db54a53
6713ff3
2655310
32ab635
1a862c7
c250bd5
71f7543
4aade51
584ffea
ae4371c
7b67efe
15ec0ed
1ef10a7
ba08f3d
bfe37a8
3d9a01c
c586c24
128ca9c
441044f
875d4ee
4e8c00c
c4b8f5f
c28e5ee
b6393cc
c638e7e
ad43100
5af8dc9
adcb901
9d11439
5e19020
f4e6bfd
8e1b031
a4c1f7a
ce2b044
cd1d09e
edfdd65
19f4feb
e34ebda
57411c6
6802175
a53bb45
d44cde9
71f4fcb
ff3a5c0
d378a37
4f8b3e9
4ea25bf
dadab10
c687ea0
09fa083
eadb61c
e8717fd
13a0cc7
f054f13
be32dcf
ff8d365
1387c5b
41cffdc
0202432
bff8fc7
5d0d464
86cd224
c31fa0e
be8d552
3dac2fd
fe8eab3
b741f01
4cf9e9d
907facd
52ff854
ab9c2d9
1f8f40b
aca16f4
2637fb8
da22efc
bfadbe2
30f7522
e1760aa
ad7e0e2
7a52751
8e2d028
edcb23a
0e9bf1b
a83a569
19ead16
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,76 @@ | ||
<?php | ||
/** | ||
* Class AMP_Site_Validation | ||
* | ||
* @package AMP | ||
*/ | ||
|
||
/** | ||
* Class AMP_Site_Validation | ||
* | ||
* @since 1.0 | ||
*/ | ||
class AMP_Site_Validation { | ||
|
||
/** | ||
* Gets the permalinks of all public post types with the status 'publish,' to check for AMP validity. | ||
* | ||
* This excludes 'attachment' post types, as it only searches for posts with the status 'publish.' | ||
* Attachments have a default status of 'inherit,' so they can depend on the status of their parent like a post. | ||
* | ||
* @todo: consider whether this should also return 'attachment' IDs. | ||
* @param int $number_posts The maximum amount of posts to get the permalinks for (optional). | ||
* @return string[] $permalinks The post permalinks in an array. | ||
*/ | ||
public static function get_post_permalinks( $number_posts = 200 ) { | ||
$post_types = get_post_types( array( 'public' => true ), 'names' ); | ||
$query = new WP_Query( array( | ||
'posts_per_page' => $number_posts, | ||
'post_type' => array_values( $post_types ), | ||
'post_status' => 'publish', | ||
) ); | ||
|
||
$post_ids = wp_list_pluck( $query->posts, 'ID' ); | ||
return array_map( 'get_permalink', $post_ids ); | ||
} | ||
|
||
/** | ||
* Gets the front-end links for public taxonomy terms, like categories and tags. | ||
* | ||
* For example, https://example.org/?cat=2 | ||
* This includes categories and tags, and any more that are registered. | ||
* But it excludes post_format terms. | ||
* | ||
* @param int $number_links The maximum amount of links to get (optional). | ||
* @return string[] $links The term links in an array. | ||
*/ | ||
public static function get_taxonomy_links( $number_links = 200 ) { | ||
$public_taxonomies = get_taxonomies( array( | ||
'public' => true, | ||
) ); | ||
|
||
// It doesn't seem necessary to get links for post format terms, like asides, galleries, or quotes. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, I think it is necessary. There are archive links for post formats: https://codex.wordpress.org/Function_Reference/get_post_format_link There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, commit db54a removes the exemption of |
||
unset( $public_taxonomies['post_format'] ); | ||
$terms = get_terms( array( | ||
'taxonomy' => $public_taxonomies, | ||
'count' => $number_links, | ||
) ); | ||
|
||
return array_map( 'get_term_link', $terms ); | ||
} | ||
|
||
/** | ||
* Validates the URLs on the site. | ||
* | ||
* @todo: Consider wrapping this function with another, as different use cases will probably require a different return value or display. | ||
* For example, the <button> in /wp-admin that makes an AJAX request for this will need a different response than a WP-CLI command. | ||
* @return array $validation_result The post ID as the index, and the result of validation as the value. | ||
*/ | ||
public static function validate_site_urls() { | ||
$site_post_ids = self::get_post_ids(); | ||
foreach ( $site_post_ids as $id ) { | ||
AMP_Validation_Manager::$posts_pending_frontend_validation[ $id ] = true; | ||
} | ||
return AMP_Validation_Manager::validate_queued_posts_on_frontend(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As you suggested, this
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for your fast reply, @westonruter! These details really help. |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,134 @@ | ||
<?php | ||
/** | ||
* Tests for AMP_Site_Validation class. | ||
* | ||
* @package AMP | ||
*/ | ||
|
||
/** | ||
* Tests for AMP_Site_Validation class. | ||
* | ||
* @since 1.0 | ||
*/ | ||
class Test_AMP_Site_Validation extends \WP_UnitTestCase { | ||
|
||
/** | ||
* The name of the tested class. | ||
* | ||
* @var string | ||
*/ | ||
const TESTED_CLASS = 'AMP_Site_Validation'; | ||
|
||
/** | ||
* The name of the tag to test. | ||
* | ||
* @var string | ||
*/ | ||
const TAG_NAME = 'img'; | ||
|
||
/** | ||
* A disallowed element, which should cause a validation error. | ||
* | ||
* @var string | ||
*/ | ||
const DISALLOWED_TAG = '<script>doThis();</script>'; | ||
|
||
/** | ||
* An instance of DOMElement to test. | ||
* | ||
* @var DOMElement | ||
*/ | ||
public $node; | ||
|
||
/** | ||
* Setup. | ||
* | ||
* @inheritdoc | ||
* @global $wp_registered_widgets | ||
*/ | ||
public function setUp() { | ||
parent::setUp(); | ||
$dom_document = new DOMDocument( '1.0', 'utf-8' ); | ||
$this->node = $dom_document->createElement( self::TAG_NAME ); | ||
AMP_Validation_Manager::reset_validation_results(); | ||
} | ||
|
||
/** | ||
* Test get_post_permalinks. | ||
* | ||
* @covers AMP_Site_Validation::get_post_permalinks() | ||
*/ | ||
public function test_get_post_permalinks() { | ||
$number_posts_each_post_type = 20; | ||
$post_types = get_post_types( array( 'public' => true ), 'names' ); | ||
$expected_post_permalinks = array(); | ||
|
||
/** | ||
* The tested method does not get attachment permalinks. | ||
* It only searches for posts with the status 'publish.' | ||
* Attachments have a default status of 'inherit,' to depend on the status of their parent post. | ||
*/ | ||
unset( $post_types['attachment'] ); | ||
foreach ( $post_types as $post_type ) { | ||
for ( $i = 0; $i < $number_posts_each_post_type; $i++ ) { | ||
$expected_post_permalinks[] = get_permalink( $this->factory()->post->create( array( | ||
'post_type' => $post_type, | ||
) ) ); | ||
} | ||
} | ||
$number_of_posts = count( $post_types ) * $number_posts_each_post_type; | ||
$actual_post_permalinks = AMP_Site_Validation::get_post_permalinks( $number_of_posts ); | ||
|
||
/* | ||
* The factory() method above creates posts so quickly that the WP_Query() default argument of 'orderby' => 'date' | ||
* doesn't return them in the exact order they were created. | ||
* So this simply ensures all of the created $post_ids are present in the return value of the tested method. | ||
*/ | ||
$this->assertEquals( 0, count( array_diff( $expected_post_permalinks, $actual_post_permalinks ) ) ); | ||
$this->assertEquals( count( $expected_post_permalinks ), count( $actual_post_permalinks ) ); | ||
} | ||
|
||
/** | ||
* Test get_taxonomy_links. | ||
* | ||
* @covers AMP_Site_Validation::get_taxonomy_links() | ||
*/ | ||
public function test_get_taxonomy_links() { | ||
$number_links_each_taxonomy = 20; | ||
$taxonomies = get_taxonomies( array( | ||
'public' => true, | ||
) ); | ||
|
||
unset( $taxonomies['post_format'] ); | ||
$all_terms = array(); | ||
|
||
foreach ( $taxonomies as $taxonomy ) { | ||
$terms_for_current_taxonomy = array(); | ||
for ( $i = 0; $i < $number_links_each_taxonomy; $i++ ) { | ||
$terms_for_current_taxonomy[] = $this->factory()->term->create( array( | ||
'taxonomy' => $taxonomy, | ||
) ); | ||
} | ||
|
||
// Terms need to be associated with a post in order to be returned in get_terms(). | ||
wp_set_post_terms( | ||
$this->factory()->post->create(), | ||
$terms_for_current_taxonomy, | ||
$taxonomy | ||
); | ||
$all_terms = array_merge( $all_terms, $terms_for_current_taxonomy ); | ||
} | ||
|
||
$expected_links = array_map( 'get_term_link', $all_terms ); | ||
$number_of_links = 100; | ||
$actual_links = AMP_Site_Validation::get_taxonomy_links( $number_of_links ); | ||
|
||
/* | ||
* Test that all of the $expected_links are present. | ||
* There is already one term present before this test method adds any, | ||
* so that can also appear in the returned $actual_links. | ||
*/ | ||
$this->assertEquals( 0, count( array_diff( $expected_links, $actual_links ) ) ); | ||
$this->assertLessThan( $number_of_links, count( $actual_links ) ); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There should be pagination support here. The
WP_Term_Query
supports an$offset
arg. I suggest that this method only return links for one specific taxonomy. The site crawler will probably need to make multiple calls to such a method to iterate over all of the taxonomy terms in batches. One way to do it would be to get the terms sorted by ID in ascending order. That ensures that when you paginate through the terms you won't miss terms that get added while you're processing through them.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, good ideas. These commits apply your suggestions, as I understand them. This now has
$taxonomy
and$offset
parameters.