WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 2 months ago

Last modified 2 months ago

#39868 closed enhancement (fixed)

add unit tests for wp_validate_boolean() in function.php

Reported by: pbearne Owned by: desrosj
Milestone: 5.3 Priority: normal
Severity: normal Version: 4.8
Component: Build/Test Tools Keywords: has-patch
Focuses: Cc:
PR Number:

Description

just some more unit tests

Attachments (3)

wp-validate-boolean.php.patch (1.7 KB) - added by pbearne 3 years ago.
unit tests
39868.diff (3.2 KB) - added by desrosj 2 months ago.
39868-tests-WpValidateBoolean-fix-assertion.patch (949 bytes) - added by jrf 2 months ago.
[tests] WpValidateBoolean: fix assertion

Download all attachments as: .zip

Change History (12)

@pbearne
3 years ago

unit tests

#1 @desrosj
6 months ago

  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 5.3
  • Owner set to desrosj
  • Status changed from new to reviewing

#2 @SergeyBiryukov
6 months ago

  • Component changed from General to Build/Test Tools

@desrosj
2 months ago

#3 @desrosj
2 months ago

Thanks for this, @pbearne!

It seems that there is already a class for testing wp_validate_boolean(), but your data provider method has some additional scenarios that are not currently tested.

39868.diff combines the preexisting conditions with your added tests, replaces individual methods with a data provider method, and improves some of the documentation.

See #30238 for the original test class introduction.

#4 @desrosj
2 months ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 46159:

Build/Test Tools: Improve test coverage for wp_validate_boolean().

This change also reworks the test class to use a data provider.

Props pbearne, desrosj
Fixes #39868

This ticket was mentioned in Slack in #core-restapi by timothybjacobs. View the logs.


2 months ago

#6 @jrf
2 months ago

This patch has changed the test to a loose type comparison of the $result to $expected which will allow for type juggling and basically means the method is not actually being tested correctly anymore. I'll upload a patch shortly to fix this.

Refs:

#7 @jrf
2 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

@jrf
2 months ago

[tests] WpValidateBoolean: fix assertion

#8 @SergeyBiryukov
2 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 46224:

Tests: Correct assertion in test_wp_validate_boolean() to make sure the return type is properly tested.

Props jrf.
Fixes #39868.

#9 @jrf
2 months ago

Thanks @SergeyBiryukov !

Note: See TracTickets for help on using tickets.