Update/patternception saving bug tests #183
Conversation
…into update/patternception-saving-bug-tests
|
I'm not sure why the diff is showing changes outside phpunit changes, but that's all this one is. |
|
Thanks, I'll review this today. |
There was a problem hiding this comment.
Hi @johnstonphilip,
Really nice work.
Some comments below, but no blocker to merging.
| $post->post_title = $pattern['title']; | ||
| $post->post_content = 'This is modified pattern content, but nothing else is changed!'; | ||
| $post->post_status = 'publish'; | ||
| $post->post_name = $pattern['slug']; |
There was a problem hiding this comment.
Nitpick:
| $post->post_name = $pattern['slug']; | |
| $post->post_name = 'mismatched-name'; |
There was a problem hiding this comment.
Is there a reason you'd set the slug to this as opposed to pulling from the file?
There was a problem hiding this comment.
It's a nitpick 😄
But if it were to come from the file, it should be the $pattern['name'].
In tests, I like setting values to string literals, as string variables are mutable.
| $pattern = \PatternManager\PatternDataHandlers\get_pattern_by_name( 'mismatched-name' ); | ||
|
|
||
| // Mock a post object so we can test modifying only the content. | ||
| $post_id = -997; // negative ID, to avoid clash with a valid post. |
There was a problem hiding this comment.
Good idea to mock a new WP_Post.
When the filename is changed, the WP_Post in the editor will be different.
| $post->filter = 'raw'; | ||
|
|
||
| // Convert to WP_Post object. | ||
| $wp_post = new \WP_Post( $post ); |
There was a problem hiding this comment.
Sometimes the post factory from WP_UnitTestCase helps:
$wp_post = $this->factory()->post->create_and_get(
[
'post_title' => 'New Pattern, originally created with Pattern Manager.',
'post_content' => 'test pattern content',
'post_name' => 'new-pattern-originally-created-with-pattern-manager',
'post_type' => get_pattern_post_type(),
]
);| /** | ||
| * Tests that a pattern newly created with Pattern Manager results in a slug that matches the title. | ||
| */ | ||
| public function test_new_pattern_title_matches_slug() { |
There was a problem hiding this comment.
Nitpick that shouldn't block merging:
The pattern PHP files written in these tests shouldn't persist into the next tests.
Maybe:
function setUp() {
// Create a tmp directory
// $this->stylesheet_dir = created tmp directory
add_filter( 'stylesheet_directory', [ $this, 'get_stylesheet_dir' ] ); // So update_pattern() will save patterns to the tmp dir, not the actual theme.
}
function tearDown() {
// Delete $this->stylesheet_dir, maybe with the WP filesystem API
}
function get_stylesheet_dir() {
return $this->stylesheet_dir;
}There was a problem hiding this comment.
If you'd like, I'll open a PR for this, as it's my nitpick 😄
There was a problem hiding this comment.
Yeah if you don't mind, I'd love to fully understand how this works :)
There was a problem hiding this comment.
Sure, I'll do that. Still, don't wait to merge this.
| $content_modified_pattern = \PatternManager\PatternDataHandlers\get_pattern_by_name( 'mismatched-name' ); | ||
|
|
||
| // Make sure the slug does not get changed when only the content is modified. | ||
| $this->assertSame( 'this-slug-remains-the-same-after-content-changes', $content_modified_pattern['slug'] ); |
There was a problem hiding this comment.
Great work figuring out a test for this. Now, we can be more confident in our PRs.
There was a problem hiding this comment.
Really nice naming of 'this-slug-remains-the-same-after-content-changes'
Co-authored-by: Ryan Kienstra <kienstraryan@gmail.com>
Co-authored-by: Ryan Kienstra <kienstraryan@gmail.com>
Co-authored-by: Ryan Kienstra <kienstraryan@gmail.com>
Tasks
Summary of changes
This is a PR to #177 which simply adds some PHPunit tests for the changes in question there.