WordPress.org

Make WordPress Core

Opened 8 years ago

Closed 6 years ago

#27411 closed defect (bug) (wontfix)

Allow adding custom @ticket types for unit tests

Reported by: rmccue Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords:
Focuses: Cc:

Description

It's possible to set tests as being skipped by adding @ticket xxx, @ticket UTxxx or @ticket PluginXXX depending on which you're referring to. Additionally, you can conditionally force running these via the command line with --group xxx, --group UTxxx or @ticket PluginXXX.

It's possible to add your own types for @ticket by subclassing WP_UnitTestCase, but it's not possible to use the --group flag for these.

It'd be nice to make this configurable (filterable?) to add extra types. Maybe a define in wp-tests-config.php?

Change History (6)

This ticket was mentioned in IRC in #wordpress-dev by rmccue. View the logs.


8 years ago

#2 @rmccue
8 years ago

So, here's what I'm thinking: change the regex to match ^\w+\d+$ instead, which matches any prefix before a number (HM123, wat123, etc). We don't really need to worry about accidentally adding things here, since if they're not ticket numbers, they'll just never be checked.

#3 follow-up: @nacin
8 years ago

The checkRequirements method, I imagine, would need to run a generic knownBug method (or the like) for an @ticket it doesn't recognize. How would it know whether the test should be skipped, otherwise?

Seems to me like extending checkRequirements, calling parent:checkedRequirements(), calling $tickets = PHPUnit_Util_Test::getTickets( get_class( $this ), $this->getName( false ) ); on your own, and such, is a better method. We could abstract out that into a method, at least; that would help.

I've been thinking for some time now about getting rid of the skipped test properties of @ticket. We now only commit tests with the code for them, which tells me we could over time remove all existing "skipped" tests. Indeed, we haven't exactly added any skipped tests in some time. They'd still function as documentation as well as a shortcut for running them.

#4 in reply to: ↑ 3 ; follow-up: @rmccue
8 years ago

Replying to nacin:

The checkRequirements method, I imagine, would need to run a generic knownBug method (or the like) for an @ticket it doesn't recognize. How would it know whether the test should be skipped, otherwise?

Seems to me like extending checkRequirements, calling parent:checkedRequirements(), calling $tickets = PHPUnit_Util_Test::getTickets( get_class( $this ), $this->getName( false ) ); on your own, and such, is a better method. We could abstract out that into a method, at least; that would help.

That's not so much an issue, as you can simply subclass and implement checkRequirements yourself. The group-checking code, however, is a global thing, so you have to create your own in parallel that basically does exactly the same. (That is: it has no real code in it, whereas checkRequirements handles calling the relevant API.)

I've been thinking for some time now about getting rid of the skipped test properties of @ticket. We now only commit tests with the code for them, which tells me we could over time remove all existing "skipped" tests. Indeed, we haven't exactly added any skipped tests in some time. They'd still function as documentation as well as a shortcut for running them.

I actually prefer this method, as it gives you an idea of what's currently broken. That said, these types of tests (xfail) are a bit controversial, so I could understand removing them.

#5 in reply to: ↑ 4 @jdgrimes
6 years ago

Replying to rmccue:

Replying to nacin:

I've been thinking for some time now about getting rid of the skipped test properties of @ticket. We now only commit tests with the code for them, which tells me we could over time remove all existing "skipped" tests. Indeed, we haven't exactly added any skipped tests in some time. They'd still function as documentation as well as a shortcut for running them.

I actually prefer this method, as it gives you an idea of what's currently broken. That said, these types of tests (xfail) are a bit controversial, so I could understand removing them.

The skipped tests were removed in #30284.

#6 @nacin
6 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from new to closed

At this point, this ::checkRequirements() code is dead IMO.

Note: See TracTickets for help on using tickets.