WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#37008 closed enhancement (fixed)

Unit test for the remove_query_arg method

Reported by: borgesbruno Owned by: boonebgorges
Milestone: 4.6 Priority: normal
Severity: normal Version:
Component: General Keywords: has-patch has-unit-tests
Focuses: Cc:
PR Number:

Description

Continuing my noble purpose to increase the code coverage =D, I wrote some tests for the remove_query_arg, this one removes query from the current URI using all query keys (Array or string) specified on the first param.

Attachments (1)

37008.diff (1.3 KB) - added by borgesbruno 4 years ago.

Download all attachments as: .zip

Change History (5)

@borgesbruno
4 years ago

#1 @borgesbruno
4 years ago

  • Keywords has-patch has-unit-tests added
  • Type changed from defect (bug) to enhancement

#2 @boonebgorges
4 years ago

  • Milestone changed from Awaiting Review to 4.6
  • Owner set to boonebgorges
  • Status changed from new to assigned

Thanks, @borgesbruno! It is indeed a noble purpose :)

A couple of minor notes about formatting and convention:

  • I'm going to move these tests to their own file
  • The argument format for the dataProvider seems backward to me, so I'm going to change it to match the syntax of the function more closely
  • You've provided error messages, but they're not really descriptive. We generally don't give error messages like this unless they're useful enough to help debug without opening the test files
  • It seems to me that the core functionality of remove_query_arg() can be more succinctly tested by passing in an explicit URL. It's worthwhile to test that the current URL is used as a fallback, but I think it should happen in a separate test
  • No need for a @ticket annotation if the tests being introduced don't demonstrate a bug or a bugfix

#3 @boonebgorges
4 years ago

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

In 37631:

Add tests for remove_query_arg().

Props borgesbruno.
Fixes #37008.

#4 @borgesbruno
4 years ago

Tks for your feedback @boonebgorges, I've seen your commit, it is pretty good.

Note: See TracTickets for help on using tickets.