WordPress.org

Make WordPress Core

Opened 4 months ago

Closed 4 months ago

Last modified 4 months ago

#26725 closed defect (bug) (fixed)

Wordpress-tests' WP_PHPUnit_Text_UI Throws Unhandled Exceptions

Reported by: wawco Owned by: nacin
Milestone: 3.9 Priority: normal
Severity: normal Version: trunk
Component: Unit Tests Keywords: has-patch
Focuses: Cc:

Description

Summary
In Wordpress-test's boostrap.php, the WP_PHPUnit_TextUI_Command class extends the PHPUnit test runner class simply to parse command line arguments and find some specific ones. PHPUnit's test runner is not actually be used for any other reason.

If it encounters command line arguments that PHPUnit doesn't recognize, an unhandled exception occurs. This is problematic for a couple reasons (see problem details), but the most important one is that enforcing the correct PHPUnit command line arguments is out of the scope of this particular code. If there is a problem, it might be better if it were handled by PHPUnit and not this Wordpress class that we're just using to find specific arguments.

Problem Details

Currently the WP_PHPUnit_TextUI_Command constructor contains a code snippet cribbed directly from the PHPUnit_Text_UI_Command::handleArguments() method.

            $this->options = PHPUnit_Util_Getopt::getopt(
              $argv,
              'd:c:hv',
              array_keys($this->longOptions)
            );

This works fine when you run PHPUnit directly (and correctly). If you try to use one of a handful of different integration or continuous testing tools, PHPUnit is not being run directly from the command line and so unhandled exceptions will occur until boostrap.php is modified by hand.

Specific use case
Trying to use the MakeGood plugin for Eclipse as a tool for continuous testing will cause unhandled exceptions and prevent testing from happening. Here's what MakeGood does...

[PATH_TO_ECLIPSE]\plugins\com.piece_framework.makegood.stagehandtestrunner_2.5.0.v201311031709\resources\php\bin\testrunner.php --no-ansi phpunit --log-junit=[PATH_TO_TEMP_DIR]\com.piece_framework.makegood.launch\MakeGood1388104275775.xml --log-junit-realtime --phpunit-config=[PATH_TO_phpunit.xml] -R --test-file-pattern=Test(?:Case)?\.php$ [TEST_PATH]

PHPUnit won't recognize --no-ansi (if not a few other things) and an exception is thrown.

Proposed Solution
Loop through ARGV directly and call longOption on each one, catching the exceptions thrown by any errors but not acting on them. Dealing with bad command line parameters is something that is either handled earlier or later, by PHPUnit.

Also, instead of extending the PHPUnit runner class, just go directly to the source and extend PHPUnit_Util_Getopt.

Attachments (1)

wordpress-test-boostrap-argument-exception-fix.patch (1.8 KB) - added by wawco 4 months ago.
Implementing changes to the boostrap.php to ingore PHPUnit framework exceptions

Download all attachments as: .zip

Change History (6)

wawco4 months ago

Implementing changes to the boostrap.php to ingore PHPUnit framework exceptions

comment:1 wawco4 months ago

  • Keywords has-patch added

comment:2 jdgrimes4 months ago

  • Cc jdg@… added

comment:3 follow-up: nacin4 months ago

  • Milestone changed from Awaiting Review to 3.9

Looks great, thanks wawco. The only thing I noticed was that $options is no longer as nested, so I needed to remove an $options[0] reference.

comment:4 nacin4 months ago

  • Owner set to nacin
  • Resolution set to fixed
  • Status changed from new to closed

In 26871:

Test runner: Extend WP_PHPUnit_Util_Getopt instead of PHPUnit_TextUI_Command to parse incoming options.

props wawco.
fixes #26725.

comment:5 in reply to: ↑ 3 wawco4 months ago

Replying to nacin:

Looks great, thanks wawco. The only thing I noticed was that $options is no longer as nested, so I needed to remove an $options[0] reference.

Oops, good catch!

Note: See TracTickets for help on using tickets.