Skip to content

Commit d62bb5b

Browse files
committed
feat: tell the developer how to see warning detail
Without displayDetailsOnTestsThatTriggerWarnings PHPUnit prints only a count — 'Warnings: 11' — which is visible but not actionable. The first warning of a run now carries the pointer to --display-warnings and the config setting. Once per run, not per finding: tests run with processIsolation, so each test is a separate process and a static flag cannot track 'first'. A marker file keyed to the project and the hour serves as the shared signal.
1 parent 7539d84 commit d62bb5b

2 files changed

Lines changed: 50 additions & 2 deletions

File tree

php-packages/testing/src/integration/RepeatedQueryDetector.php

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,49 @@ private static function isWorthCounting(string $sql): bool
117117
return (bool) preg_match('/^\s*(select|insert|update|delete)\b/i', $sql);
118118
}
119119

120+
/**
121+
* A pointer, appended to the first warning of a run, explaining how to see
122+
* the detail.
123+
*
124+
* PHPUnit only prints warning messages when the phpunit config sets
125+
* `displayDetailsOnTestsThatTriggerWarnings`; without it the run reports a
126+
* bare count. Since that is the default in most extensions, the first
127+
* warning has to carry its own instructions — and only the first, or it
128+
* repeats on every finding.
129+
*
130+
* Tests usually run with `processIsolation`, so "first" cannot be tracked
131+
* in memory: each test is a fresh process. A marker file next to the
132+
* findings log gives one hint per run instead of one per process.
133+
*/
134+
public static function hintOnce(): string
135+
{
136+
$hint = "\n\nRun with --display-warnings, or set"
137+
."\ndisplayDetailsOnTestsThatTriggerWarnings=\"true\" in your phpunit config, to see"
138+
."\nwhich queries these were.";
139+
140+
// Keyed to the project and to the hour, so every isolated test process
141+
// in a run agrees on what "first" means without needing cleanup that a
142+
// per-test process can't reliably perform. A later run gets a fresh
143+
// hint; a run straddling the hour boundary may hint twice, which is a
144+
// fair trade for not carrying state between processes.
145+
$marker = sprintf(
146+
'%s/flarum-repeated-queries-hinted-%s',
147+
sys_get_temp_dir(),
148+
md5((string) realpath('.').gmdate('YmdH'))
149+
);
150+
151+
// `x` mode succeeds only for whoever gets there first.
152+
$handle = @fopen($marker, 'x');
153+
154+
if ($handle === false) {
155+
return '';
156+
}
157+
158+
fclose($handle);
159+
160+
return $hint;
161+
}
162+
120163
/**
121164
* Record a finding for tooling to pick up after the run.
122165
*

php-packages/testing/src/integration/TestCase.php

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -408,10 +408,15 @@ private function reportRepeatedQueries(ServerRequestInterface $request, array $q
408408
// Raised as a PHP warning: PHPUnit surfaces these against the test
409409
// that triggered them (with `displayDetailsOnTestsThatTriggerWarnings`
410410
// it prints the detail) without failing the run.
411+
//
412+
// Without that setting PHPUnit reports only a count — "Warnings: 11"
413+
// — so the first warning of the run says how to see the rest.
414+
// Otherwise the default experience is a number with no next step.
411415
trigger_error(sprintf(
412-
"%s repeated queries for the same few values — consider memoising.\n%s",
416+
"%s repeated queries for the same few values — consider memoising.\n%s%s",
413417
$where,
414-
RepeatedQueryDetector::describe($wasteful)
418+
RepeatedQueryDetector::describe($wasteful),
419+
RepeatedQueryDetector::hintOnce()
415420
), E_USER_WARNING);
416421
}
417422

0 commit comments

Comments
 (0)