Re: Re: run-tests.php

..PS - here is a slightly longer explanation of how I got to the idea that to removing the "s" modifier would be a better fix. I did a little more work to understand how the regex matching for %s was working in run-tests.php. In the current version %s maps to .+? This *should* match one or more characters but the non-greedy qualifyier should guarentee a minumum match. Also, the "." would not normally match an end of line unless it was used with a /s modifier. I was confused buy the fact that in run-tests.php .+? appears from my previous example to be acting in a greedy way and matching end of line characters. More staring at the run-tests.php code revealed the explanation in this line of code: if (preg_match("/^$wanted_re$/s", $output)) Here the $ at the end of wanted_re has the effect of making the match greedy - and the /s modifier is making "." match eol characters. The /s modifier has been in the code ever since the --EXPECTF-- section was introduced and the $ was added between v1.73 and 1.74 by Derick, so it's been there since 2002. Johannes Schlüter wrote:

Hi Nuno, On Thu, 2007-09-13 at 20:20 +0100, Nuno Lopes wrote:

now i get the issue. It grabbs more than one line! This means run-tests.php has to be fixed. The whole thing is designed under the assumption that %s catches no new lines and hence only one line.

That would be the best answer but I think it's a difficult fix and whatever it was might break other tests. I think it would mean replacing %s with something other than .+?. I'll have a look tomorrow and see if I can find a regex that would do - one of the things that had me confused for a while was that I thought the ? (non-greedy qualifier) should guarentee the minimum match but it doesn't work quite like that.

OK, so basically it is impossible to fix the current regex to match what we want (I had a quick chat with the PCRE author and he thinks the same). I propose the following patch: http://web.ist.utl.pt/nuno.lopes/php_run_tests_%s.txt

Should be http://web.ist.utl.pt/nuno.lopes/php_run_tests_%25s.txt I guess ;-)

The main changes are: '%s' => '[^\r\n]+' '%a' => '.+' This means that some tests have to be fixed, but the majority passes. Most important is that with this patch I found some tests that had the expected output wrong. So unless someone rejects this patch, I'll commit it (and update the http://qa.php.net/write-test.php page too). Anyway, I don't think that a function name should be replaced by %s. The expected output should be as stricter as possible.

from a short review of the patch I think it's fine and a good idea. So please go on and commit. johannes