Skip to content

feat(phpt): add PHP .phpt filter for php-src run-tests.php (-99%) - #1503

Merged
KuSh merged 4 commits into
rtk-ai:developfrom
iliaal:phpt-support
Aug 31, 2026
Merged

KuSh merged 4 commits into
rtk-ai:developfrom
iliaal:phpt-support

Conversation

@iliaal

@iliaal iliaal commented Apr 24, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Adds rtk phpt, a filter for php run-tests.php. It's the driver php-src and PHP extensions use for their .phpt test suite.

A full php-src run emits one status line per test plus a ========DIFF======== block per failure. On a 5322-test run that's about 1.3 MB of output, 99%+ of which is PASS chatter. After filtering:

Scenario Raw Filtered Reduction
php-src full run (4738 PASS, 467 FAIL, 116 SKIP, 1 XFAIL) 1,301,105 B 6,279 B 99.5%
Single passing test ~200 B 2 lines ~85%
Small all-pass dir (11 tests) ~4 KB 2 lines ~97%

Filtered output keeps what the agent needs: aggregate counts, PHP version/SAPI/OS, and up to 20 failures with the first 6 diff lines each.

Example

phpt: 4738 passed, 467 failed, 116 skipped, 1 xfail  (5322 total, 391.4s)
PHP 8.4.20  SAPI cli  OS Linux

FAILURES (467):
  Zend/tests/__debugInfo_reference.phpt -- __debugInfo with reference return
    001- object(Test)#1 (1) {
    002-   ["x"]=>
    003-   int(1)
    004- }
    001+ Fatal error: __debuginfo() must return an array in /home/.../__debugInfo_reference.php on line 12

  Zend/tests/abstract_implicit.phpt -- Abstract methods not allowed in classes (GH-16067)
    001- Fatal error: Class NotAbstract declares abstract method bar() ... in %s on line %d
    001+ Fatal error: Class NotAbstract contains 1 abstract method ... on line 11

... +447 more failures

Usage

cd /path/to/php-src
rtk phpt                           # full suite
rtk phpt Zend/tests/                # directory
rtk phpt Zend/tests/67468.phpt     # single test
rtk phpt -j8 ext/standard/         # args pass through to run-tests.php

Implementation

src/cmds/php/phpt_cmd.rs is a new filter module modeled on python/pytest_cmd.rs: stateful line scan, regex anchors, inline tests.

Status tokens match via (?:^|\])(PASS|FAIL|SKIP|BORK|WARN|LEAK|XFAIL)\s+(.*?)\s*\[([^\[\]]+\.phpt)\]. The (?:^|\]) anchor is what prevents the trailing FAILED TEST SUMMARY block from being re-counted. Its lines are description [path] with no status tag.

ANSI is stripped via the existing utils::strip_ansi. Execution goes through runner::run_filtered with stdout_only().tee("phpt"), same as pytest. The command is added to the is_operational_command whitelist so it flows through the hook pipeline.

Tests

12 inline unit tests:

  • All-pass run
  • Single FAIL with DIFF block
  • Diff truncation (more than 6 lines)
  • Failure-list truncation (more than 20 failures)
  • SKIP and XFAIL don't produce a FAILURES section
  • BORK counts as a failure
  • Empty run (no tests ran)
  • FAILED TEST SUMMARY block doesn't re-count failures
  • ANSI stripping
  • Compression ratio on a synthesized 1000-test pass run (>99%)
  • PHP_OS kernel info trimmed to just the OS family
  • Summary-line counts override scanned counts when capture is truncated

Test plan

  • cargo test -p rtk phpt passes 12/12
  • cargo fmt --all -- --check clean
  • cargo clippy -p rtk no new warnings in diff (pre-existing errors unchanged)
  • Live rtk phpt Zend/tests/67468.phpt on real php-src, correct output
  • Live rtk phpt Zend/tests/ArrayAccess/ (11 tests), correct output
  • Captured 5322-test output run through filter: 99.5% reduction, 467 failures with diffs

@CLAassistant

CLAassistant commented Apr 24, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@pszymkowiak pszymkowiak added effort-medium 1-2 jours, quelques fichiers enhancement New feature or request labels Apr 24, 2026
@pszymkowiak

Copy link
Copy Markdown
Collaborator

[w] wshm · Automated triage by AI

📊 Automated PR Analysis

✨ Type feature
🟢 Risk low

Summary

Adds a new rtk phpt filter command for PHP's run-tests.php test runner, reducing output by ~99% by keeping only aggregate counts, environment info, and bounded failure diffs. The implementation follows the existing pytest filter pattern with a stateful line scanner, regex-based parsing, and 12 inline unit tests.

Review Checklist

  • Tests present
  • Breaking change
  • Docs updated

Analyzed automatically by wshm · This is an automated analysis, not a human review.

@iliaal
iliaal changed the base branch from master to develop April 24, 2026 17:35
iliaal added a commit to iliaal/rtk that referenced this pull request May 17, 2026
…pest, paratest, ecs, pint)

Consolidates the PHP-tooling work from three upstream PRs plus a new
Pint module, leaving phpt to its own PR (rtk-ai#1503).

- rtk php / rtk artisan: syntax check (-l) and Laravel artisan wrapper.
- rtk phpunit: structured-state parser, aggregate counts, bounded
  failure list. Uses runner::run_filtered.
- rtk phpstan: typed serde::Deserialize parser for --error-format=json,
  groups errors by file, sorts by count desc. Utility commands
  (--version, list, clear-result-cache) pass through unchanged.
- rtk pest / rtk paratest: shared test_output helper.
- rtk ecs / rtk pint: code-style fixers; pint uses --format=json for
  structured per-file rule counts.

Composer custom-bin-dir detection: composer_bin_dirs() reads
COMPOSER_BIN_DIR and composer.json config.bin-dir, so tools/bin/phpunit
classifies identically to vendor/bin/phpunit. registry.rs normalizes
tool paths before matching.

Sources:
- rtk-ai#1246 (aaronflorey, self-closed): php, artisan, ecs, pest,
  paratest, test_output, utils, composer_bin_dirs, registry normalization.
- rtk-ai#874 (Beninho, open): phpunit state-machine parser.
- rtk-ai#1110 (LucianoVandi, open): phpstan typed parser.
- New: pint_cmd.rs.

Tests: discover::registry 253 pass; cmds::php 36 pass; cargo build
--release 0 errors; cargo fmt --check clean.
iliaal added a commit to iliaal/rtk that referenced this pull request May 30, 2026
…pest, paratest, ecs, pint)

Consolidates the PHP-tooling work from three upstream PRs plus a new
Pint module, leaving phpt to its own PR (rtk-ai#1503).

- rtk php / rtk artisan: syntax check (-l) and Laravel artisan wrapper.
- rtk phpunit: structured-state parser, aggregate counts, bounded
  failure list. Uses runner::run_filtered.
- rtk phpstan: typed serde::Deserialize parser for --error-format=json,
  groups errors by file, sorts by count desc. Utility commands
  (--version, list, clear-result-cache) pass through unchanged.
- rtk pest / rtk paratest: shared test_output helper.
- rtk ecs / rtk pint: code-style fixers; pint uses --format=json for
  structured per-file rule counts.

Composer custom-bin-dir detection: composer_bin_dirs() reads
COMPOSER_BIN_DIR and composer.json config.bin-dir, so tools/bin/phpunit
classifies identically to vendor/bin/phpunit. registry.rs normalizes
tool paths before matching.

Sources:
- rtk-ai#1246 (aaronflorey, self-closed): php, artisan, ecs, pest,
  paratest, test_output, utils, composer_bin_dirs, registry normalization.
- rtk-ai#874 (Beninho, open): phpunit state-machine parser.
- rtk-ai#1110 (LucianoVandi, open): phpstan typed parser.
- New: pint_cmd.rs.

Tests: discover::registry 253 pass; cmds::php 36 pass; cargo build
--release 0 errors; cargo fmt --check clean.
iliaal added a commit to iliaal/rtk that referenced this pull request Jun 9, 2026
…pest, paratest, ecs, pint)

Consolidates the PHP-tooling work from three upstream PRs plus a new
Pint module, leaving phpt to its own PR (rtk-ai#1503).

- rtk php / rtk artisan: syntax check (-l) and Laravel artisan wrapper.
- rtk phpunit: structured-state parser, aggregate counts, bounded
  failure list. Uses runner::run_filtered.
- rtk phpstan: typed serde::Deserialize parser for --error-format=json,
  groups errors by file, sorts by count desc. Utility commands
  (--version, list, clear-result-cache) pass through unchanged.
- rtk pest / rtk paratest: shared test_output helper.
- rtk ecs / rtk pint: code-style fixers; pint uses --format=json for
  structured per-file rule counts.

Composer custom-bin-dir detection: composer_bin_dirs() reads
COMPOSER_BIN_DIR and composer.json config.bin-dir, so tools/bin/phpunit
classifies identically to vendor/bin/phpunit. registry.rs normalizes
tool paths before matching.

Sources:
- rtk-ai#1246 (aaronflorey, self-closed): php, artisan, ecs, pest,
  paratest, test_output, utils, composer_bin_dirs, registry normalization.
- rtk-ai#874 (Beninho, open): phpunit state-machine parser.
- rtk-ai#1110 (LucianoVandi, open): phpstan typed parser.
- New: pint_cmd.rs.

Tests: discover::registry 253 pass; cmds::php 36 pass; cargo build
--release 0 errors; cargo fmt --check clean.
@iliaal iliaal closed this Jun 18, 2026
@iliaal
iliaal deleted the phpt-support branch June 18, 2026 16:16
@iliaal
iliaal restored the phpt-support branch June 18, 2026 16:21
@iliaal iliaal reopened this Jun 18, 2026
iliaal added a commit to iliaal/rtk that referenced this pull request Jun 26, 2026
…pest, paratest, ecs, pint)

Consolidates the PHP-tooling work from three upstream PRs plus a new
Pint module, leaving phpt to its own PR (rtk-ai#1503).

- rtk php / rtk artisan: syntax check (-l) and Laravel artisan wrapper.
- rtk phpunit: structured-state parser, aggregate counts, bounded
  failure list. Uses runner::run_filtered.
- rtk phpstan: typed serde::Deserialize parser for --error-format=json,
  groups errors by file, sorts by count desc. Utility commands
  (--version, list, clear-result-cache) pass through unchanged.
- rtk pest / rtk paratest: shared test_output helper.
- rtk ecs / rtk pint: code-style fixers; pint uses --format=json for
  structured per-file rule counts.

Composer custom-bin-dir detection: composer_bin_dirs() reads
COMPOSER_BIN_DIR and composer.json config.bin-dir, so tools/bin/phpunit
classifies identically to vendor/bin/phpunit. registry.rs normalizes
tool paths before matching.

Sources:
- rtk-ai#1246 (aaronflorey, self-closed): php, artisan, ecs, pest,
  paratest, test_output, utils, composer_bin_dirs, registry normalization.
- rtk-ai#874 (Beninho, open): phpunit state-machine parser.
- rtk-ai#1110 (LucianoVandi, open): phpstan typed parser.
- New: pint_cmd.rs.

Tests: discover::registry 253 pass; cmds::php 36 pass; cargo build
--release 0 errors; cargo fmt --check clean.

@KuSh KuSh left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @iliaal, you'll have to rebase your work now that your other PR has been merged. Few nitpicks and suggestions

Comment thread src/cmds/php/phpt_cmd.rs
Comment thread src/main.rs
Comment thread src/cmds/php/phpt_cmd.rs Outdated
Comment thread src/cmds/php/phpt_cmd.rs Outdated
Comment thread src/cmds/php/phpt_cmd.rs
Comment thread src/cmds/php/phpt_cmd.rs Outdated
@iliaal

iliaal commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto develop — conflict cleared, PR is mergeable again. All six comments addressed in 6aea5c7; details inline. Thanks for the thorough pass.

@iliaal
iliaal requested a review from KuSh July 1, 2026 12:39
@iliaal

iliaal commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

Heads-up on the red benchmark check: it's the flaky live-network endpoint, not this PR. The only failing item is rtk curl https://mockhttp.org/json (38 → 61, flagged negative). This branch doesn't touch the curl filter at all — the diff is phpt-only — so the filter binary is identical to develop's, where the same benchmark passed (curl json 38 → 38, 0 negative) on b52e02b. The difference is purely the live mockhttp.org response between the two runs (the same commit that switched benches to mockhttp.org notes httpbin was "struggling"). All correctness checks — test on macOS/Ubuntu/Windows, clippy, fmt, test presence, security scans — are green. A benchmark re-run should clear it.

@KuSh

KuSh commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

Heads-up on the red benchmark check: it's the flaky live-network endpoint, not this PR.

Yes I'll check the benchmark, I've fixed a reliability problem but introduced a flakiness :/ Sorry about that

@KuSh

KuSh commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

Hi @iliaal, sorry for the delay I was away for 2 weeks. I've reverted the insta part of your code as it was a mistake, our documentation was outdated and I blindly followed it, my bad, sorry.

Everything looks good to me, but in the meanwhile, never_worse and emit_guarded have been induced to avoid emitting more tokens than the raw command, see transparency. Could you rebase your code and take that into account?

Thanks

iliaal and others added 3 commits July 20, 2026 08:49
Wraps `php run-tests.php` and collapses its per-test PASS chatter into
one summary line plus a bounded list of failure diffs. On php-src's
5322-test suite with 467 failures, output drops from 1.3 MB to 6 KB
(99.5% reduction). An all-pass run collapses to two lines.

Handles PASS/FAIL/SKIP/BORK/WARN/LEAK/XFAIL/XLEAK, ANSI color codes, the
========DIFF======== / ========DONE======== blocks, and the trailing
FAILED TEST SUMMARY block (which must not be re-counted). XFAIL and
XLEAK are expected results (reported under "Expected fail"/"Expected
leak" by run-tests.php) and do not count as failures.

The hook rewrite maps `php run-tests.php …` to `rtk phpt …` via a rule
in discover/rules.rs, with `phpt` added to the passthrough classifier.
Diff buffering is capped at the display limit while a separate counter
keeps the "+N more diff lines" note accurate. When the summary block
reports failures but per-test lines were truncated out of the capture,
the FAILURES header is still emitted with a note rather than dropped.

Tests: inline units plus an insta snapshot and a count_tokens-based
token-savings assertion (>=60%), per .claude/rules/cli-testing.md.
It was my fault I shouldn't have made that request
Running the filter against php-src instead of captured fixtures exposed
three defects.

run-tests.php redraws its progress line, so a serial run (no -j) delivers
the status after a carriage return: "TEST 1/2 [x.phpt]\rFAIL desc
[x.phpt]". The status anchor never matched, so every per-test status was
lost and failures printed as "details unavailable". Splitting on CR fixes
it. The original 5322-test validation ran with -j8, where workers suppress
the progress line, which is why this stayed hidden.

php writes its startup diagnostics to stderr ("Could not open input file:
run-tests.php", the illegal-switch help), and stdout_only dropped them.
The never-worse guard then had an empty base, so rtk printed nothing at
all. The filter now reads both streams and passes output through unchanged
when it isn't run-tests.php output.

run-tests.php emits DIFF blocks only with --show-diff or for a single
selected test, so multi-test runs listed failing test names without a
diff. rtk now asks for them unless the caller passed its own --show-* flag.

Verified on php-src: 733-test serial run, 12 failures with diffs, 97.1%
reduction (153884 to 4476 bytes).
@iliaal

iliaal commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto develop (5d32d07), and thanks for reverting the insta commit yourself.

On the guard: phpt goes through runner::run_filtered, which already routes emission via print_with_hint → emit_guarded → guard::never_worse, so the never-worse property holds without a filter change. What the guard did surface is a bug. RunOptions::stdout_only() dropped php's stderr, and php writes its startup diagnostics there, so running rtk phpt from the wrong directory left the guard with an empty base and rtk printed nothing at all. Exit code 1, zero output.

Verifying that against real php-src turned up two more, both mine from the original commit:

Serial runs lost every per-test status. run-tests.php redraws its progress line, so without -j the status arrives after a carriage return: TEST 1/2 [bad.phpt]\rFAIL failing sample [bad.phpt]. The (?:^|\]) anchor never matched, so counts came only from the summary block and the FAILURES section printed "(per-test details unavailable)". My 5322-test validation ran with -j8, and workers suppress the progress line, which is why I missed it. The filter now splits on CR.

No diffs on multi-test runs. run-tests.php emits ========DIFF======== only with --show-diff or when exactly one test is selected, so the diff lines this PR advertises never appeared for a directory run. rtk now passes --show-diff unless you passed your own --show-* flag. That grows raw output, which flatters the savings ratio, so I'd rather flag it than bury it: if you want the filter to stay passive, I'll drop the injection.

Verified on php-src, serial run of ext/standard/tests/strings/:

phpt: 666 passed, 12 failed, 55 skipped  (733 total, 20.9s)
PHP 8.4.23  SAPI cli  OS Linux

FAILURES (12):
  ext/standard/tests/strings/chr_out_of_range.phpt -- chr() with out of range values
    001- Deprecated: chr(): Providing a value not in-between 0 and 255 ...

153,884 → 4,476 bytes, 97.1%. The wrong-directory case now prints Could not open input file: run-tests.php. Two new tests cover the CR shape and the passthrough.

fmt, clippy --all-targets and test --all are clean apart from hooks::rewrite_cmd::tests::unattestable_passthrough::{test_plain_command_still_rewrites,test_fd_dup_redirect_still_rewrites}, which fail identically on a clean develop worktree here.

iliaal added a commit to iliaal/rtk that referenced this pull request Jul 20, 2026
…pest, paratest, ecs, pint)

Consolidates the PHP-tooling work from three upstream PRs plus a new
Pint module, leaving phpt to its own PR (rtk-ai#1503).

- rtk php / rtk artisan: syntax check (-l) and Laravel artisan wrapper.
- rtk phpunit: structured-state parser, aggregate counts, bounded
  failure list. Uses runner::run_filtered.
- rtk phpstan: typed serde::Deserialize parser for --error-format=json,
  groups errors by file, sorts by count desc. Utility commands
  (--version, list, clear-result-cache) pass through unchanged.
- rtk pest / rtk paratest: shared test_output helper.
- rtk ecs / rtk pint: code-style fixers; pint uses --format=json for
  structured per-file rule counts.

Composer custom-bin-dir detection: composer_bin_dirs() reads
COMPOSER_BIN_DIR and composer.json config.bin-dir, so tools/bin/phpunit
classifies identically to vendor/bin/phpunit. registry.rs normalizes
tool paths before matching.

Sources:
- rtk-ai#1246 (aaronflorey, self-closed): php, artisan, ecs, pest,
  paratest, test_output, utils, composer_bin_dirs, registry normalization.
- rtk-ai#874 (Beninho, open): phpunit state-machine parser.
- rtk-ai#1110 (LucianoVandi, open): phpstan typed parser.
- New: pint_cmd.rs.

Tests: discover::registry 253 pass; cmds::php 36 pass; cargo build
--release 0 errors; cargo fmt --check clean.
Comment thread src/cmds/php/phpt_cmd.rs Outdated
Comment thread src/cmds/php/phpt_cmd.rs Outdated
--show-diff was injected unless any --show-* flag was present, but only
--show-diff and --show-all produce the DIFF block. --show-slow, --show-mem,
--show-out, etc. don't, so `rtk phpt --show-slow 1000 …` suppressed the
injected --show-diff and stripped every failure of its actionable diff.

Match only --show-diff/--show-all, print the truthful injected command in
verbose mode, and add a regression test for the --show-slow case.
iliaal added a commit to iliaal/rtk that referenced this pull request Jul 20, 2026
Port of 127de99 (phpt-support / PR rtk-ai#1503 r2). --show-diff was injected
unless any --show-* flag was present, but only --show-diff and --show-all
produce the DIFF block; --show-slow/--show-mem/--show-out don't, so those
flags suppressed the injection and stripped every failure of its diff.

Match only --show-diff/--show-all, print the truthful injected command in
verbose mode, and add a regression test for the --show-slow case.
iliaal added a commit to iliaal/rtk that referenced this pull request Jul 27, 2026
Port of 127de99 (phpt-support / PR rtk-ai#1503 r2). --show-diff was injected
unless any --show-* flag was present, but only --show-diff and --show-all
produce the DIFF block; --show-slow/--show-mem/--show-out don't, so those
flags suppressed the injection and stripped every failure of its diff.

Match only --show-diff/--show-all, print the truthful injected command in
verbose mode, and add a regression test for the --show-slow case.
iliaal added a commit to iliaal/rtk that referenced this pull request Aug 5, 2026
Port of 127de99 (phpt-support / PR rtk-ai#1503 r2). --show-diff was injected
unless any --show-* flag was present, but only --show-diff and --show-all
produce the DIFF block; --show-slow/--show-mem/--show-out don't, so those
flags suppressed the injection and stripped every failure of its diff.

Match only --show-diff/--show-all, print the truthful injected command in
verbose mode, and add a regression test for the --show-slow case.
iliaal added a commit to iliaal/rtk that referenced this pull request Aug 10, 2026
Port of 127de99 (phpt-support / PR rtk-ai#1503 r2). --show-diff was injected
unless any --show-* flag was present, but only --show-diff and --show-all
produce the DIFF block; --show-slow/--show-mem/--show-out don't, so those
flags suppressed the injection and stripped every failure of its diff.

Match only --show-diff/--show-all, print the truthful injected command in
verbose mode, and add a regression test for the --show-slow case.
iliaal added a commit to iliaal/rtk that referenced this pull request Aug 29, 2026
Port of 127de99 (phpt-support / PR rtk-ai#1503 r2). --show-diff was injected
unless any --show-* flag was present, but only --show-diff and --show-all
produce the DIFF block; --show-slow/--show-mem/--show-out don't, so those
flags suppressed the injection and stripped every failure of its diff.

Match only --show-diff/--show-all, print the truthful injected command in
verbose mode, and add a regression test for the --show-slow case.
@KuSh
KuSh merged commit e8541d1 into rtk-ai:develop Aug 31, 2026
11 checks passed
@rtk-release-bot rtk-release-bot Bot mentioned this pull request Aug 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort-medium 1-2 jours, quelques fichiers enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants