Hello internals folks!
As you may already know, run-tests.php is an old legacy app that is in
serious need of a refactor. Some things that are badly needed amongst other
things are:
- Better maintainability
- Unit tests
- Concurrency
- Prettier output
I have seen numerous attempts of rewriting run-tests from scratch after
some rallying at a UG or PHP conference only to see the initial excitement
die off. The general advice for refactoring legacy apps is to not start
with a clean slate, rather refactor a little bit at a time. We should treat
run-tests as we would any other legacy app.
We need a structured process to allow the run-tests app to get refactored a
little bit at a time. I propose the following (open to discussion of
course).
-
Create a new run-tests karma with access to php-src.git/run-tests.php
(and hopefully the soon-to-be created php-src.git/run-tests folder). -
Elect 2-3 people and grant the new run-tests karma to them. They will
champion the PR's that incrementally improve run-tests. -
The new run-tests champions (we'll call them "champions" because, why
not?) will run the entire test suite ensuring that the refactor didn't
inadvertently break the functionality and if all good, merge. (We might be
able to get a solid Docker container or an automated CI process for an
end-to-end test - thoughts?) -
The run-tests champions could be elected for a year-long term with the
possibility of being reelected for another year commitment. This would help
prevent run-test champion drop-off (because sometimes, life happens).
Once we get a process in place, I've drafted up a possible proposal of next
steps to get run-tests refactoring kicked off:
https://github.com/SammyK/run-tests#proposed-refactoring-path
It's time to circle the wagons on run-tests! :)
Thanks,
Sammy Kaye Powers
sammyk.me
230 S Clark St #194
Chicago, IL 60604
--
Andreas Heigl
Andreas@heigl.org
Am 17.05.2017 um 16:28 schrieb Sammy Kaye Powers me@sammyk.me:
Hello internals folks!
As you may already know, run-tests.php is an old legacy app that is in
serious need of a refactor. Some things that are badly needed amongst other
things are:
- Better maintainability
- Unit tests
- Concurrency
- Prettier output
I have seen numerous attempts of rewriting run-tests from scratch after
some rallying at a UG or PHP conference only to see the initial excitement
die off. The general advice for refactoring legacy apps is to not start
with a clean slate, rather refactor a little bit at a time. We should treat
run-tests as we would any other legacy app.We need a structured process to allow the run-tests app to get refactored a
little bit at a time. I propose the following (open to discussion of
course).
- Create a new run-tests karma with access to php-src.git/run-tests.php
(and hopefully the soon-to-be created php-src.git/run-tests folder).
Just a stupid question for a non-regular: Why not a separate repo? In the end run-tests.php is a PHP-application and not a C one and with tests and all such things to do it might be wasier to develop independent from the PHP-sources.
It needs to be available to the PHP-sources in the end, but that might be possible by using git-submodules (or downloading a phar-file?)
Just my 0.02€
Cheers
Andreas
Elect 2-3 people and grant the new run-tests karma to them. They will
champion the PR's that incrementally improve run-tests.The new run-tests champions (we'll call them "champions" because, why
not?) will run the entire test suite ensuring that the refactor didn't
inadvertently break the functionality and if all good, merge. (We might be
able to get a solid Docker container or an automated CI process for an
end-to-end test - thoughts?)The run-tests champions could be elected for a year-long term with the
possibility of being reelected for another year commitment. This would help
prevent run-test champion drop-off (because sometimes, life happens).Once we get a process in place, I've drafted up a possible proposal of next
steps to get run-tests refactoring kicked off:https://github.com/SammyK/run-tests#proposed-refactoring-path
It's time to circle the wagons on run-tests! :)
Thanks,
Sammy Kaye Powers
sammyk.me230 S Clark St #194
Chicago, IL 60604
Am 17.05.2017 um 17:00 schrieb Andreas Heigl:
seriously a signature on top breaks reply and quotes for any sane MUA
which strips signatures and so quotes only stuff on top of the "-- "
line....
Just a stupid question for a non-regular: Why not a
separate repo? In the end run-tests.php is a PHP-application
and not a C one and with tests and all such things to do
it might be wasier to develop independent from the PHP-sources.
that's invisible for endusers
It needs to be available to the PHP-sources in the end, but that
might > be possible by using git-submodules (or downloading a phar-file?)
a sane RPM build is using "run-tests.php" after compilation abd before
generate the final packages
- configure
- make prof-gen
- run application code
- make prof-clean
- make prof-use
- run-tests.sh
- build rpm packages
I like the idea and the proposed approach. I would also offer my help
with anything here. What I would definitely avoid is moving this out of
the PHP repository itself. Ideally we get to a point where people enter
a single command and it does take care of everything (given that the
required dependencies are present on the system, e.g. a working git,
make, etc.).
--
Richard "Fleshgrinder" Fussenegger
Is Sebastian copied in here? Why can't we just use the super-battle-tested
PHPUnit? It supports phpt and a ton of plugins, plus everyone uses it and
is familiar with it.
Am I missing something?
I like the idea and the proposed approach. I would also offer my help
with anything here. What I would definitely avoid is moving this out of
the PHP repository itself. Ideally we get to a point where people enter
a single command and it does take care of everything (given that the
required dependencies are present on the system, e.g. a working git,
make, etc.).--
Richard "Fleshgrinder" Fussenegger
Is Sebastian copied in here? Why can't we just use the super-battle-
tested
PHPUnit? It supports phpt and a ton of plugins, plus everyone uses it
and
is familiar with it.
PHPUnit is huge. run-tests is a small script in a single file which I
can quickly edit. For PHPUnit I have multiple files and need tooling to
phar them up.
PHPUnit (according to it's website) also has more dependencies (DOM and
JSON) which are not included in --disable-all. used
I'm not sure if PHPUnit meanwhile supports redirect tests as in PDO.
Is there a performance comparison? - make test runs for a loooong time.
A difference might have an impact. (both ways round ;) )
To the original question: Granting run-tests.php karma is trivial. We
can easily give access to people working on it. While contributors
should be aware that this is a key component f the PHP development :-)
johannes
On Thu, May 18, 2017 at 2:35 AM, Johannes Schlüter johannes@schlueters.de
wrote:
Is Sebastian copied in here? Why can't we just use the super-battle-
tested
PHPUnit? It supports phpt and a ton of plugins, plus everyone uses it
and
is familiar with it.PHPUnit is huge. run-tests is a small script in a single file which I
can quickly edit. For PHPUnit I have multiple files and need tooling to
phar them up.
You don't need to phar them up - just run a typical composer installation,
like everyone else, and like every PHP package running CI on
travis-ci/circle-ci/continuous-php ever built in the last 5 years.
At this point, composer and phpunit are such big players that breaking them
or breaking PHP makes no difference at all for the user-base.
PHPUnit (according to it's website) also has more dependencies (DOM and
JSON) which are not included in --disable-all. used
Ah yes, that is indeed a problem. I think these deps are used in some
assertions only, so it may be possible to drop them if the assertions are
segregated somewhere else.
I'm not sure if PHPUnit meanwhile supports redirect tests as in PDO.
Unsure - got a link to those?
Is there a performance comparison? - make test runs for a loooong time.
A difference might have an impact. (both ways round ;) )
PHPUnit has plugins to parallelize and aggregate results too - the point of
using a battle-tested and commonly used solution is precisely the benefits
you get from community support.
Marco Pivetta
On Thu, May 18, 2017 at 2:35 AM, Johannes Schlüter <johannes@schluete
rs.de> wrote:Is Sebastian copied in here? Why can't we just use the super-
battle-
tested
PHPUnit? It supports phpt and a ton of plugins, plus everyone
uses it
and
is familiar with it.PHPUnit is huge. run-tests is a small script in a single file which
I
can quickly edit. For PHPUnit I have multiple files and need
tooling to
phar them up.
You don't need to phar them up - just run a typical composer
installation, like everyone else, and like every PHP package running
CI on travis-ci/circle-ci/continuous-php ever built in the last 5
years.
At this point, composer and phpunit are such big players that
breaking them or breaking PHP makes no difference at all for the
user-base.
I love composer. But we're not talking application development here,
but runtime development, where I'm sometimes testing PHP on some
embedded or otherwise strange platform. Keeping things simple has a
value there.
PHPUnit (according to it's website) also has more dependencies (DOM
and
JSON) which are not included in --disable-all. used
Ah yes, that is indeed a problem. I think these deps are used in some
assertions only, so it may be possible to drop them if the assertions
are segregated somewhere else.I'm not sure if PHPUnit meanwhile supports redirect tests as in
PDO.
Unsure - got a link to those?
ext/pdo_*/tests/common.phpt
Is there a performance comparison? - make test runs for a loooong
time.
A difference might have an impact. (both ways round ;) )
PHPUnit has plugins to parallelize and aggregate results too - the
point of using a battle-tested and commonly used solution is
precisely the benefits you get from community support.
phpUnit might be battle tested with parallelization, the PHP test suite
isn't. There are tons of tests using the same resource (i.e. different
mysqli and pdo_mysql tests might use the same tables some other tests
use same port for some socket stuff etc.) Would be great if we get
parallelization, though.
Oh, and while thinking about performance: run-tests.php integrates with
tools like valgrind ... I guess phpUnit doesn't do that either, yet.
johannes
Hi all,
Johannes Schlüter wrote:
phpUnit might be battle tested with parallelization, the PHP test suite
isn't. There are tons of tests using the same resource (i.e. different
mysqli and pdo_mysql tests might use the same tables some other tests
use same port for some socket stuff etc.) Would be great if we get
parallelization, though.
It occurred to me a little while ago that if we ran the test suite in
parallel, but only run one test from a given directory (or extension) at
a time, it should probably avoid any such problems, and it would not be
particularly difficult to implement.
Is that worth a shot, do you think?
--
Andrea Faulds
https://ajf.me/
It occurred to me a little while ago that if we ran the test suite in
parallel, but only run one test from a given directory (or extension) at a
time, it should probably avoid any such problems, and it would not be
particularly difficult to implement.Is that worth a shot, do you think?
Yes, definitely. Some of the tests also have side-effects and don't work if
not run in the original order. They shouldn't have such side-effects, of
course, but auditing nearly 15k tests for this is not a fun task. Within
each directory there should only be one execution thread and the tests
should be run in their original order. That still leaves plenty of room for
concurrency, of course. We have 152 separate directories containing .phpt
files. And if we are smart about it, we sort the directories based on the
number of tests in each and start off with the larger ones to make sure we
don't end with a single process still working on a huge directory. The 152
directories sorted by the number of tests are:
[./Zend/tests] => 1613
[./ext/standard/tests/strings] => 924
[./ext/standard/tests/array] => 923
[./ext/standard/tests/file] => 800
[./ext/spl/tests] => 697
[./ext/date/tests] => 632
[./ext/intl/tests] => 474
[./ext/mysqli/tests] => 396
[./ext/oci8/tests] => 360
[./ext/reflection/tests] => 334
[./ext/gd/tests] => 329
[./ext/mbstring/tests] => 317
[./ext/phar/tests] => 300
[./tests/classes] => 277
[./ext/standard/tests/general_functions] => 272
[./ext/session/tests] => 228
[./ext/standard/tests/math] => 219
[./ext/dom/tests] => 219
[./tests/lang] => 216
[./ext/zlib/tests] => 170
[./ext/pcre/tests] => 118
[./ext/pdo_mysql/tests] => 116
[./ext/simplexml/tests] => 112
[./ext/openssl/tests] => 112
[./ext/ldap/tests] => 107
[./Zend/tests/traits] => 107
[./ext/curl/tests] => 107
[./ext/phar/tests/tar] => 102
[./ext/posix/tests] => 100
[./ext/standard/tests/serialize] => 99
[./ext/standard/tests/streams] => 96
[./ext/filter/tests] => 94
[./ext/phar/tests/zip] => 91
[./Zend/tests/generators] => 86
[./ext/soap/tests/schema] => 85
[./ext/soap/tests/bugs] => 85
[./ext/xml/tests] => 85
[./ext/sockets/tests] => 85
[./ext/sqlite3/tests] => 82
[./tests/basic] => 80
[./ext/iconv/tests] => 77
[./ext/imap/tests] => 77
[./ext/soap/tests/soap12] => 73
[./sapi/cli/tests] => 73
[./ext/soap/tests/interop/Round2/Base] => 72
[./ext/ctype/tests] => 70
[./ext/pgsql/tests] => 70
[./ext/pdo/tests] => 69
[./ext/standard/tests/class_object] => 68
[./tests/output] => 66
[./ext/soap/tests] => 65
[./ext/mcrypt/tests] => 64
[./ext/dba/tests] => 64
[./ext/gmp/tests] => 63
[./tests/lang/operators] => 62
[./ext/json/tests] => 62
[./ext/standard/tests/dir] => 61
[./ext/xsl/tests] => 61
[./ext/zip/tests] => 60
[./ext/phar/tests/cache_list] => 60
[./ext/soap/tests/interop/Round4/GroupH] => 58
[./ext/standard/tests/network] => 58
[./Zend/tests/type_declarations] => 56
[./ext/opcache/tests] => 56
[./Zend/tests/return_types] => 54
[./ext/hash/tests] => 49
[./ext/exif/tests] => 49
[./tests/security] => 48
[./ext/standard/tests/url] => 45
[./Zend/tests/try] => 43
[./ext/tidy/tests] => 39
[./ext/xmlwriter/tests] => 38
[./ext/fileinfo/tests] => 36
[./sapi/phpdbg/tests] => 36
[./ext/soap/tests/interop/Round4/GroupI] => 35
[./ext/wddx/tests] => 34
[./ext/pdo_pgsql/tests] => 34
[./ext/snmp/tests] => 33
[./ext/bcmath/tests] => 33
[./ext/xmlrpc/tests] => 32
[./ext/ftp/tests] => 32
[./ext/tokenizer/tests] => 32
[./ext/calendar/tests] => 32
[./ext/standard/tests/image] => 31
[./ext/pdo_sqlite/tests] => 28
[./ext/standard/tests/mail] => 27
[./ext/pdo_oci/tests] => 27
[./ext/enchant/tests] => 25
[./ext/gettext/tests] => 23
[./Zend/tests/grammar] => 23
[./ext/xmlreader/tests] => 22
[./Zend/tests/assert] => 22
[./sapi/fpm/tests] => 22
[./ext/interbase/tests] => 22
[./ext/soap/tests/interop/Round3/GroupD] => 21
[./ext/standard/tests/assert] => 20
[./ext/standard/tests/misc] => 20
[./ext/standard/tests/filters] => 18
[./ext/libxml/tests] => 18
[./ext/pcntl/tests] => 18
[./ext/readline/tests] => 16
[./Zend/tests/variadic] => 16
[./Zend/tests/use_function] => 16
[./ext/soap/tests/interop/Round2/GroupB] => 15
[./Zend/tests/varSyntax] => 15
[./ext/pdo_dblib/tests] => 14
[./ext/standard/tests/http] => 14
[./Zend/tests/traits/bugs] => 14
[./sapi/cgi/tests] => 14
[./tests/func] => 14
[./tests/run-test] => 14
[./ext/pdo_firebird/tests] => 14
[./ext/odbc/tests] => 14
[./ext/com_dotnet/tests] => 14
[./Zend/tests/arg_unpack] => 13
[./Zend/tests/anon] => 12
[./ext/bz2/tests] => 12
[./tests/strings] => 11
[./Zend/tests/use_const] => 11
[./ext/standard/tests/password] => 10
[./ext/sysvshm/tests] => 9
[./tests/lang/string] => 9
[./Zend/tests/generators/errors] => 9
[./Zend/tests/generators/finally] => 8
[./sapi/tests] => 8
[./ext/standard/tests/time] => 7
[./ext/standard/tests] => 7
[./Zend/tests/multibyte] => 7
[./ext/sysvmsg/tests] => 6
[./ext/soap/tests/interop/Round3/GroupE] => 6
[./ext/soap/tests/interop/Round3/GroupF] => 6
[./ext/pspell/tests] => 5
[./ext/intl/uchar/tests] => 4
[./ext/standard/tests/versioning] => 4
[./Zend/tests/constants] => 4
[./ext/standard/tests/file/windows_acls] => 4
[./ext/standard/tests/random] => 4
[./ext/standard/tests/file/windows_links] => 4
[./ext/standard/tests/directory] => 4
[./ext/standard/tests/crypt] => 3
[./ext/pdo_odbc/tests] => 3
[./Zend/tests/bug67436] => 2
[./ext/recode/tests] => 2
[./Zend/tests/typehints] => 2
[./tests/lang/constants] => 2
[./ext/shmop/tests] => 2
[./ext/sysvsem/tests] => 2
[./ext/spl/examples/tests] => 1
[./ext/phar/tests/bug64931] => 1
[./ext/gmp] => 1
[./ext/skeleton/tests] => 1
[./Zend/tests/ast] => 1
It occurred to me a little while ago that if we ran the test suite in
parallel, but only run one test from a given directory (or extension) at
a
time, it should probably avoid any such problems, and it would not be
particularly difficult to implement.Is that worth a shot, do you think?
Yes, definitely. Some of the tests also have side-effects and don't work if
not run in the original order. They shouldn't have such side-effects, of
course, but auditing nearly 15k tests for this is not a fun task. Within
each directory there should only be one execution thread and the tests
should be run in their original order. That still leaves plenty of room for
concurrency, of course. We have 152 separate directories containing .phpt
files. And if we are smart about it, we sort the directories based on the
number of tests in each and start off with the larger ones to make sure we
don't end with a single process still working on a huge directory. The 152
directories sorted by the number of tests are:
During normal development I usually only run either Zend/ tests, or the
tests in the extension I'm modifying (the rest is what CI is for).
Parallelizing by directory will speed up our CI builds (which is important,
we're often suffering from timeouts), but probably not help much during
development.
Nikita
[./Zend/tests] => 1613 [./ext/standard/tests/strings] => 924 [./ext/standard/tests/array] => 923 [./ext/standard/tests/file] => 800 [./ext/spl/tests] => 697 [./ext/date/tests] => 632 [./ext/intl/tests] => 474 [./ext/mysqli/tests] => 396 [./ext/oci8/tests] => 360 [./ext/reflection/tests] => 334 [./ext/gd/tests] => 329 [./ext/mbstring/tests] => 317 [./ext/phar/tests] => 300 [./tests/classes] => 277 [./ext/standard/tests/general_functions] => 272 [./ext/session/tests] => 228 [./ext/standard/tests/math] => 219 [./ext/dom/tests] => 219 [./tests/lang] => 216 [./ext/zlib/tests] => 170 [./ext/pcre/tests] => 118 [./ext/pdo_mysql/tests] => 116 [./ext/simplexml/tests] => 112 [./ext/openssl/tests] => 112 [./ext/ldap/tests] => 107 [./Zend/tests/traits] => 107 [./ext/curl/tests] => 107 [./ext/phar/tests/tar] => 102 [./ext/posix/tests] => 100 [./ext/standard/tests/serialize] => 99 [./ext/standard/tests/streams] => 96 [./ext/filter/tests] => 94 [./ext/phar/tests/zip] => 91 [./Zend/tests/generators] => 86 [./ext/soap/tests/schema] => 85 [./ext/soap/tests/bugs] => 85 [./ext/xml/tests] => 85 [./ext/sockets/tests] => 85 [./ext/sqlite3/tests] => 82 [./tests/basic] => 80 [./ext/iconv/tests] => 77 [./ext/imap/tests] => 77 [./ext/soap/tests/soap12] => 73 [./sapi/cli/tests] => 73 [./ext/soap/tests/interop/Round2/Base] => 72 [./ext/ctype/tests] => 70 [./ext/pgsql/tests] => 70 [./ext/pdo/tests] => 69 [./ext/standard/tests/class_object] => 68 [./tests/output] => 66 [./ext/soap/tests] => 65 [./ext/mcrypt/tests] => 64 [./ext/dba/tests] => 64 [./ext/gmp/tests] => 63 [./tests/lang/operators] => 62 [./ext/json/tests] => 62 [./ext/standard/tests/dir] => 61 [./ext/xsl/tests] => 61 [./ext/zip/tests] => 60 [./ext/phar/tests/cache_list] => 60 [./ext/soap/tests/interop/Round4/GroupH] => 58 [./ext/standard/tests/network] => 58 [./Zend/tests/type_declarations] => 56 [./ext/opcache/tests] => 56 [./Zend/tests/return_types] => 54 [./ext/hash/tests] => 49 [./ext/exif/tests] => 49 [./tests/security] => 48 [./ext/standard/tests/url] => 45 [./Zend/tests/try] => 43 [./ext/tidy/tests] => 39 [./ext/xmlwriter/tests] => 38 [./ext/fileinfo/tests] => 36 [./sapi/phpdbg/tests] => 36 [./ext/soap/tests/interop/Round4/GroupI] => 35 [./ext/wddx/tests] => 34 [./ext/pdo_pgsql/tests] => 34 [./ext/snmp/tests] => 33 [./ext/bcmath/tests] => 33 [./ext/xmlrpc/tests] => 32 [./ext/ftp/tests] => 32 [./ext/tokenizer/tests] => 32 [./ext/calendar/tests] => 32 [./ext/standard/tests/image] => 31 [./ext/pdo_sqlite/tests] => 28 [./ext/standard/tests/mail] => 27 [./ext/pdo_oci/tests] => 27 [./ext/enchant/tests] => 25 [./ext/gettext/tests] => 23 [./Zend/tests/grammar] => 23 [./ext/xmlreader/tests] => 22 [./Zend/tests/assert] => 22 [./sapi/fpm/tests] => 22 [./ext/interbase/tests] => 22 [./ext/soap/tests/interop/Round3/GroupD] => 21 [./ext/standard/tests/assert] => 20 [./ext/standard/tests/misc] => 20 [./ext/standard/tests/filters] => 18 [./ext/libxml/tests] => 18 [./ext/pcntl/tests] => 18 [./ext/readline/tests] => 16 [./Zend/tests/variadic] => 16 [./Zend/tests/use_function] => 16 [./ext/soap/tests/interop/Round2/GroupB] => 15 [./Zend/tests/varSyntax] => 15 [./ext/pdo_dblib/tests] => 14 [./ext/standard/tests/http] => 14 [./Zend/tests/traits/bugs] => 14 [./sapi/cgi/tests] => 14 [./tests/func] => 14 [./tests/run-test] => 14 [./ext/pdo_firebird/tests] => 14 [./ext/odbc/tests] => 14 [./ext/com_dotnet/tests] => 14 [./Zend/tests/arg_unpack] => 13 [./Zend/tests/anon] => 12 [./ext/bz2/tests] => 12 [./tests/strings] => 11 [./Zend/tests/use_const] => 11 [./ext/standard/tests/password] => 10 [./ext/sysvshm/tests] => 9 [./tests/lang/string] => 9 [./Zend/tests/generators/errors] => 9 [./Zend/tests/generators/finally] => 8 [./sapi/tests] => 8 [./ext/standard/tests/time] => 7 [./ext/standard/tests] => 7 [./Zend/tests/multibyte] => 7 [./ext/sysvmsg/tests] => 6 [./ext/soap/tests/interop/Round3/GroupE] => 6 [./ext/soap/tests/interop/Round3/GroupF] => 6 [./ext/pspell/tests] => 5 [./ext/intl/uchar/tests] => 4 [./ext/standard/tests/versioning] => 4 [./Zend/tests/constants] => 4 [./ext/standard/tests/file/windows_acls] => 4 [./ext/standard/tests/random] => 4 [./ext/standard/tests/file/windows_links] => 4 [./ext/standard/tests/directory] => 4 [./ext/standard/tests/crypt] => 3 [./ext/pdo_odbc/tests] => 3 [./Zend/tests/bug67436] => 2 [./ext/recode/tests] => 2 [./Zend/tests/typehints] => 2 [./tests/lang/constants] => 2 [./ext/shmop/tests] => 2 [./ext/sysvsem/tests] => 2 [./ext/spl/examples/tests] => 1 [./ext/phar/tests/bug64931] => 1 [./ext/gmp] => 1 [./ext/skeleton/tests] => 1 [./Zend/tests/ast] => 1
During normal development I usually only run either Zend/ tests, or the
tests in the extension I'm modifying (the rest is what CI is for).
Parallelizing by directory will speed up our CI builds (which is important,
we're often suffering from timeouts), but probably not help much during
development.
The best way to address that is probably to split some of those 1600
general Zend/tests into sub-directories to get better concurrency when
running all the Zend tests. I think we are in for a world of hurt if we try
to run adjacent tests in parallel. Anything that hits any sort of
datastore, even if it is just the filesystem, will break badly.
-Rasmus
-----Original Message-----
From: Rasmus Lerdorf [mailto:rasmus@lerdorf.com]
Sent: Thursday, May 25, 2017 3:08 PM
To: Nikita Popov nikita.ppv@gmail.com
Cc: Andrea Faulds ajf@ajf.me; PHP internals internals@lists.php.net
Subject: Re: [PHP-DEV] Implement formal process for run-tests.php refactorDuring normal development I usually only run either Zend/ tests, or
the tests in the extension I'm modifying (the rest is what CI is for).
Parallelizing by directory will speed up our CI builds (which is
important, we're often suffering from timeouts), but probably not help
much during development.The best way to address that is probably to split some of those 1600 general
Zend/tests into sub-directories to get better concurrency when running all the
Zend tests. I think we are in for a world of hurt if we try to run adjacent tests in
parallel. Anything that hits any sort of datastore, even if it is just the filesystem,
will break badly.
On the other hand, there is a number of isolated tests that won't hurt parallelism. I've suggested a section like --PARALLEL-- already, but maybe it'd be also more simple to have an extra subdir in every tests dir, like Zend/tests/parallel, where only the capable tests would lay. In that case, the tests itself wouldn't have be touched, just verified and moved into parallel subdir if suitable. An approach with the section would require to edit the test file, but the advantage would be that the file doesn't have to be moved. Either way, there's a work to verify and adapt tests to the choosen approach.
Regards
Anatol
Hi!
Is that worth a shot, do you think?
Definitely. There's no reason also why it shouldn't work so failing
tests in such scenario probably should be treated as bugs.
A lot of tests also inherently parallel-safe - probably majority of
them, though wouldn't claim I am 100% sure - so maybe marking them as
such is also not bad.
Stas Malyshev
smalyshev@gmail.com
Is that worth a shot, do you think?
Definitely. There's no reason also why it shouldn't work so failing
tests in such scenario probably should be treated as bugs.A lot of tests also inherently parallel-safe - probably majority of
them, though wouldn't claim I am 100% sure - so maybe marking them as
such is also not bad.
HHVM imported PHP's core test suite and its test running is
parallelized. We ran into a number of tests which aren't parallel
friendly (shared, hard-coded port number and/or tempfile path) and I
upstreamed fixed for all the tests we had issue with, so the state of
PHP's tests now should be pretty good. (Of the extensions HHVM
actually has in common, anyway).
There are a handful of cases where some tests simply expect to be run
in order, and we flagged these using a marker file:
testname.php.serial (See:
https://gist.github.com/sgolemon/429e4deb413eb539d653460ae062c228 )
-Sara
Am 18.05.2017 um 12:08 schrieb Marco Pivetta:
On Thu, May 18, 2017 at 2:35 AM, Johannes Schlüter johannes@schlueters.de
wrote:Is Sebastian copied in here? Why can't we just use the super-battle-
tested
PHPUnit? It supports phpt and a ton of plugins, plus everyone uses it
and
is familiar with it.PHPUnit is huge. run-tests is a small script in a single file which I
can quickly edit. For PHPUnit I have multiple files and need tooling to
phar them up.You don't need to phar them up - just run a typical composer installation,
like everyone else, and like every PHP package running CI on
travis-ci/circle-ci/continuous-php ever built in the last 5 years.
At this point, composer and phpunit are such big players that breaking them
or breaking PHP makes no difference at all for the user-base
who is the userbase you are talking about?
- no composer here
- no php-unit here
but 250000 LOC and own testsuites and own deployment systems written in
PHP without any third party libraries driving some hundret domains in
place as well as a rpmbuild running "run-tests.php" as part of the build
process
Is Sebastian copied in here? Why can't we just use the super-battle-tested
PHPUnit? It supports phpt and a ton of plugins, plus everyone uses it and
is familiar with it.
IMO, PHPUnit is not the right tool to test PHP itself. Yes, it supports
a subset of PHPT. But its purpose is to test PHP code, not PHP's C code.
Hi Sammy,
-----Original Message-----
From: sammyk@sammykmedia.com [mailto:sammyk@sammykmedia.com] On
Behalf Of Sammy Kaye Powers
Sent: Wednesday, May 17, 2017 4:29 PM
To: PHP Internals internals@lists.php.net
Subject: [PHP-DEV] Implement formal process for run-tests.php refactorHello internals folks!
As you may already know, run-tests.php is an old legacy app that is in serious
need of a refactor. Some things that are badly needed amongst other things are:
- Better maintainability
- Unit tests
- Concurrency
- Prettier output
I have seen numerous attempts of rewriting run-tests from scratch after some
rallying at a UG or PHP conference only to see the initial excitement die off. The
general advice for refactoring legacy apps is to not start with a clean slate,
rather refactor a little bit at a time. We should treat run-tests as we would any
other legacy app.We need a structured process to allow the run-tests app to get refactored a
little bit at a time. I propose the following (open to discussion of course).
Create a new run-tests karma with access to php-src.git/run-tests.php (and
hopefully the soon-to-be created php-src.git/run-tests folder).Elect 2-3 people and grant the new run-tests karma to them. They will
champion the PR's that incrementally improve run-tests.The new run-tests champions (we'll call them "champions" because, why
not?) will run the entire test suite ensuring that the refactor didn't inadvertently
break the functionality and if all good, merge. (We might be able to get a solid
Docker container or an automated CI process for an end-to-end test -
thoughts?)The run-tests champions could be elected for a year-long term with the
possibility of being reelected for another year commitment. This would help
prevent run-test champion drop-off (because sometimes, life happens).Once we get a process in place, I've drafted up a possible proposal of next steps
to get run-tests refactoring kicked off:https://github.com/SammyK/run-tests#proposed-refactoring-path
It's time to circle the wagons on run-tests! :)
That's a fair amount on pre evaluation already ?.
AFM, the parallelism is the most wanted feature that would improve QA a lot, so that is what should stand in the foreground. Having a better structured app might help on it, yes, but having the parallelism to be targeted "some when" is probably not a good sign. Otherwise, an nice app with colors and styles, etc. would be indeed nice but less helpful, IMHO, as the current run-tests.php is great for simplicity and still maintainable well.
I would suggest therefore, to put the harder goals into the foreground, as that would require and warrant a certain rewrite. It's not for nothing, that previous rewrite attempts targeting similar things was abandoned previously, and it's probably not wise to allow for the same mistakes. I'd see as first step an app that
- is a very minimal rewrite of run-tests.php, made OO and whatsoever
- mimics all the existing features and options, but only them
- has as less dependencies as possible, be it PHP extensions or PHP code itself, best requires pure core only
- is compatible with the current test format
- is able to run tests in parallel
- some simple tests are ported for parallel runs already
I'd see such a bare app first developed as a pull request, agreed upon so then run-tests.php is completely replaced by it. Then where the main work can begin - the most of it will be anyway not the app itself, but the porting of the existing tests. The required karma would include the app itself, ext/*/tests, Zend/tests, tests, and other related areas. Perhaps, the tests able to run in parallel can be marked as such with a new section or alike. There are probably thousands of tests that have to be touched to run in parallel otherwise, be it DB tests utilizing same table name or built-in server based tests that want to occupy same port. If there were such an app merged into master, that supports current test format and can run a mix of tests in old/new ways, it'd allow an organic integration and step-by-step improvement of the test suite. Then also, the other features from your evaluation could be developed while the test porting work is being done.
Thanks
Anatol
Hi Sammy,
-----Original Message-----
From: sammyk@sammykmedia.com [mailto:sammyk@sammykmedia.com] On
Behalf Of Sammy Kaye Powers
Sent: Wednesday, May 17, 2017 4:29 PM
To: PHP Internals internals@lists.php.net
Subject: [PHP-DEV] Implement formal process for run-tests.php refactorHello internals folks!
As you may already know, run-tests.php is an old legacy app that is in serious
need of a refactor. Some things that are badly needed amongst other things are:
- Better maintainability
- Unit tests
- Concurrency
- Prettier output
I have seen numerous attempts of rewriting run-tests from scratch after some
rallying at a UG or PHP conference only to see the initial excitement die off. The
general advice for refactoring legacy apps is to not start with a clean slate,
rather refactor a little bit at a time. We should treat run-tests as we would any
other legacy app.We need a structured process to allow the run-tests app to get refactored a
little bit at a time. I propose the following (open to discussion of course).
Create a new run-tests karma with access to php-src.git/run-tests.php (and
hopefully the soon-to-be created php-src.git/run-tests folder).Elect 2-3 people and grant the new run-tests karma to them. They will
champion the PR's that incrementally improve run-tests.The new run-tests champions (we'll call them "champions" because, why
not?) will run the entire test suite ensuring that the refactor didn't inadvertently
break the functionality and if all good, merge. (We might be able to get a solid
Docker container or an automated CI process for an end-to-end test -
thoughts?)The run-tests champions could be elected for a year-long term with the
possibility of being reelected for another year commitment. This would help
prevent run-test champion drop-off (because sometimes, life happens).Once we get a process in place, I've drafted up a possible proposal of next steps
to get run-tests refactoring kicked off:https://github.com/SammyK/run-tests#proposed-refactoring-path
It's time to circle the wagons on run-tests! :)
That's a fair amount on pre evaluation already ?.
AFM, the parallelism is the most wanted feature that would improve QA a lot, so that is what should stand in the foreground. Having a better structured app might help on it, yes, but having the parallelism to be targeted "some when" is probably not a good sign. Otherwise, an nice app with colors and styles, etc. would be indeed nice but less helpful, IMHO, as the current run-tests.php is great for simplicity and still maintainable well.
I would suggest therefore, to put the harder goals into the foreground, as that would require and warrant a certain rewrite. It's not for nothing, that previous rewrite attempts targeting similar things was abandoned previously, and it's probably not wise to allow for the same mistakes. I'd see as first step an app that
- is a very minimal rewrite of run-tests.php, made OO and whatsoever
- mimics all the existing features and options, but only them
- has as less dependencies as possible, be it PHP extensions or PHP code itself, best requires pure core only
- is compatible with the current test format
- is able to run tests in parallel
- some simple tests are ported for parallel runs already
I'd see such a bare app first developed as a pull request, agreed upon so then run-tests.php is completely replaced by it. Then where the main work can begin - the most of it will be anyway not the app itself, but the porting of the existing tests. The required karma would include the app itself, ext/*/tests, Zend/tests, tests, and other related areas. Perhaps, the tests able to run in parallel can be marked as such with a new section or alike. There are probably thousands of tests that have to be touched to run in parallel otherwise, be it DB tests utilizing same table name or built-in server based tests that want to occupy same port. If there were such an app merged into master, that supports current test format and can run a mix of tests in old/new ways, it'd allow an organic integration and step-by-step improvement of the test suite. Then also, the other features from your evaluation could be developed while the test porting work is being done.
Thanks
Anatol
Hey Anatol!
I agree that parallelism should be the first and primary goal of the
refactor. I just mentioned maintainability and unit tests first since
I foresee those things happing as we refactor to parallelism. :)
I'm OK doing a very minimal "proof-of-concept" project that would
showcase a possible concurrency model so that we could discuss the
tradeoffs of the implementation details in more depth. But after we
agree on an implementation, I don't think we need to merge it
directly, instead we should make small refactorings of the live
run-tests.php that would eventually guide it toward the agreed-upon
proof-of-concept implementation.
As far as tests that can't be run in parallel, we could look to HHVM
which checks for an empty file with the same name as the test file
with the ".serial" extension. If it finds that, the test is put into a
special "run in serial" bucket. I know make has a ".NOTPARALLEL"
target to specify that things should be run serially. We could also
have a new section in the .phpt file "--NOTPARALLEL--" or a tag
similar to ===DONE=== so it's not parsed as a section:
"===NOTPARALLEL===". At any rate, we'll need to decide on something
but I think an empty ".serial" file makes a lot of sense. :)
What would you say would be good next steps? Should I draft up a more
formal process in the WIKI so we can discuss specifics further?
https://wiki.php.net/qa/runtests
Hi Sammy,
-----Original Message-----
From: sammyk@sammykmedia.com [mailto:sammyk@sammykmedia.com] OnI agree that parallelism should be the first and primary goal of the refactor. I just
mentioned maintainability and unit tests first since I foresee those things
happing as we refactor to parallelism. :)
ACK.
I'm OK doing a very minimal "proof-of-concept" project that would showcase a
possible concurrency model so that we could discuss the tradeoffs of the
implementation details in more depth. But after we agree on an
implementation, I don't think we need to merge it directly, instead we should
make small refactorings of the live run-tests.php that would eventually guide it
toward the agreed-upon proof-of-concept implementation.
From your first message, it looked to me like you had intention to rewrite the most of the tool. My suggestion was based on that. The PR I mentioned were to see the initial work in that case. As that were the base for the sustainable improvement, which could be continued directly in the core. Say, a tool that can run both the existing tests, even unchanged, and some tests also in parallel by whatsoever new approach. OFC run-tests.php should stay, literally, just it'd have a new engine under the hood. Once that initial work would exist, it could be merged and more detailed incremental process would take place, that would include also the aspects you mentioned in the first mail and on github. The plus I'd see to proceed this way were to stay compatible, while having the base for the further improvements.
As far as tests that can't be run in parallel, we could look to HHVM which checks
for an empty file with the same name as the test file with the ".serial" extension.
If it finds that, the test is put into a special "run in serial" bucket. I know make
has a ".NOTPARALLEL"
target to specify that things should be run serially. We could also have a new
section in the .phpt file "--NOTPARALLEL--" or a tag similar to ===DONE=== so
it's not parsed as a section:
"===NOTPARALLEL===". At any rate, we'll need to decide on something but I
think an empty ".serial" file makes a lot of sense. :)
Some extra file to mark parallelism looks excessive for me, it should be only one test file with different sections. So either way, --SERIAL-- or --PARALLEL-- section would be enough to tell the runner about parallelism, depends on what makes sense to mark by default. As for me, it looks like we should expect tests to be not parallelism capable by default, at least at the start - in that case less things can go wrong. Only those that are explicitly marked as parallel, would be executed that way, otherwise the runner would act same way as before.
It looks like the idea to split some tests into sub folders might simplify things a lot, as noted by Andrea and Rasmus. With the subdir approach - yeah, there can be also a huge simplification, also with regard to tests sensitive to the run order. I recall at least ext/pgsql is based on that approach. But for sure, fe one can run tests from different extension folders in parallel, for example ext/pgsql and ext/mysqli should not collide, many others won't, too, as there are also lots of exts not using any resources at all. It'd be indeed a fast way to achieve some parallelism without big effort. To call some - ext/pcre, ext/date. Issues are to expect
- DB extensions for same database, fe ext/mysqli and ext/pdo_mysqli
- built-in server based tests
- tests working on same test files - creating/deleting/etc.
- any other tests using same resources
- slow tests possibly to occupy execution slots
Even then, there are quite some tests located in the same dir, that still can be run in parallel. For directories with a big amount of tests, so the subdir approach might be less usefyl. Thus, having support for the exact sections still should be implemented, AFM.
What would you say would be good next steps? Should I draft up a more formal
process in the WIKI so we can discuss specifics further?
https://wiki.php.net/qa/runtests
That's a good idea, yep. Especially to articulate the minimum required and the pieces of work to be done. As you told, there are already some people interested in this work, so everyone could fork and update the wiki with the part of the taken work. That'd be helpful to both track the progress and issues and to avoid duplicate effort. We've used similar approach to track the work on the 64-bit branch https://wiki.php.net/rfc/string-size_t/progress .
Regards
Anatol