If behavior wasn't changed unless ENV var or CLI option was specified, then I think it can go into 7.1 (run-test.php isn't production code part of a build, etc...).
Behavior remaining the same of course wouldn't break anybody's existing CI. For those who benefit from this in 7.3, they should benefit for 7.1 and 7.2 which we have to provide the same level of support for a couple of years still. Also, ENV var could be TRUE to change behavior, or could be a string, in which case the exit code is non-zero only if a failing test's name contains that string. That would allow some CI scripts to only focus on certain tests, extensions, etc.... That would be easy to add. That wouldn't help me but I just thought of that and maybe it helps somebody else. Regards -M On Thu, Mar 1, 2018 at 2:15 AM, Anatol Belski <weltl...@outlook.de> wrote: > Hi Stas, > > > -----Original Message----- > > From: Stanislav Malyshev [mailto:smalys...@gmail.com] > > Sent: Wednesday, February 28, 2018 9:26 PM > > To: PHP Internals <internals@lists.php.net> > > Subject: [PHP-DEV] run-tests.php exit code > > > > Hi! > > > > When running tests with run-tests.php, if the tests fail, the script > will exit with > > non-zero exit code, but only if REPORT_EXIT_STATUS is set. This was the > case > > since 2002 when this capability has been introduced. > > > > I think it would be nice if we reversed the default and made the script > use exit > > code by default, unless NO_EXIT_STATUS is set. All usages of this script > that I > > know (including our own CI suite) use this setting, and I don't think > there's a > > good case for not using it. I think it makes sense to default to the > common use > > case. So: > > > > 1. Does it make sense to change the default? > I've stumbled upon this a couple of times, too, especially when writing > new scripts for non core test automation. IMO, given today there's much > more automation than ever, defaulting to the exit code translation makes > sense. Test fails should not be subtle by default at any levels. > > > 2. Do we want RFC for it? > IMHO not really deserving an RFC. > > > 3. Can we put it in 7.1 or do we want to wait for 7.3? > > > If someone doesn't use it on CI already, that's probably just a blessed > ignorance as the tests runs would always pass. Depends if we want the > change to be going more soft for some non core CI tests and avoid confusion > on manual test runs as in some cases an additional output could be > produced. Given it's in many cases harder to investigate failures on CI, > perhaps 7.3 would be safe to put it with the corresponding documentation, > etc. > > Regarding the option itself, we'd probably not need to introduce a new > one. Instead, a piece like below could be put at the top > > If (getenv("REPORT_EXIT_STATUS") === false) putenv("REPORT_EXIT_STATUS=1") > ; > > Regards > > Anatol >
