Open
Conversation
costdev
requested changes
Jul 1, 2023
| * @since 6.3.0 | ||
| * | ||
| * @param object $object An object instance. | ||
| * @return array |
Contributor
There was a problem hiding this comment.
Suggested change
| * @return array | |
| * @return array The properties of the given object. |
Comment on lines
+68
to
+69
| * @dataProvider data_test_wp_comment_null_byte | ||
| * @ticket 52738 |
Contributor
There was a problem hiding this comment.
Suggested change
| * @dataProvider data_test_wp_comment_null_byte | |
| * @ticket 52738 | |
| * @ticket 52738 | |
| * | |
| * @covers WP_Comment::__construct | |
| * | |
| * @dataProvider data_test_wp_comment_null_byte | |
| * | |
| * @param stdClass $comment_data The comment's data. | |
| * @param string $expected The expected comment content. |
- See this ticket for the order of annotations in the test suite.
- A
@coversannotation should be used. - Test method parameters should be documented.
🔢 Applies elsewhere in the PR.
| $this->assertSame( $comment->comment_content, $expected ); | ||
| } | ||
|
|
||
| public function data_test_wp_comment_null_byte() { |
Contributor
There was a problem hiding this comment.
Suggested change
| public function data_test_wp_comment_null_byte() { | |
| /** | |
| * Data provider. | |
| * | |
| * @return array[] | |
| */ | |
| public function data_test_wp_comment_null_byte() { |
🔢 Applies elsewhere in the PR.
| ';-)' => "\xf0\x9f\x98\x89", | ||
| // This one transformation breaks regular text with frequency. | ||
| // '8)' => "\xf0\x9f\x98\x8e", | ||
| // '8)' => "\xf0\x9f\x98\x8e", |
Contributor
There was a problem hiding this comment.
This looks like an unrelated change.
Comment on lines
+5971
to
+5972
| /* | ||
| First we check if the DOMDocument class exists. If it does not exist, then we cannot |
Contributor
There was a problem hiding this comment.
This looks like an unrelated change.
|
|
||
| public function data_test_wp_comment_null_byte() { | ||
| return array( | ||
| array( |
Contributor
There was a problem hiding this comment.
Datasets should have named keys so that it's immediately clear what's being tested, and that passing --testdox to PHPUnit makes it clear what is being tested.
For example:
'a key that is only a NUL byte''a key starting with a NUL byte''a key ending with a NUL byte'
🔢 Applies elsewhere in the PR.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Trac ticket: https://core.trac.wordpress.org/ticket/52738
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.