Skip to content

Commit

Permalink
Fixes #1374: tearDown() is called despite unmet requirements.
Browse files Browse the repository at this point in the history
  • Loading branch information
whatthejeff committed Aug 1, 2014
1 parent e4c97ee commit 427eb1f
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 6 deletions.
16 changes: 10 additions & 6 deletions src/Framework/TestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -806,7 +806,9 @@ public function runBare()
$hookMethods = PHPUnit_Util_Test::getHookMethods(get_class($this));

try {
$hasRequirements = false;
$this->checkRequirements();
$hasRequirements = true;

This comment has been minimized.

Copy link
@sun

sun Aug 1, 2014

Contributor

This variable name confused me. There's a difference between having requirements and meeting requirements.

$hasMetRequirements (or $isValidTest or similar) would be more clear, IMO.

This comment has been minimized.

Copy link
@whatthejeff

whatthejeff Aug 1, 2014

Author Contributor

I'll change it to $hasMetRequirements to avoid ambiguity. Thanks :)


if ($this->inIsolation) {
foreach ($hookMethods['beforeClass'] as $method) {
Expand Down Expand Up @@ -845,14 +847,16 @@ public function runBare()
// Tear down the fixture. An exception raised in tearDown() will be
// caught and passed on when no exception was raised before.
try {
foreach ($hookMethods['after'] as $method) {
$this->$method();
}

if ($this->inIsolation) {
foreach ($hookMethods['afterClass'] as $method) {
if ($hasRequirements) {
foreach ($hookMethods['after'] as $method) {
$this->$method();
}

if ($this->inIsolation) {
foreach ($hookMethods['afterClass'] as $method) {
$this->$method();
}
}
}
} catch (Exception $_e) {
if (!isset($e)) {
Expand Down
21 changes: 21 additions & 0 deletions tests/Regression/GitHub/1374.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
--TEST--
GH-1374: tearDown() is called despite unmet requirements
--FILE--
<?php

$_SERVER['argv'][1] = '--no-configuration';
$_SERVER['argv'][2] = 'Issue1374Test';
$_SERVER['argv'][3] = dirname(__FILE__).'/1374/Issue1374Test.php';

require __DIR__ . '/../../bootstrap.php';
PHPUnit_TextUI_Command::main();
?>
--EXPECTF--
PHPUnit %s by Sebastian Bergmann.

S

Time: %s, Memory: %sMb

OK, but incomplete, skipped, or risky tests!
Tests: 1, Assertions: 0, Skipped: 1.
21 changes: 21 additions & 0 deletions tests/Regression/GitHub/1374/Issue1374Test.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<?php
/**
* @requires extension I_DO_NOT_EXIST
*/
class Issue1374Test extends PHPUnit_Framework_TestCase
{
protected function setUp()
{
print __FUNCTION__;
}

public function testSomething()
{
$this->fail('This should not be reached');
}

protected function tearDown()
{
print __FUNCTION__;
}
}

0 comments on commit 427eb1f

Please sign in to comment.