Skip to content
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

[WIP] wp parse args(): expand tests (Trac 53363) #1559

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Aug 9, 2021

Loosely related to #1558 as all test failures for that function originated from this one.

Note: trac ticket link below not added yet on purpose while this ticket is still WIP.

Trac ticket:


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.

Abstract out some duplicate code to a helper method.
Preferable tests should contain only one assertion.
When testing the same function with different inputs, it is better to use a data provider with named data sets than to have multiple assertions in a test.
This also makes adding more test cases in the future more straight forward.

This commit:
* Moves the test cases from three different tests into a data provider with named data sets.
* Replaces those three test functions with one generic test function.

The actual tests being run remain the same as before.
1. When multiple assertions are used in a test function, each should be accompanied by a `$message` which will be displayed in case of failure. This allows for easier debugging.
2. Tests should validate all assumptions.

This particular test violated both the above rules.

This commit:
* Makes the assumptions (returned values is an array, returned array will contain the expected keys) explicit by adding assertions for each.
* Adds the `$message` parameter for each of the assertions.
This is not done exhaustively. More extensive tests are done in the test class for the `wp_parse_str()` function.
... to document the behaviour when mixed keyed/unkeyed, numeric/associated indexed arrays are provided.
@jrfnl jrfnl force-pushed the trac-53363/wp_parse_args-expand-tests branch from bbc2867 to 9901565 Compare August 13, 2021 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants