Skip to content

Commit 6e8e56a

Browse files
kkmuffmekkmuffme
committed
Fix 5851
1 parent 3e6c0dc commit 6e8e56a

File tree

4 files changed

+473
-25
lines changed

4 files changed

+473
-25
lines changed

‎src/Framework/TestCase.php

Lines changed: 213 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,13 @@
4242
use function is_string;
4343
use function libxml_clear_errors;
4444
use function method_exists;
45+
use function ob_clean;
4546
use function ob_end_clean;
46-
use function ob_get_clean;
47+
use function ob_end_flush;
48+
use function ob_flush;
4749
use function ob_get_contents;
4850
use function ob_get_level;
51+
use function ob_get_status;
4952
use function ob_start;
5053
use function parse_url;
5154
use function pathinfo;
@@ -191,6 +194,7 @@ abstract class TestCase extends Assert implements Reorderable, SelfDescribing, T
191194
private ?string $outputExpectedString = null;
192195
private bool $outputBufferingActive = false;
193196
private int $outputBufferingLevel;
197+
private ?int $outputBufferingFlushed = null;
194198
private bool $outputRetrievedForAssertion = false;
195199
private bool $doesNotPerformAssertions = false;
196200

@@ -558,11 +562,13 @@ final public function runBare(): void
558562
$outputBufferingStopped = false;
559563

560564
if (!isset($e) &&
561-
$this->hasExpectationOnOutput() &&
562-
$this->stopOutputBuffering()) {
565+
$this->hasExpectationOnOutput()) {
566+
// if it fails now, we shouldn't try again later either
563567
$outputBufferingStopped = true;
564568

565-
$this->performAssertionsOnOutput();
569+
if ($this->stopOutputBuffering()) {
570+
$this->performAssertionsOnOutput();
571+
}
566572
}
567573

568574
if ($this->status->isSuccess()) {
@@ -1813,43 +1819,235 @@ private function markSkippedForMissingDependency(ExecutionOrderDependency $depen
18131819
$this->status = TestStatus::skipped($message);
18141820
}
18151821

1822+
private function outputBufferingCallback(string $output, int $phase): string
1823+
{
1824+
// assign it here to not get output from uncleanable children at the end
1825+
// as well as to ensure we have all output available to check
1826+
// if any children buffers have a low chunk size and already returned data
1827+
// or ob_flush was called
1828+
if ($this->outputBufferingActive) {
1829+
$this->output .= $output;
1830+
1831+
if (($phase & PHP_OUTPUT_HANDLER_FINAL) === PHP_OUTPUT_HANDLER_FINAL) {
1832+
// don't handle, since we report an error already if our handler is missing
1833+
// since it's inconsistent here with ob_end_flush/ob_end_clean
1834+
return '';
1835+
}
1836+
1837+
if (($phase & PHP_OUTPUT_HANDLER_CLEAN) === PHP_OUTPUT_HANDLER_CLEAN) {
1838+
$this->outputBufferingFlushed = PHP_OUTPUT_HANDLER_CLEAN;
1839+
} elseif (($phase & PHP_OUTPUT_HANDLER_FLUSH) === PHP_OUTPUT_HANDLER_FLUSH) {
1840+
$this->outputBufferingFlushed = PHP_OUTPUT_HANDLER_FLUSH;
1841+
}
1842+
}
1843+
1844+
return '';
1845+
}
1846+
1847+
// private to ensure it cannot be restarted after ending it from outside this class
18161848
private function startOutputBuffering(): void
18171849
{
1818-
ob_start();
1850+
ob_start([$this, 'outputBufferingCallback']);
18191851

18201852
$this->outputBufferingActive = true;
18211853
$this->outputBufferingLevel = ob_get_level();
18221854
}
18231855

1856+
/**
1857+
* @throws Exception
1858+
* @throws NoPreviousThrowableException
1859+
*/
18241860
private function stopOutputBuffering(): bool
18251861
{
1826-
$bufferingLevel = ob_get_level();
1862+
$bufferingLevel = ob_get_level();
1863+
$bufferingStatus = ob_get_status();
1864+
$bufferingCallbackName = $bufferingStatus['name'] ?? '';
1865+
$expectedBufferingCallable = static::class . '::outputBufferingCallback';
1866+
1867+
if ($bufferingLevel !== $this->outputBufferingLevel ||
1868+
($this->outputBufferingActive && $bufferingCallbackName !== $expectedBufferingCallable) ||
1869+
$this->outputBufferingFlushed !== null) {
1870+
$asFailure = true;
18271871

1828-
if ($bufferingLevel !== $this->outputBufferingLevel) {
18291872
if ($bufferingLevel > $this->outputBufferingLevel) {
18301873
$message = 'Test code or tested code did not close its own output buffers';
1831-
} else {
1874+
} elseif ($bufferingLevel < $this->outputBufferingLevel) {
18321875
$message = 'Test code or tested code closed output buffers other than its own';
1876+
} elseif ($this->outputBufferingActive && $bufferingCallbackName !== $expectedBufferingCallable) {
1877+
$message = 'Test code or tested code first closed output buffers other than its own and later started output buffers it did not close';
1878+
} elseif ($this->outputBufferingFlushed !== null) {
1879+
// if we weren't in phpunit this would lead to a PHP notice
1880+
$message = 'Test code or tested code flushed or cleaned global output buffers other than its own';
1881+
} else {
1882+
$this->outputBufferingLevel = ob_get_level();
1883+
1884+
return true;
1885+
}
1886+
1887+
$hasExpectedCallable = false;
1888+
1889+
if ($this->outputBufferingActive) {
1890+
$fullObStatus = ob_get_status(true);
1891+
$bufferingCallbackNameLevel = $fullObStatus[$this->outputBufferingLevel - 1]['name'] ?? '';
1892+
$bufferingCallbackSizeUsed = $fullObStatus[$this->outputBufferingLevel - 1]['buffer_used'] ?? PHP_INT_MAX;
1893+
1894+
if ($bufferingCallbackNameLevel === $expectedBufferingCallable) {
1895+
$hasExpectedCallable = true;
1896+
1897+
foreach ($fullObStatus as $index => $obStatus) {
1898+
if ($index < $this->outputBufferingLevel) {
1899+
continue;
1900+
}
1901+
1902+
if (($obStatus['flags'] & PHP_OUTPUT_HANDLER_REMOVABLE) === PHP_OUTPUT_HANDLER_REMOVABLE) {
1903+
continue;
1904+
}
1905+
1906+
if (!$this->inIsolation && !$this->shouldRunInSeparateProcess()) {
1907+
$message = 'Test code contains a non-removable output buffer - run test in separate process to avoid side-effects';
1908+
$hasExpectedCallable = false;
1909+
1910+
break;
1911+
}
1912+
1913+
// allow non-removable handler 1 level deeper than our handler to allow unit tests for non-removable handlers
1914+
// however only if our own handler is empty, as we cannot retrieve that from our handler if we are in a non-removable handler in a level deeper
1915+
if ($index === $this->outputBufferingLevel && $bufferingCallbackSizeUsed === 0) {
1916+
continue;
1917+
}
1918+
1919+
if ($index === $this->outputBufferingLevel) {
1920+
$message = 'Tests with non-removable output buffer handlers must not call flush on them and the chunk size must be bigger than the expected output';
1921+
} else {
1922+
// we cannot get the data from the handlers between our handler and the topmost non-removable handler
1923+
$message = 'Tests with multiple output buffers where any, except the first, are non-removable are not supported';
1924+
}
1925+
1926+
$hasExpectedCallable = false;
1927+
1928+
break;
1929+
}
1930+
1931+
if ($hasExpectedCallable && $this->outputBufferingFlushed === null) {
1932+
$asFailure = false;
1933+
}
1934+
} else {
1935+
// the original buffer doesn't exist anymore at that level, which means it was closed
1936+
$message = 'Test code or tested code first closed output buffers other than its own and later started output buffers it did not close';
1937+
}
1938+
} else {
1939+
$asFailure = false;
18331940
}
18341941

18351942
while (ob_get_level() >= $this->outputBufferingLevel) {
1836-
ob_end_clean();
1943+
$obStatus = ob_get_status();
1944+
1945+
if ($obStatus === []) {
1946+
break;
1947+
}
1948+
1949+
// 'level' is off by 1 because 0-indexed
1950+
if ($hasExpectedCallable && $obStatus['name'] === $expectedBufferingCallable && $obStatus['level'] + 1 === $this->outputBufferingLevel) {
1951+
// our own handler
1952+
ob_end_clean();
1953+
1954+
continue;
1955+
}
1956+
1957+
if (($obStatus['flags'] & PHP_OUTPUT_HANDLER_REMOVABLE) === PHP_OUTPUT_HANDLER_REMOVABLE) {
1958+
// bubble it up
1959+
ob_end_flush();
1960+
1961+
continue;
1962+
}
1963+
1964+
if ($hasExpectedCallable && $obStatus['level'] === $this->outputBufferingLevel) {
1965+
$fullObStatus = ob_get_status(true);
1966+
$bufferingCallbackSizeUsed = $fullObStatus[$this->outputBufferingLevel - 1]['buffer_used'] ?? PHP_INT_MAX;
1967+
1968+
// we are 1 level deeper than our buffer
1969+
// check again to be sure, as we cannot retrieve what's in the buffer
1970+
if ($bufferingCallbackSizeUsed !== 0) {
1971+
$hasExpectedCallable = false;
1972+
$asFailure = true;
1973+
$message = 'Tests with non-removable output buffer handlers must not call flush on them and the chunk size must be bigger than the expected output';
1974+
} else {
1975+
// assign it since we cannot trigger our callback
1976+
// this is the reason why it's risky even then, since the ob callback of the non-removable buffer isn't called
1977+
// which could modify the output
1978+
$this->output .= (string) ob_get_contents();
1979+
1980+
// if we have the default output handler which doesn't modify output
1981+
// this isn't even risky
1982+
if ($obStatus['name'] === 'default output handler' && $this->outputBufferingFlushed === null) {
1983+
$message = null;
1984+
} elseif ($this->outputBufferingFlushed === null) {
1985+
$message = 'Non-removable output handler callback was not called, which could alter output';
1986+
} else {
1987+
$asFailure = true;
1988+
}
1989+
}
1990+
} elseif (($obStatus['flags'] & PHP_OUTPUT_HANDLER_FLUSHABLE) === PHP_OUTPUT_HANDLER_FLUSHABLE) {
1991+
// bubble it up
1992+
ob_flush();
1993+
}
1994+
1995+
if (($obStatus['flags'] & PHP_OUTPUT_HANDLER_CLEANABLE) === PHP_OUTPUT_HANDLER_CLEANABLE) {
1996+
// make sure it's empty for subsequent runs to reduce unrelated errors
1997+
ob_clean();
1998+
}
1999+
2000+
// can't end any parents either
2001+
break;
18372002
}
18382003

1839-
Event\Facade::emitter()->testConsideredRisky(
2004+
// reset it to stop adding more output
2005+
$this->outputBufferingActive = false;
2006+
$this->outputBufferingFlushed = null;
2007+
$this->outputBufferingLevel = ob_get_level();
2008+
2009+
if ($message === null) {
2010+
return true;
2011+
}
2012+
2013+
if (!$asFailure) {
2014+
Event\Facade::emitter()->testConsideredRisky(
2015+
$this->valueObjectForEvents(),
2016+
$message,
2017+
);
2018+
2019+
$this->status = TestStatus::risky($message);
2020+
2021+
return true;
2022+
}
2023+
2024+
// it's impossible to tell if there were any PHP errors or failed assertions
2025+
$this->status = TestStatus::failure($message);
2026+
2027+
Event\Facade::emitter()->testFailed(
18402028
$this->valueObjectForEvents(),
1841-
$message,
2029+
Event\Code\ThrowableBuilder::from(new Exception($message)),
2030+
null,
18422031
);
18432032

1844-
$this->status = TestStatus::risky($message);
2033+
if ($this->numberOfAssertionsPerformed() === 0 &&
2034+
$this->hasExpectationOnOutput()) {
2035+
// no error that no assertions were performed
2036+
$this->addToAssertionCount(1);
2037+
}
18452038

18462039
return false;
18472040
}
18482041

1849-
$this->output = ob_get_clean();
2042+
if (!$this->outputBufferingActive) {
2043+
return true;
2044+
}
18502045

1851-
$this->outputBufferingActive = false;
1852-
$this->outputBufferingLevel = ob_get_level();
2046+
ob_end_clean();
2047+
2048+
$this->outputBufferingActive = false;
2049+
$this->outputBufferingFlushed = null;
2050+
$this->outputBufferingLevel = ob_get_level();
18532051

18542052
return true;
18552053
}

‎tests/end-to-end/regression/5342.phpt

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,21 +17,15 @@ F 1 / 1 (100%)
1717

1818
Time: %s, Memory: %s
1919

20-
There was 1 failure:
20+
There were 2 failures:
2121

2222
1) PHPUnit\TestFixture\Issue5342Test::testFailure
2323
Failed asserting that false is true.
2424

2525
%sIssue5342Test.php:%i
2626

27-
--
28-
29-
There was 1 risky test:
30-
31-
1) PHPUnit\TestFixture\Issue5342Test::testFailure
32-
Test code or tested code closed output buffers other than its own
33-
34-
%sIssue5342Test.php:%i
27+
2) PHPUnit\TestFixture\Issue5342Test::testFailure
28+
PHPUnit\Framework\Exception: Test code or tested code first closed output buffers other than its own and later started output buffers it did not close
3529

3630
FAILURES!
37-
Tests: 1, Assertions: 1, Failures: 1, Risky: 1.
31+
Tests: 1, Assertions: 1, Failures: 2.

‎tests/end-to-end/regression/5851.phpt

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
--TEST--
2+
https://github.com/sebastianbergmann/phpunit/pull/5861
3+
--FILE--
4+
<?php declare(strict_types=1);
5+
$_SERVER['argv'][] = '--do-not-cache-result';
6+
$_SERVER['argv'][] = '--no-configuration';
7+
$_SERVER['argv'][] = __DIR__ . '/5851/Issue5851Test.php';
8+
9+
require_once __DIR__ . '/../../bootstrap.php';
10+
(new PHPUnit\TextUI\Application)->run($_SERVER['argv']);
11+
--EXPECTF--
12+
PHPUnit %s by Sebastian Bergmann and contributors.
13+
14+
Runtime: %s
15+
16+
FFFIllegaly hide thisFFSneakyFNaughtySafeFFRRFRF. 14 / 14 (100%)
17+
18+
Time: %s, Memory: %s
19+
20+
There were 10 failures:
21+
22+
1) PHPUnit\TestFixture\Issue5851Test::testInvalidFlushBuffer
23+
PHPUnit\Framework\Exception: Test code or tested code flushed or cleaned global output buffers other than its own
24+
25+
2) PHPUnit\TestFixture\Issue5851Test::testInvalidSilencedFlushBuffer
26+
PHPUnit\Framework\Exception: Test code or tested code flushed or cleaned global output buffers other than its own
27+
28+
3) PHPUnit\TestFixture\Issue5851Test::testInvalidFlushBufferEmpty
29+
PHPUnit\Framework\Exception: Test code or tested code flushed or cleaned global output buffers other than its own
30+
31+
4) PHPUnit\TestFixture\Issue5851Test::testInvalidCleanExternalBuffer
32+
PHPUnit\Framework\Exception: Test code or tested code flushed or cleaned global output buffers other than its own
33+
34+
5) PHPUnit\TestFixture\Issue5851Test::testRemovedAndAddedBufferNoOutput
35+
PHPUnit\Framework\Exception: Test code or tested code first closed output buffers other than its own and later started output buffers it did not close
36+
37+
6) PHPUnit\TestFixture\Issue5851Test::testRemovedAndAddedBufferOutput
38+
PHPUnit\Framework\Exception: Test code or tested code first closed output buffers other than its own and later started output buffers it did not close
39+
40+
7) PHPUnit\TestFixture\Issue5851Test::testRemovedAndAddedBufferExpectedOutput
41+
PHPUnit\Framework\Exception: Test code or tested code first closed output buffers other than its own and later started output buffers it did not close
42+
43+
8) PHPUnit\TestFixture\Issue5851Test::testNonClosedBufferShouldntBeIgnored
44+
Failed asserting that two strings are identical.
45+
--- Expected
46+
+++ Actual
47+
@@ @@
48+
-''
49+
+'Do not ignore thisor this'
50+
51+
9) PHPUnit\TestFixture\Issue5851Test::testNonRemovableBuffer
52+
PHPUnit\Framework\Exception: Test code contains a non-removable output buffer - run test in separate process to avoid side-effects
53+
54+
10) PHPUnit\TestFixture\Issue5851Test::testNonRemovableBufferChunkSizeTooLow
55+
PHPUnit\Framework\Exception: Tests with non-removable output buffer handlers must not call flush on them and the chunk size must be bigger than the expected output
56+
57+
--
58+
59+
There were 4 risky tests:
60+
61+
1) PHPUnit\TestFixture\Issue5851Test::testNonClosedBufferShouldntBeIgnored
62+
Test code or tested code did not close its own output buffers
63+
64+
%s%eIssue5851Test.php:%i
65+
66+
2) PHPUnit\TestFixture\Issue5851Test::testNonClosedBufferShouldntBeIgnored2
67+
Test code or tested code did not close its own output buffers
68+
69+
%s%eIssue5851Test.php:%i
70+
71+
3) PHPUnit\TestFixture\Issue5851Test::testNonRemovableBufferSeparateProcess
72+
Non-removable output handler callback was not called, which could alter output
73+
74+
%s%eIssue5851Test.php:%i
75+
76+
4) PHPUnit\TestFixture\Issue5851Test::testNonRemovableBufferSeparateProcessAgain
77+
Non-removable output handler callback was not called, which could alter output
78+
79+
%s%eIssue5851Test.php:%i
80+
81+
FAILURES!
82+
Tests: 14, Assertions: 14, Failures: 10, Risky: 4.

0 commit comments

Comments
 (0)