Skip to content

Fix ArgumentCountError in ReportPrinter when test names contain % characters#6927

Open
Fahl-Design wants to merge 6 commits into
Codeception:mainfrom
Fahl-Design:fix/test-reporter-output-fails-on-dataset-with-percent
Open

Fix ArgumentCountError in ReportPrinter when test names contain % characters#6927
Fahl-Design wants to merge 6 commits into
Codeception:mainfrom
Fahl-Design:fix/test-reporter-output-fails-on-dataset-with-percent

Conversation

@Fahl-Design

@Fahl-Design Fahl-Design commented Feb 23, 2026

Copy link
Copy Markdown
Contributor

Description

This PR fixes a bug in Codeception\Reporter\ReportPrinter where an ArgumentCountError is thrown if a test name contains a percent character (%), and the test suite is executed with the --report flag.

Root Cause

ReportPrinter->printTestResult() retrieves the test name and passes it directly to $this->message(), which internally acts as a wrapper for sprintf(...func_get_args()). When a test name contains % (e.g., from a DataProvider outputting "100% coverage" or "test with %s"), sprintf tries to interpret it as a format specifier. Since no additional arguments are passed, it panics and throws a fatal ArgumentCountError.

Solution

Before passing the test name into $this->message(), any lone % character is properly escaped to %%. This tells sprintf to treat the percent sign as a literal string character rather than a formatting placeholder. To prevent double-escaping (if the test name somehow already contained %%), the logic safely converts %% back to % before escaping them.

Testing

A new comprehensive test suite tests/unit/Codeception/Reporter/ReportPrinterTest.php has been added.
Using a @dataProvider, it successfully verifies the correct escaping behavior for:

  • Basic percentage strings: 'testWith100%Coverage'
  • Lone percents: '%'
  • Multiple percents: 'there are %%% from %%% cases solved'
  • Sprintf formatting tokens: 'string with %s format', %d, %f, %x, etc.

Test Command Run:

./codecept run unit tests/unit/Codeception/Reporter/ReportPrinterTest.php
XDEBUG_MODE=coverage ./codecept run unit --report

All tests pass successfully.

(bug found and fixed by human (me), reviewed, tests written and pr text by Gemini 3.1 Pro)

@Fahl-Design
fix: add failing tests to show escape percent symbols in test names c…
01ba9ab 
…ause ArgumentCountError

Signed-off-by: Benjamin Fahl <git@fahl-design.de>
@Fahl-Design
fix: prevent unescaped escape percent symbols in test names to cause …
506ca01 
…ArgumentCountError

Signed-off-by: Benjamin Fahl <git@fahl-design.de>
@Fahl-Design Fahl-Design mentioned this pull request Feb 23, 2026
Open
@Fahl-Design Fahl-Design mentioned this pull request Mar 5, 2026
Open
@Fahl-Design

Fahl-Design commented Apr 13, 2026

Copy link
Copy Markdown
Contributor Author

hey @burned42 , can you please review and merge this anytime soon? It's just a small fix with tests to verify ✌️

@burned42

burned42 commented Apr 16, 2026

Copy link
Copy Markdown
Contributor

@Fahl-Design I can take a look, sure, but my review does not necessarily help move this PR forward as I don't have any permissions in this repo 😅

Generally the PR looks fine to me. Having actual assertions instead of the dummy "true === true" check would be nice, but I'm not sure if that's possible.

There's only one thing I don't yet understand and that's the double-escape prevention. In case the test name contains %%, shouldn't we still keep it as is? With the line $name = str_replace('%%', '%', $name); we're basically changing the name for the output and I don't really see a reason for it. Without that line the name seems to get printed exactly as provided, which I feel like is the expected behavior. So IMHO we can probably drop that line from the PR.

@Fahl-Design

Fahl-Design commented Apr 16, 2026

Copy link
Copy Markdown
Contributor Author

@Fahl-Design I can take a look, sure, but my review does not necessarily help move this PR forward as I don't have any permissions in this repo 😅

Generally the PR looks fine to me. Having actual assertions instead of the dummy "true === true" check would be nice, but I'm not sure if that's possible.

There's only one thing I don't yet understand and that's the double-escape prevention. In case the test name contains %%, shouldn't we still keep it as is? With the line $name = str_replace('%%', '%', $name); we're basically changing the name for the output and I don't really see a reason for it. Without that line the name seems to get printed exactly as provided, which I feel like is the expected behavior. So IMHO we can probably drop that line from the PR.

Oh sorry^^ and thanks anyway to take a look at it. Somehow i mixed up the names.

I think I got all '%' cases now, I'm still surprised that nobody ran into this before, maybe not many use dataProvider with % in case name and than use the "--report" flag

  • the assertTrue is useless for sure, i replaced it with "expectNotToPerformAssertions" to make the tests go green
  • and added some more test cases

maybe @TavoNiievez can help to get this merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Fahl-Design @burned42