Skip to content

ext/readline: write test for readline CLI - #22994

Open
LamentXU123 wants to merge 7 commits into
php:masterfrom
LamentXU123:readline_test
Open

ext/readline: write test for readline CLI#22994
LamentXU123 wants to merge 7 commits into
php:masterfrom
LamentXU123:readline_test

Conversation

@LamentXU123

@LamentXU123 LamentXU123 commented Aug 2, 2026

Copy link
Copy Markdown
Member

ext/readline/readline_cli.c has little tests resulting to a 42% test coverage. There ain't any (or very few) behavioral tests to the whole CLI system. The test coverage is only coming from bug regression tests.

This PR offers basic behavior tests for ext/readline/readline_cli.c

@LamentXU123 LamentXU123 changed the title ext/readline: enhance test coverage ext/readline: enhance readline CLI test coverage Aug 2, 2026
@LamentXU123
LamentXU123 requested a review from iliaal August 2, 2026 08:24
@LamentXU123 LamentXU123 changed the title ext/readline: enhance readline CLI test coverage ext/readline: write test for readline CLI Aug 3, 2026
@LamentXU123
LamentXU123 requested a review from devnexen August 3, 2026 10:44
@devnexen

devnexen commented Aug 3, 2026

Copy link
Copy Markdown
Member

looks good but please wait for someone else review, I m definitely not the most appropriate person for this extension.

@LamentXU123
LamentXU123 requested review from arnaud-lb and removed request for devnexen August 3, 2026 16:09
@iliaal

iliaal commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

php -a echoes stdin back to stdout, so %Atoken%A matches the echoed input as well as the evaluated output. Three of these expectations pass regardless of what the shell does; details inline.

One of them is masking a real bug. readline_cli_completion_readline leaves Uncaught Error: Undefined constant ReadlineCliCompletionClass::COMPLETION_CLASS_CON in its own output and still passes. readline_cli.c:560 passes text, the whole ClassName::PREFIX, to the constant generator while textlen is already cut to the post-:: length, so constants match against the leading characters of the class name. The function branch passes lc_text and works:

php > class Foo { const FooBar = "MATCHED\n"; const Zed = "zed\n"; }
php > echo Foo::Ze        (tab)
php > echo Foo::ZeFooBar;
MATCHED

Foo::Ze should give Zed. Separate issue, but it is why that expectation has nothing to bite on.

proc_close($proc);
?>
--EXPECTF--
%Apager output%A

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This expectation cannot fail. Set cli.pager to a command that never runs and pager output still appears, echoed from the input line.

A pager that transforms its input pins it instead:

-d cli.pager='tr a-z A-Z'

with %APAGER OUTPUT%A, which nothing but the pager can produce.

proc_close($proc);
?>
--EXPECTF--
%AInteractive shell%Asingle%Adouble%APAREN%A3%Ablock%Acomment%Aattribute%Aoutside-%Ainside%A

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

single, double, block, comment, attribute, inside and outside- all appear in the lines written to stdin, so they match the echo whether or not the shell evaluates anything.

PAREN is the one that holds, because strtoupper() makes the output differ from the source. Same trick on the rest would make the file assert what it says it does.

%A3%A is also thin on its own, since a single digit matches a line number or a version string. echo 6/2 + 100; and 103 is harder to hit by accident.

public static function completionMethod() { echo "method\n"; }
}
echo ReadlineCliCompletionCla ::class . "\n";
echo ReadlineCliCompletionClass::COMPLETION_CLASS_CON ;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The class constant case does not complete. This run leaves Uncaught Error: Undefined constant ReadlineCliCompletionClass::COMPLETION_CLASS_CON in the output and the test still passes, since class constant is matched against the echoed public const line above it.

Giving the constant a value that differs from its declaration, say "CLASSCONST_OK\n", separates the two. The underlying completion bug is noted in the main thread.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is a bug in the readline extension. I will deal this in a separate PR to make git log useful, as the main purpose of this PR is to add behavioral tests to the readline CLI.

@LamentXU123
LamentXU123 requested a review from iliaal August 4, 2026 04:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants