#54725 closed task (blessed) (fixed)
Test tool and unit test improvements for 6.0
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 6.0 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Build/Test Tools | Keywords: | has-patch has-unit-tests fixed-major dev-reviewed commit |
Focuses: | Cc: |
Attachments (4)
Change History (46)
This ticket was mentioned in PR #1995 on WordPress/wordpress-develop by johnbillion.
3 years ago
#1
- Keywords has-patch has-unit-tests added
johnbillion commented on PR #1995:
3 years ago
#2
Thanks for the comments @jrfnl , I've pushed some changes. I've left the whitespace unadjusted for now to keep the diff clean, the whitespace can be corrected at the point of commit.
Next up, we've actually got some failures as a result of these additional assertions that need investigating.
3 years ago
#3
Next up, we've actually got some failures as a result of these additional assertions that need investigating.
Nah, not test failures: test successes as making these changes has probably uncovered some bugs in the underlying code 😁
@
3 years ago
Use data provider tests/phpunit/tests/functions/wpToKebabCase.php; typo in tests/phpunit/tests/functions/wpValidateBoolean.php
#11
in reply to:
↑ 10
;
follow-up:
↓ 12
@
3 years ago
Replying to SergeyBiryukov:
In 52778:
wooops! I didn't know and i submit new patch again in 54725.diff, damn it(sorry)!
but there are some typo i've fixed in functions docblock. You should take a look. thanks.
#12
in reply to:
↑ 11
@
3 years ago
Replying to azouamauriac:
wooops! I didn't know and i submit new patch again in 54725.diff, damn it(sorry)!
but there are some typo i've fixed in functions docblock. You should take a look. thanks.
Ah, sorry, I didn't know someone else was working on it. Looking at the DocBlocks, it seems like we're not very consistent with using third-person singular verbs in these test descriptions, so let's fix that at least in a few other files.
This ticket was mentioned in PR #2453 on WordPress/wordpress-develop by jrfnl.
3 years ago
#17
Started this branch to debug a couple of tests (webp related) which are failing on PHP 5.6 when NOT run inside the Docker container, using a standard PHP 5.6 install.
This generally would indicate a missing @requires
tag or missing skip condition.
I've done a large test refactor for that particular test class as nearly all methods were testing using foreach()
loops instead of data providers, so it was impossible to see what was really failing.
While looking into this I found that the following curiousities:
- A function which was changed in WP 3.5.0
wp_save_image_file()
, apparently also changed return type - frombool
toarray|WP_Error
(BC-break anyone ?) -, but the documentation does not reflect that.- Does nobody use the function ?
- Or if they do, how can it be that nobody noticed this in 25 major releases ?
- The failing tests are in part not actually testing what they should be testing and if tested this way, there seems to be some dependency on a particular (not necessarily standard) PHP compilation. I haven't been able to track down the exact difference or rather how to detect the difference, but the result is the failing tests.
Note: the last commit is very much WIP/debugging. The commits before that are valid improvements which can be (and should be) committed anyway.
Trac ticket: https://core.trac.wordpress.org/ticket/54725
#19
@
3 years ago
I've opened DRAFT PR https://github.com/WordPress/wordpress-develop/pull/2453
Most changes in that PR are ready for commit, except for the "WIP/DEBUG" commit at the top of the stack.
See the PR description for additional context.
There is one documentation fix commit in the chain which belong with #54729
3 years ago
#22
I've updated the third commit - "Add helper function for creating named data provider" - as discussed with @hellofromtonya in today's call:
- Moved the function from this particular test class to the generic
WP_UnitTestCase_Base
class. - Added an extra safeguard to prevent duplicate entries overwriting each other.
Other than that, I've added an extra commit at the top of the commit chain with the additional debug code used in today's call with @adamsilverstein and @hellofromtonya .
As discussed, the first 13 commits can, and should, be committed as soon as the have been reviewed and found to be okay.
The last two commits will then remain as helpers to solve the underlying issue now identified with webP support on PHP 5.6/7.0.
3 years ago
#23
Ha, just noticed I'd forgotten the @since
tag for the new test helper, so updated that commit.
I've now pushed the first 13 commits again to show a passing build for those, so the committer can see those commits are "clean" and ready to go.
Once that build has finished, I will add the last two commits again.
This ticket was mentioned in PR #2545 on WordPress/wordpress-develop by desrosj.
3 years ago
#25
Trac ticket: https://core.trac.wordpress.org/ticket/54725
3 years ago
#27
Merged into Core in https://core.trac.wordpress.org/changeset/53112.
This ticket was mentioned in PR #2701 on WordPress/wordpress-develop by hellofromtonya.
3 years ago
#31
The odd / even class attribute global variables are causing issues in comments tests when a new test is added or an existing test is modified. To stabilize the odd / even comment tests, the comment global variables are added to the base test class' tear_down()
using the same patterns as the other global resets. This change ensures each comment test starts at the same state. In doing so, the expected odd / even class attributes are no longer affected by previous tests, i.e. test leaks.
Adding this to a 6.0 test improvement ticket for backport to 6.0-branch since this issue was identified in the 6.0 cycle.
Trac ticket: https://core.trac.wordpress.org/ticket/54725
#33
@
3 years ago
- Keywords fixed-major dev-reviewed commit added
- Resolution fixed deleted
- Status changed from closed to reopened
Reopening to backport [53430] to 6.0 branch, ie where the original issue was found.
This ticket was mentioned in PR #2742 on WordPress/wordpress-develop by hellofromtonya.
3 years ago
#34
Backports changeset [53353] and [53430] + moves the global resets from WP_UnitTestCase_Base::tear_down()
to the test's tear_down()
to avoid potential issues for extenders (see @peterwilsoncc comment https://github.com/WordPress/wordpress-develop/pull/2701#discussion_r870830662].
Trac ticket: https://core.trac.wordpress.org/ticket/54725
#35
@
3 years ago
- Owner set to hellofromTonya
- Resolution set to fixed
- Status changed from reopened to closed
In 53432:
hellofromtonya commented on PR #2701:
3 years ago
#37
hellofromtonya commented on PR #2742:
3 years ago
#38
This ticket was mentioned in Slack in #core by sergey. View the logs.
3 years ago
SergeyBiryukov commented on PR #2453:
3 years ago
#41
Shall I rebase the PR ?
Not sure if that's necessary, the test improvement commits still seem to apply correctly :)
Several tests perform assertions conditionally or iterate dynamic arrays without ensuring they're populated. If the test is faulty and the condition never evaluates to true or the array being iterated is empty then we'll never know about it.
This adds additional assertions that cover these situations.
Update: Moving this PR to the new 6.0 ticket.
Trac ticket: https://core.trac.wordpress.org/ticket/54725