Skip to content

Commit 2d4895f

Browse files
kAlvarokAlvaro
authored
Fix #19 - Print all error messages (#20)
* Tested that duplicate warnings are reported once. * Fixed reporting of warning/notice/deprecation so all unique issues are printed. * Fixed carriage return generation when printing issues. * Simplified issue hash. * Removed unnecessary variable. * Fixed carriage return generation when printing issues. * Removed redundant silenced warning from capabilities test. * Documented direct addition of Risky event trace to unique traces array. * Separated test to test the interaction between silenced and non-silenced traces. * Refactored test to reuse private method. * Replaced inline warning in test with helper method.
1 parent 3111fb5 commit 2d4895f

File tree

8 files changed

+198
-18
lines changed

8 files changed

+198
-18
lines changed

‎src/Printer.php‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,14 +32,22 @@ final class Printer implements Tracer
3232

3333
private ?Throwable $throwable = null;
3434

35-
private ?Trace $trace = null;
35+
/**
36+
* @var array<string, Trace>
37+
*/
38+
private array $uniqueTraces = [];
3639

3740
private bool $flawless = true;
3841

3942
public function __construct(private readonly PipConfig $config)
4043
{
4144
}
4245

46+
private function addUniqueTrace(Trace $trace): void
47+
{
48+
$this->uniqueTraces[$trace->getIssueId()] = $trace;
49+
}
50+
4351
public function trace(Event $event): void
4452
{
4553
if ($event instanceof ExecutionStarted) {
@@ -77,28 +85,29 @@ public function trace(Event $event): void
7785
$this->status = TestStatus::Risky;
7886
}
7987

80-
$this->trace = new Trace($event->message(), $event->test()->file(), $event->test()->line());
88+
// No duplicate handling needed. Risky is a test status (rather than issue status) and it's final.
89+
$this->uniqueTraces[] = new Trace(TestStatus::Risky, $event->message(), $event->test()->file(), $event->test()->line());
8190
}
8291
if ($event instanceof PhpNoticeTriggered) {
8392
if (!$event->wasSuppressed()) {
8493
$this->status ??= TestStatus::Notice;
8594
}
8695

87-
$this->trace = Trace::fromEvent($event);
96+
$this->addUniqueTrace(Trace::fromEvent($event));
8897
}
8998
if ($event instanceof PhpWarningTriggered) {
9099
if (!$event->wasSuppressed()) {
91100
$this->status ??= TestStatus::Warning;
92101
}
93102

94-
$this->trace = Trace::fromEvent($event);
103+
$this->addUniqueTrace(Trace::fromEvent($event));
95104
}
96105
if ($event instanceof PhpDeprecationTriggered) {
97106
if (!$event->wasSuppressed()) {
98107
$this->status ??= TestStatus::Deprecated;
99108
}
100109

101-
$this->trace = Trace::fromEvent($event);
110+
$this->addUniqueTrace(Trace::fromEvent($event));
102111
}
103112

104113
if ($event instanceof Finished) {
@@ -135,10 +144,11 @@ public function trace(Event $event): void
135144
$ms,
136145
$performance,
137146
$this->throwable,
138-
$this->trace,
147+
$this->uniqueTraces
139148
));
140149

141-
$this->trace = $this->throwable = $this->status = null;
150+
$this->uniqueTraces = [];
151+
$this->throwable = $this->status = null;
142152
}
143153

144154
if ($event instanceof \PHPUnit\Event\TestRunner\Finished) {

‎src/TestResult.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@ public function __construct(
1515
public readonly int $testDurationMs,
1616
public readonly TestPerformance $testPerformance,
1717
public readonly ?Throwable $throwable,
18-
public readonly ?Trace $trace,
18+
/** @var array<string, Trace> */
19+
public readonly array $uniqueTraces,
1920
) {
2021
}
2122

‎src/Theme/ClassicTheme.php‎

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use ScriptFUSION\Pip\TestPerformance;
88
use ScriptFUSION\Pip\TestResult;
99
use ScriptFUSION\Pip\TestStatus;
10+
use ScriptFUSION\Pip\Trace;
1011

1112
final class ClassicTheme implements Theme
1213
{
@@ -46,15 +47,21 @@ public function onTestFinished(TestResult $result): void
4647
$throwable = $throwable->previous();
4748
}
4849

49-
if ($result->trace && !$result->trace->suppressed) {
50-
printf(
51-
Color::colorize("fg-$statusColour", '%s%s: %s in %s on line %s%1$s%1$s'),
52-
PHP_EOL,
53-
$result->status->name,
54-
$result->trace->message,
55-
$result->trace->file,
56-
$result->trace->line,
57-
);
50+
$firstIssue = true;
51+
foreach ($result->uniqueTraces as $trace) {
52+
if (!$trace->suppressed) {
53+
$issueStatusColour = self::getColour($trace->issueStatus);
54+
printf(
55+
Color::colorize("fg-$issueStatusColour", '%s%s: %s in %s on line %d%s'),
56+
$firstIssue ? PHP_EOL : '',
57+
$trace->issueStatus->name,
58+
$trace->message,
59+
$trace->file,
60+
$trace->line,
61+
PHP_EOL . PHP_EOL,
62+
);
63+
$firstIssue = false;
64+
}
5865
}
5966
}
6067

‎src/Trace.php‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
final class Trace
1111
{
1212
public function __construct(
13+
public readonly TestStatus $issueStatus,
1314
public readonly string $message,
1415
public readonly string $file,
1516
public readonly int $line,
@@ -18,6 +19,31 @@ public function __construct(
1819

1920
public static function fromEvent(PhpWarningTriggered|PhpNoticeTriggered|PhpDeprecationTriggered $event): self
2021
{
21-
return new self($event->message(), $event->file(), $event->line(), $event->wasSuppressed());
22+
return new self(
23+
match (true) {
24+
$event instanceof PhpWarningTriggered => TestStatus::Warning,
25+
$event instanceof PhpNoticeTriggered => TestStatus::Notice,
26+
$event instanceof PhpDeprecationTriggered => TestStatus::Deprecated,
27+
},
28+
$event->message(),
29+
$event->file(),
30+
$event->line(),
31+
$event->wasSuppressed(),
32+
);
33+
}
34+
35+
/**
36+
* Key to identify identical issues, using the same rules as PHPUnit.
37+
*
38+
* @see \PHPUnit\TestRunner\TestResult\Collector::issueId
39+
*/
40+
public function getIssueId(): string
41+
{
42+
return sprintf(
43+
'%s:%s:%s',
44+
$this->file,
45+
$this->line,
46+
$this->message,
47+
);
2248
}
2349
}

‎test/CapabilitiesTest.php‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,55 @@ public function testSilencedDeprecation(): void
9898
self::assertTrue(true);
9999
}
100100

101+
private function triggerWarning(): void
102+
{
103+
foreach (1 as $n) {}
104+
}
105+
106+
public function testDuplicateWarningsA(): void
107+
{
108+
$this->triggerWarning();
109+
110+
self::assertTrue(true);
111+
}
112+
113+
public function testDuplicateWarningsB(): void
114+
{
115+
$this->triggerWarning();
116+
$this->triggerWarning();
117+
$this->triggerWarning();
118+
119+
foreach (1 as $n) {}
120+
121+
self::assertTrue(true);
122+
}
123+
124+
public function testDuplicateWarningsC(): void
125+
{
126+
self::assertTrue(true);
127+
}
128+
129+
public function testMixedSeverities(): void
130+
{
131+
// Notice.
132+
$foo = &self::provideData();
133+
$this->triggerWarning();
134+
// Deprecated.
135+
trim(null);
136+
137+
self::assertTrue(true);
138+
}
139+
140+
public function testSilencedWarningNotAffectsStatus(): void
141+
{
142+
@$this->triggerWarning();
143+
144+
// Notice.
145+
$foo = &self::provideData();
146+
147+
self::assertTrue(true);
148+
}
149+
101150
#[DataProvider('provideData')]
102151

103152
public function testDataProvider(): void

‎test/functional/duplicate warnings.phpt‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
--TEST--
2+
PHPUnit only reports warnings with globally unique locations within executed tests.
3+
4+
--ARGS--
5+
-c test --colors=always test/CapabilitiesTest.php --filter ::testDuplicateWarnings
6+
7+
--FILE_EXTERNAL--
8+
../PHPUnit runner.php
9+
10+
--EXPECTF--
11+
PHPUnit %s
12+
13+
Runtime: %s
14+
Configuration: %s
15+
16+
33% W ScriptFUSIONTest\Pip\CapabilitiesTest::testDuplicateWarningsA (%d ms)
17+

18+
Warning: foreach() argument must be of type array|object, int given in %s%eCapabilitiesTest.php on line %d
19+
20+
 66% W ScriptFUSIONTest\Pip\CapabilitiesTest::testDuplicateWarningsB (%d ms)
21+

22+
Warning: foreach() argument must be of type array|object, int given in %s%eCapabilitiesTest.php on line %d
23+
24+
Warning: foreach() argument must be of type array|object, int given in %s%eCapabilitiesTest.php on line %d
25+
26+
100% . ScriptFUSIONTest\Pip\CapabilitiesTest::testDuplicateWarningsC (%d ms)
27+
28+
29+
Time: %s
30+
%A
31+
OK, but %s!
32+
Tests: 3, Assertions: 3, Warnings: 2.

‎test/functional/mixed severities.phpt‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
--TEST--
2+
When a test generates errors with different severities and silences some of them, first non-silenced error determines
3+
overall test status and all non-silenced messages are shown.
4+
5+
--ARGS--
6+
-c test --colors=always test/CapabilitiesTest.php --filter ::testMixedSeverities$
7+
8+
--FILE_EXTERNAL--
9+
../PHPUnit runner.php
10+
11+
--EXPECTF--
12+
PHPUnit %s
13+
14+
Runtime: %s
15+
Configuration: %s
16+
17+
100% N ScriptFUSIONTest\Pip\CapabilitiesTest::testMixedSeverities (%d ms)
18+

19+
Notice: Only variables should be assigned by reference in %s%eCapabilitiesTest.php on line %d
20+
21+
Warning: foreach() argument must be of type array|object, int given in %s%eCapabilitiesTest.php on line %d
22+
23+
Deprecated: trim(): Passing null to parameter #1 ($string) of type string is deprecated in %s%eCapabilitiesTest.php on line %d
24+
25+

26+
27+
Time: %s
28+
%A
29+
OK, but %s!
30+
Tests: 1, Assertions: 1, Warnings: 1, Deprecations: 1, Notices: 1.

‎test/functional/status notice warning silenced.phpt‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
--TEST--
2+
Test that first notice determines test status and prior silenced warning does not.
3+
4+
--ARGS--
5+
-c test --colors=always test/CapabilitiesTest.php --filter ::testSilencedWarningNotAffectsStatus$
6+
7+
--FILE_EXTERNAL--
8+
../PHPUnit runner.php
9+
10+
--EXPECTF--
11+
PHPUnit %s
12+
13+
Runtime: %s
14+
Configuration: %s
15+
16+
100% N ScriptFUSIONTest\Pip\CapabilitiesTest::testSilencedWarningNotAffectsStatus (%d ms)
17+

18+
Notice: Only variables should be assigned by reference in %s%eCapabilitiesTest.php on line %d
19+
20+

21+
22+
Time: %s
23+
%A
24+
OK, but %s!
25+
Tests: 1, Assertions: 1, Notices: 1.

0 commit comments

Comments
 (0)