Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

how to properly escape a string for use in --filter #5706

Open
staabm opened this issue Feb 18, 2024 · 8 comments
Open

how to properly escape a string for use in --filter #5706

staabm opened this issue Feb 18, 2024 · 8 comments
Labels
type/bug Something is broken

Comments

@staabm
Copy link
Contributor

staabm commented Feb 18, 2024

Q A
PHPUnit version 9.5.23
PHP version 8.2.16
Installation Method Composer / PHAR

Summary

we try to feed phpunit --filter with test-case names which we determine via --list-tests-xml.
we are wondering, how to properly escape our input strings we want to pass to phpunit via --filter option.

Current behavior

the docs mention the filter is interpreted as a regex, therefore I went with preg_quote(), but this didn't work when we use a "full qualified test-case name" which contains a #.

How to reproduce

a test-class like

<?php
namespace TestNamespace;

use PHPUnit\Framework\TestCase;

class mytest extends TestCase
{
	/**
	 * @dataProvider provider
	 */
	public function testMethod($data)
	{
		$this->assertTrue($data);
	}

	public function provider()
	{
		return [
			'my name(d data' => [true],
			'my $dat)a'       => [true]
		];
	}
}

requires a escaped filter

➜  phpstan-src git:(1.11.x) ✗ vendor/bin/phpunit mytest.php --filter 'my $dat\)a'
PHPUnit 9.5.23 #StandWithUkraine

Warning:       No code coverage driver available

No tests executed!
➜  phpstan-src git:(1.11.x) ✗ vendor/bin/phpunit mytest.php --filter 'my \$dat\)a'
PHPUnit 9.5.23 #StandWithUkraine

Warning:       No code coverage driver available

.                                                                   1 / 1 (100%)

Time: 00:00.011, Memory: 32.00 MB

OK (1 test, 1 assertion)

the escaping required looks like preg_quote and it works for the case above


the following example doesn't work with preg_quote:

<?php
namespace TestNamespace;

use PHPUnit\Framework\TestCase;

class mytest extends TestCase
{
	/**
	 * @dataProvider provider
	 */
	public function testMethod($data)
	{
		$this->assertTrue($data);
	}

	public function provider()
	{
		return [
			[true],
			[true]
		];
	}
}
➜  phpstan-src git:(quote) ✗ vendor/bin/phpunit mytest.php --filter 'mytest\#1'
PHPUnit 9.5.23 #StandWithUkraine

Warning:       No code coverage driver available

No tests executed!

Expected behavior

I wonder how to properly escape the value for the --filter option.
this would be especially important as when using the wrong filter no tests would be executed.

can we make phpunit return with a non-zero exit code, when a filter is used which does not execute any tests?

edit: I just found #4314 - which described how it should be done.

@sebastianbergmann
Copy link
Owner

I just found #4314 - which described how it should be done.

I was about to comment this:

$ ./tools/phpunit --filter foo --fail-on-empty-test-suite
PHPUnit 11.0.3 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.3.3
Configuration: /usr/local/src/raytracer/phpunit.xml

No tests executed!
$ echo $?                                                
1

I just opened #5707 as --fail-on-empty-test-suite is missing from --help output.

@ondrejmirtes
Copy link

We're just interested in how the value passed to --filter should be sanitized when it comes from an unknown/dynamic source. Is doubling each \ the only thing that needs to be done, or are there other rules to follow?

@sebastianbergmann
Copy link
Owner

I honestly do not know and need to research this (ie. look at the code). I do not have time to do that right now, sorry. I might get to it later today, but it's the weekend, you know. TL;DR: I will do my best to get you an answer ASAP.

@ondrejmirtes
Copy link

There's no rush, we're just trying to improve the CI pipeline :) Thank you very much.

@sebastianbergmann
Copy link
Owner

Doubling \ should be enough. Here you can see what PHPUnit does with the value passed to --filter.

@staabm
Copy link
Contributor Author

staabm commented Feb 18, 2024

hey sebastian,

thx for doing the research.

I think we need to do more escaping, see the very first example in the PR description above.
when using data-providers with a string-key, its possible to use characters like e.g. $, (, ) which need escaping in the --filter option.

Doubling \ is only enough if your filter just contains a class-name and or test-method, but not identifier for the data-provider.

@sebastianbergmann
Copy link
Owner

Do you need something else from me here or can this be closed?

@ondrejmirtes
Copy link

I think what @staabm tried to say is that when you use a custom name for a specific data provider iteration like:

yield 'unknown filter' => [
	'mixed',
	'filter_var($mixed, $mixed)',
];

You could use a custom name that looks like a regular expression. And the question is - if we wanted to execute only this test, how should we write the argument for --filter so that it's not interpreted as a regular expression, but as a request to execute the test for this one specific data provider iteration.

@staabm's theory is that simply doubling \\ is not enough to properly escape the --filter value.

We could of course avoid this problem by using simple names for data providers (and I think we do), but if we were for example to publish a general GitHub Action that would promise you to partition your test suite and run it in parallel over multiple jobs, we could not rely on simple names anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug Something is broken
Projects
None yet
Development

No branches or pull requests

3 participants