* parallelize test suite
@ 2019-05-04 20:57 Rollins, Jameson
2019-05-04 20:57 ` [PATCH 1/3] tests: remove some redundant pre-cleanup of the corpus MAIL_DIR Rollins, Jameson
` (3 more replies)
0 siblings, 4 replies; 22+ messages in thread
From: Rollins, Jameson @ 2019-05-04 20:57 UTC (permalink / raw)
To: Notmuch Mail
This is a simple patch series that will run the entire test suite in
parallel if either the moreutils or GNU parallel utility is
available. On my 8-core machine the full test suite will now run in
under 20 seconds, which is a pretty huge improvement.
jamie.
^ permalink raw reply [flat|nested] 22+ messages in thread* [PATCH 1/3] tests: remove some redundant pre-cleanup of the corpus MAIL_DIR 2019-05-04 20:57 parallelize test suite Rollins, Jameson @ 2019-05-04 20:57 ` Rollins, Jameson 2019-05-04 20:57 ` [PATCH 2/3] tests: remove entangling corpus index optimization Rollins, Jameson 2019-05-05 10:22 ` [PATCH 1/3] tests: remove some redundant pre-cleanup of the corpus MAIL_DIR Tomi Ollila 2019-05-04 21:33 ` parallelize test suite Rollins, Jameson ` (2 subsequent siblings) 3 siblings, 2 replies; 22+ messages in thread From: Rollins, Jameson @ 2019-05-04 20:57 UTC (permalink / raw) To: Notmuch Mail From: Jameson Graef Rollins <jrollins@finestructure.net> add_email_corpus itself does an rm -rf $MAIL_DIR, so these are not necessary. --- test/T100-search-by-folder.sh | 1 - test/T650-regexp-query.sh | 1 - 2 files changed, 2 deletions(-) diff --git a/test/T100-search-by-folder.sh b/test/T100-search-by-folder.sh index a090f3d2..409cfdcc 100755 --- a/test/T100-search-by-folder.sh +++ b/test/T100-search-by-folder.sh @@ -56,7 +56,6 @@ output=$(notmuch search folder:duplicate/bad/olds | notmuch_search_sanitize) test_expect_equal "$output" "thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; Bears (inbox unread)" # folder: and path: searches with full corpus -rm -rf $MAIL_DIR add_email_corpus # add some more dupes diff --git a/test/T650-regexp-query.sh b/test/T650-regexp-query.sh index 92334ba0..43af3b47 100755 --- a/test/T650-regexp-query.sh +++ b/test/T650-regexp-query.sh @@ -47,7 +47,6 @@ output=$(notmuch search 'path:/^bad$/' | notmuch_search_sanitize) test_expect_equal "$output" "thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; To the bone (inbox unread)" # Use "standard" corpus from here on. -rm -rf $MAIL_DIR add_email_corpus notmuch search --output=messages from:cworth > cworth.msg-ids -- 2.20.1 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 2/3] tests: remove entangling corpus index optimization 2019-05-04 20:57 ` [PATCH 1/3] tests: remove some redundant pre-cleanup of the corpus MAIL_DIR Rollins, Jameson @ 2019-05-04 20:57 ` Rollins, Jameson 2019-05-04 20:57 ` [PATCH 3/3] tests: run all tests in parallel, if available Rollins, Jameson 2019-05-05 10:22 ` [PATCH 1/3] tests: remove some redundant pre-cleanup of the corpus MAIL_DIR Tomi Ollila 1 sibling, 1 reply; 22+ messages in thread From: Rollins, Jameson @ 2019-05-04 20:57 UTC (permalink / raw) To: Notmuch Mail From: Jameson Graef Rollins <jrollins@finestructure.net> The add_email_corpus test utility includes logic that tries to re-use an index of the corpus if available. This was seemingly done as an optimization, so that every test that uses the corpus didn't have to create it's own index of the corpus. However, this has the perverse side effect of entangling tests together, and breaks parallelization. Forcing each test to do it's own index does increase the overall time of the test slightly (~6%), but this will be more than made up for in the next patch that introduces paraellization. --- test/.gitignore | 1 - test/Makefile.local | 2 +- test/notmuch-test | 2 +- test/test-lib.sh | 10 ++-------- 4 files changed, 4 insertions(+), 11 deletions(-) diff --git a/test/.gitignore b/test/.gitignore index 69080e5e..f5968404 100644 --- a/test/.gitignore +++ b/test/.gitignore @@ -1,5 +1,4 @@ /arg-test -/corpora.mail /hex-xcode /parse-time /random-corpus diff --git a/test/Makefile.local b/test/Makefile.local index 1cf09778..47244e8f 100644 --- a/test/Makefile.local +++ b/test/Makefile.local @@ -81,4 +81,4 @@ check: test SRCS := $(SRCS) $(test_srcs) CLEAN += $(TEST_BINARIES) $(addsuffix .o,$(TEST_BINARIES)) \ $(dir)/database-test.o \ - $(dir)/corpora.mail $(dir)/test-results $(dir)/tmp.* + $(dir)/test-results $(dir)/tmp.* diff --git a/test/notmuch-test b/test/notmuch-test index ca68dd41..1a1ae811 100755 --- a/test/notmuch-test +++ b/test/notmuch-test @@ -59,6 +59,6 @@ $NOTMUCH_SRCDIR/test/aggregate-results.sh $NOTMUCH_BUILDDIR/test/test-results/* ev=$? # Clean up -rm -rf $NOTMUCH_BUILDDIR/test/test-results $NOTMUCH_BUILDDIR/test/corpora.mail +rm -rf $NOTMUCH_BUILDDIR/test/test-results exit $ev diff --git a/test/test-lib.sh b/test/test-lib.sh index 04d93f7d..43339902 100644 --- a/test/test-lib.sh +++ b/test/test-lib.sh @@ -394,14 +394,8 @@ add_email_corpus () corpus=${1:-default} rm -rf ${MAIL_DIR} - if [ -d $TEST_DIRECTORY/corpora.mail/$corpus ]; then - cp -a $TEST_DIRECTORY/corpora.mail/$corpus ${MAIL_DIR} - else - cp -a $NOTMUCH_SRCDIR/test/corpora/$corpus ${MAIL_DIR} - notmuch new >/dev/null || die "'notmuch new' failed while adding email corpus" - mkdir -p $TEST_DIRECTORY/corpora.mail - cp -a ${MAIL_DIR} $TEST_DIRECTORY/corpora.mail/$corpus - fi + cp -a $NOTMUCH_SRCDIR/test/corpora/$corpus ${MAIL_DIR} + notmuch new >/dev/null || die "'notmuch new' failed while adding email corpus" } test_begin_subtest () -- 2.20.1 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 3/3] tests: run all tests in parallel, if available 2019-05-04 20:57 ` [PATCH 2/3] tests: remove entangling corpus index optimization Rollins, Jameson @ 2019-05-04 20:57 ` Rollins, Jameson 2019-05-06 4:44 ` [PATCH] tests: environment variable to specify that tests should be serialized Rollins, Jameson 0 siblings, 1 reply; 22+ messages in thread From: Rollins, Jameson @ 2019-05-04 20:57 UTC (permalink / raw) To: Notmuch Mail From: Jameson Graef Rollins <jrollins@finestructure.net> If either the moreutils or GNU parallel utility are available, run all tests in parallel. On my eight core machine this makes for a ~x7 speed-up in the full test suite (1m24s -> 12s). The design of the test suite makes this parallelization trivial. --- test/notmuch-test | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/test/notmuch-test b/test/notmuch-test index 1a1ae811..a4b7a1eb 100755 --- a/test/notmuch-test +++ b/test/notmuch-test @@ -40,17 +40,27 @@ fi trap 'e=$?; kill $!; exit $e' HUP INT TERM # Run the tests -for test in $TESTS; do - $TEST_TIMEOUT_CMD $test "$@" & - wait $! - # If the test failed without producing results, then it aborted, - # so we should abort, too. - RES=$? - testname=$(basename $test .sh) - if [[ $RES != 0 && ! -e "$NOTMUCH_BUILDDIR/test/test-results/$testname" ]]; then - exit $RES +if command -v parallel >/dev/null ; then + if parallel -h | grep -q GNU ; then + echo "INFO: running tests with GNU parallel" + printf '%s\n' $TESTS | $TEST_TIMEOUT_CMD parallel + else + echo "INFO: running tests with moreutils parallel" + $TEST_TIMEOUT_CMD parallel -- $TESTS fi -done +else + for test in $TESTS; do + $TEST_TIMEOUT_CMD $test "$@" & + wait $! + # If the test failed without producing results, then it aborted, + # so we should abort, too. + RES=$? + testname=$(basename $test .sh) + if [[ $RES != 0 && ! -e "$NOTMUCH_BUILDDIR/test/test-results/$testname" ]]; then + exit $RES + fi + done +fi trap - HUP INT TERM # Report results -- 2.20.1 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH] tests: environment variable to specify that tests should be serialized 2019-05-04 20:57 ` [PATCH 3/3] tests: run all tests in parallel, if available Rollins, Jameson @ 2019-05-06 4:44 ` Rollins, Jameson 2019-05-06 10:32 ` [PATCH] test: add configurable port to smtp-dummy David Bremner 2019-05-06 19:15 ` [PATCH] tests: environment variable to specify that tests should be serialized Tomi Ollila 0 siblings, 2 replies; 22+ messages in thread From: Rollins, Jameson @ 2019-05-06 4:44 UTC (permalink / raw) To: Notmuch Mail From: Jameson Graef Rollins <jrollins@finestructure.net> If NOTMUCH_TEST_SERIALIZE is non-null all tests will be run in series, rather than in parallel. --- test/README | 8 ++++++-- test/notmuch-test | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/test/README b/test/README index b378c3ff..3f54af58 100644 --- a/test/README +++ b/test/README @@ -43,7 +43,6 @@ these system tools. Most often the tests are written, reviewed and tested on Linux system so such portability issues arise from time to time. - Running Tests ------------- The easiest way to run tests is to say "make test", (or simply run the @@ -105,9 +104,14 @@ to with emacs, e.g. make test TEST_CC=gcc TEST_CFLAGS="-g -O2" +Parallel Execution +------------------ +If either the moreutils or GNU "parallel" utility is available all +tests will be run in parallel. If the NOTMUCH_TEST_SERIALIZE variable +is non-null all tests will be executed sequentially. + Quiet Execution --------------- - Normally, when new script starts and when test PASSes you get a message printed on screen. This printing can be disabled by setting the NOTMUCH_TEST_QUIET variable to a non-null value. Message on test diff --git a/test/notmuch-test b/test/notmuch-test index a4b7a1eb..bd3e080a 100755 --- a/test/notmuch-test +++ b/test/notmuch-test @@ -40,7 +40,7 @@ fi trap 'e=$?; kill $!; exit $e' HUP INT TERM # Run the tests -if command -v parallel >/dev/null ; then +if test -z "$NOTMUCH_TEST_SERIALIZE" && command -v parallel >/dev/null ; then if parallel -h | grep -q GNU ; then echo "INFO: running tests with GNU parallel" printf '%s\n' $TESTS | $TEST_TIMEOUT_CMD parallel -- 2.20.1 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH] test: add configurable port to smtp-dummy 2019-05-06 4:44 ` [PATCH] tests: environment variable to specify that tests should be serialized Rollins, Jameson @ 2019-05-06 10:32 ` David Bremner 2019-05-06 19:39 ` Tomi Ollila 2019-05-06 19:15 ` [PATCH] tests: environment variable to specify that tests should be serialized Tomi Ollila 1 sibling, 1 reply; 22+ messages in thread From: David Bremner @ 2019-05-06 10:32 UTC (permalink / raw) To: Rollins, Jameson, Notmuch Mail This is to allow smtp-dummy to potentially be used from multiple T*.sh without collisions during parallel test running. --- test/T310-emacs.sh | 3 +++ test/smtp-dummy.c | 7 ++++++- test/test-lib.sh | 4 +++- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/test/T310-emacs.sh b/test/T310-emacs.sh index c06a8133..3eab1ba9 100755 --- a/test/T310-emacs.sh +++ b/test/T310-emacs.sh @@ -5,6 +5,9 @@ test_description="emacs interface" EXPECTED=$NOTMUCH_SRCDIR/test/emacs.expected-output +# this port must be unique to each test script using smtp-dummy +export SMTP_DUMMY_PORT=25125 + add_email_corpus # syntax errors in test-lib.el cause mysterious failures diff --git a/test/smtp-dummy.c b/test/smtp-dummy.c index 71992edd..4cd66f5c 100644 --- a/test/smtp-dummy.c +++ b/test/smtp-dummy.c @@ -120,6 +120,7 @@ int main (int argc, char *argv[]) { const char *progname; + const char *port_str; char *output_filename; FILE *peer_file = NULL, *output = NULL; int sock = -1, peer, err; @@ -191,7 +192,11 @@ main (int argc, char *argv[]) memset (&addr, 0, sizeof (addr)); addr.sin_family = AF_INET; - addr.sin_port = htons (25025); + if ((port_str = getenv ("SMTP_DUMMY_PORT"))) + addr.sin_port = htons (atoi (port_str)); + else + addr.sin_port = htons (25025); + addr.sin_addr = *(struct in_addr *) hostinfo->h_addr; err = bind (sock, (struct sockaddr *) &addr, sizeof (addr)); if (err) { diff --git a/test/test-lib.sh b/test/test-lib.sh index 43339902..f3be537b 100644 --- a/test/test-lib.sh +++ b/test/test-lib.sh @@ -93,6 +93,8 @@ TEST_GDB=${TEST_GDB:-gdb} TEST_CC=${TEST_CC:-cc} TEST_CFLAGS=${TEST_CFLAGS:-"-g -O0"} +SMTP_DUMMY_PORT=${SMTP_DUMMY_PORT:-25025} + # Protect ourselves from common misconfiguration to export # CDPATH into the environment unset CDPATH @@ -324,7 +326,7 @@ emacs_deliver_message () "(let ((message-send-mail-function 'message-smtpmail-send-it) (mail-host-address \"example.com\") (smtpmail-smtp-server \"localhost\") - (smtpmail-smtp-service \"25025\")) + (smtpmail-smtp-service \"${SMTP_DUMMY_PORT}\")) (notmuch-mua-mail) (message-goto-to) (insert \"test_suite@notmuchmail.org\nDate: 01 Jan 2000 12:00:00 -0000\") -- 2.20.1 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] test: add configurable port to smtp-dummy 2019-05-06 10:32 ` [PATCH] test: add configurable port to smtp-dummy David Bremner @ 2019-05-06 19:39 ` Tomi Ollila 2019-05-06 20:55 ` Daniel Kahn Gillmor 2019-05-07 10:20 ` [PATCH] test: let the OS choose a port for smtp-dummy David Bremner 0 siblings, 2 replies; 22+ messages in thread From: Tomi Ollila @ 2019-05-06 19:39 UTC (permalink / raw) To: David Bremner, Rollins, Jameson, Notmuch Mail On Mon, May 06 2019, David Bremner wrote: > This is to allow smtp-dummy to potentially be used from multiple T*.sh > without collisions during parallel test running. > --- > test/T310-emacs.sh | 3 +++ > test/smtp-dummy.c | 7 ++++++- > test/test-lib.sh | 4 +++- > 3 files changed, 12 insertions(+), 2 deletions(-) > > diff --git a/test/T310-emacs.sh b/test/T310-emacs.sh > index c06a8133..3eab1ba9 100755 > --- a/test/T310-emacs.sh > +++ b/test/T310-emacs.sh > @@ -5,6 +5,9 @@ test_description="emacs interface" > > EXPECTED=$NOTMUCH_SRCDIR/test/emacs.expected-output > > +# this port must be unique to each test script using smtp-dummy > +export SMTP_DUMMY_PORT=25125 I suggest 25310 ! :D ( SMTP_DUMMY_PORT=${0%%-*} SMTP_DUMMY_PORT=$(( 25000 + ${SMTP_DUMMY_PORT#?} )) is probably too complicated and error prone ;) > + > add_email_corpus > > # syntax errors in test-lib.el cause mysterious failures > diff --git a/test/smtp-dummy.c b/test/smtp-dummy.c > index 71992edd..4cd66f5c 100644 > --- a/test/smtp-dummy.c > +++ b/test/smtp-dummy.c > @@ -120,6 +120,7 @@ int > main (int argc, char *argv[]) > { > const char *progname; > + const char *port_str; > char *output_filename; > FILE *peer_file = NULL, *output = NULL; > int sock = -1, peer, err; > @@ -191,7 +192,11 @@ main (int argc, char *argv[]) > > memset (&addr, 0, sizeof (addr)); > addr.sin_family = AF_INET; > - addr.sin_port = htons (25025); > + if ((port_str = getenv ("SMTP_DUMMY_PORT"))) If we required SMTP_DUMMY_PORT to be defined in environmment (and not fallback to 25025 if not) we'd have less change to forget defining it... but that may be too much to ask... ;/ ... continuing on same subject below ... > + addr.sin_port = htons (atoi (port_str)); > + else > + addr.sin_port = htons (25025); > + > addr.sin_addr = *(struct in_addr *) hostinfo->h_addr; > err = bind (sock, (struct sockaddr *) &addr, sizeof (addr)); > if (err) { > diff --git a/test/test-lib.sh b/test/test-lib.sh > index 43339902..f3be537b 100644 > --- a/test/test-lib.sh > +++ b/test/test-lib.sh > @@ -93,6 +93,8 @@ TEST_GDB=${TEST_GDB:-gdb} > TEST_CC=${TEST_CC:-cc} > TEST_CFLAGS=${TEST_CFLAGS:-"-g -O0"} > > +SMTP_DUMMY_PORT=${SMTP_DUMMY_PORT:-25025} ... in that case this would not be set here ( but since we don't have `set -u` set in our test env (I have some half-decade-old patches for that laying around though) we'd have to check that in emacs_deliver_message) well, perhaps chance having same port is very low, in 10 years there has been only one file running using that... ... but now I remember something (else) I though doing half-decade ago: we run smtp-dummy as: smtp_dummy_pid= eval `$TEST_DIRECTORY/smtp-dummy --background sent_message` if process binds using port `0` system fill find available port for it... ... which smtp-dummy could return in a variable to be evaluated ... ... and that could replace all this what has been done here ... but whatever -- and IMO we could have parallelization w/o this already not *IF* it is not done by default (or even default but then someone(tm) has to fix this some way in near future (I'd vote smtp-dummy choosing port :D, that cannot be too hard. Tomi > + > # Protect ourselves from common misconfiguration to export > # CDPATH into the environment > unset CDPATH > @@ -324,7 +326,7 @@ emacs_deliver_message () > "(let ((message-send-mail-function 'message-smtpmail-send-it) > (mail-host-address \"example.com\") > (smtpmail-smtp-server \"localhost\") > - (smtpmail-smtp-service \"25025\")) > + (smtpmail-smtp-service \"${SMTP_DUMMY_PORT}\")) > (notmuch-mua-mail) > (message-goto-to) > (insert \"test_suite@notmuchmail.org\nDate: 01 Jan 2000 12:00:00 -0000\") > -- ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] test: add configurable port to smtp-dummy 2019-05-06 19:39 ` Tomi Ollila @ 2019-05-06 20:55 ` Daniel Kahn Gillmor 2019-05-07 10:20 ` [PATCH] test: let the OS choose a port for smtp-dummy David Bremner 1 sibling, 0 replies; 22+ messages in thread From: Daniel Kahn Gillmor @ 2019-05-06 20:55 UTC (permalink / raw) To: Tomi Ollila, David Bremner, Rollins, Jameson, Notmuch Mail [-- Attachment #1: Type: text/plain, Size: 793 bytes --] On Mon 2019-05-06 22:39:26 +0300, Tomi Ollila wrote: > we run smtp-dummy as: > > smtp_dummy_pid= > eval `$TEST_DIRECTORY/smtp-dummy --background sent_message` > > if process binds using port `0` system fill find available port for it... > > ... which smtp-dummy could return in a variable to be evaluated ... > > ... and that could replace all this what has been done here ... i agree with Tomi that something like this (smtp-dummy asks for an available port and reports it), is the ideal solution, in that it means that two copies of the notmuch testsuite itself could even run independently and avoid contending for system resources. but i think Bremner's manually-set port is a completely reasonable first step, and we can adopt the proposed optimization later. LGTM. --dkg [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 227 bytes --] ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] test: let the OS choose a port for smtp-dummy 2019-05-06 19:39 ` Tomi Ollila 2019-05-06 20:55 ` Daniel Kahn Gillmor @ 2019-05-07 10:20 ` David Bremner 2019-05-07 12:38 ` Daniel Kahn Gillmor 2019-05-10 10:16 ` David Bremner 1 sibling, 2 replies; 22+ messages in thread From: David Bremner @ 2019-05-07 10:20 UTC (permalink / raw) To: Tomi Ollila, David Bremner, Rollins, Jameson, Notmuch Mail This should avoid potential collisions if we start running multiple smtp-dummy processes in parallel. --- test/smtp-dummy.c | 15 ++++++++++++++- test/test-lib.sh | 2 +- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/test/smtp-dummy.c b/test/smtp-dummy.c index 71992edd..a7c1fe4f 100644 --- a/test/smtp-dummy.c +++ b/test/smtp-dummy.c @@ -129,6 +129,7 @@ main (int argc, char *argv[]) int reuse; int background; int ret = 0; + socklen_t addrlen; progname = argv[0]; @@ -191,7 +192,7 @@ main (int argc, char *argv[]) memset (&addr, 0, sizeof (addr)); addr.sin_family = AF_INET; - addr.sin_port = htons (25025); + addr.sin_port = 0; addr.sin_addr = *(struct in_addr *) hostinfo->h_addr; err = bind (sock, (struct sockaddr *) &addr, sizeof (addr)); if (err) { @@ -202,6 +203,18 @@ main (int argc, char *argv[]) goto DONE; } + addrlen = sizeof (addr); + err = getsockname (sock, (struct sockaddr *) &addr, &addrlen); + if (err) { + fprintf (stderr, "Error: getsockname() failed: %s\n", + strerror (errno)); + close (sock); + ret = 1; + goto DONE; + } + + printf ("smtp_dummy_port='%d'\n", ntohs (addr.sin_port)); + err = listen (sock, 1); if (err) { fprintf (stderr, "Error: listen() failed: %s\n", diff --git a/test/test-lib.sh b/test/test-lib.sh index f5d367aa..507886ba 100644 --- a/test/test-lib.sh +++ b/test/test-lib.sh @@ -326,7 +326,7 @@ emacs_deliver_message () "(let ((message-send-mail-function 'message-smtpmail-send-it) (mail-host-address \"example.com\") (smtpmail-smtp-server \"localhost\") - (smtpmail-smtp-service \"25025\")) + (smtpmail-smtp-service \"${smtp_dummy_port}\")) (notmuch-mua-mail) (message-goto-to) (insert \"test_suite@notmuchmail.org\nDate: 01 Jan 2000 12:00:00 -0000\") -- 2.20.1 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] test: let the OS choose a port for smtp-dummy 2019-05-07 10:20 ` [PATCH] test: let the OS choose a port for smtp-dummy David Bremner @ 2019-05-07 12:38 ` Daniel Kahn Gillmor 2019-05-08 15:59 ` Tomi Ollila 2019-05-10 10:16 ` David Bremner 1 sibling, 1 reply; 22+ messages in thread From: Daniel Kahn Gillmor @ 2019-05-07 12:38 UTC (permalink / raw) To: David Bremner, Tomi Ollila, David Bremner, Rollins, Jameson, Notmuch Mail [-- Attachment #1: Type: text/plain, Size: 355 bytes --] On Tue 2019-05-07 07:20:49 -0300, David Bremner wrote: > This should avoid potential collisions if we start running multiple > smtp-dummy processes in parallel. This is excellent, simple, and clearly the right thing to do. I've reviewed it, and am running it on my own development branch with no problems. Thanks, Bremner! please merge. --dkg [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 227 bytes --] ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] test: let the OS choose a port for smtp-dummy 2019-05-07 12:38 ` Daniel Kahn Gillmor @ 2019-05-08 15:59 ` Tomi Ollila 0 siblings, 0 replies; 22+ messages in thread From: Tomi Ollila @ 2019-05-08 15:59 UTC (permalink / raw) To: Daniel Kahn Gillmor, David Bremner, David Bremner, Rollins, Jameson, Notmuch Mail On Tue, May 07 2019, Daniel Kahn Gillmor wrote: > On Tue 2019-05-07 07:20:49 -0300, David Bremner wrote: >> This should avoid potential collisions if we start running multiple >> smtp-dummy processes in parallel. > > This is excellent, simple, and clearly the right thing to do. I've > reviewed it, and am running it on my own development branch with no > problems. Thanks, Bremner! > > please merge. i second that, please do! Tomi > > --dkg ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] test: let the OS choose a port for smtp-dummy 2019-05-07 10:20 ` [PATCH] test: let the OS choose a port for smtp-dummy David Bremner 2019-05-07 12:38 ` Daniel Kahn Gillmor @ 2019-05-10 10:16 ` David Bremner 1 sibling, 0 replies; 22+ messages in thread From: David Bremner @ 2019-05-10 10:16 UTC (permalink / raw) To: Tomi Ollila, Rollins, Jameson, Notmuch Mail David Bremner <david@tethera.net> writes: > This should avoid potential collisions if we start running multiple > smtp-dummy processes in parallel. merged to master ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] tests: environment variable to specify that tests should be serialized 2019-05-06 4:44 ` [PATCH] tests: environment variable to specify that tests should be serialized Rollins, Jameson 2019-05-06 10:32 ` [PATCH] test: add configurable port to smtp-dummy David Bremner @ 2019-05-06 19:15 ` Tomi Ollila 2019-05-06 20:58 ` Daniel Kahn Gillmor 1 sibling, 1 reply; 22+ messages in thread From: Tomi Ollila @ 2019-05-06 19:15 UTC (permalink / raw) To: Rollins, Jameson, Notmuch Mail On Mon, May 06 2019, Jameson Rollins wrote: > From: Jameson Graef Rollins <jrollins@finestructure.net> > > If NOTMUCH_TEST_SERIALIZE is non-null all tests will be run in series, > rather than in parallel. While I like this parallelization option, and hope a version (could be even David's smtp_dummy change) of it could be available in notmuch repository as soon as possible, I would not like it being default -- just like make -J is not default... ... it being default, unsuspicious user running `make test` might have his multitasking maching eating too much resources for a particular purpose and slowing everything else. The simplest way to invoke parallelized tests could be make test-parallel and/or make test p=1 ...or NOTMUCH_TEST_PARALLELIZE make test. That's my opinion written out loud... vote me over if disagreed.. :D Tomi > --- > test/README | 8 ++++++-- > test/notmuch-test | 2 +- > 2 files changed, 7 insertions(+), 3 deletions(-) > > diff --git a/test/README b/test/README > index b378c3ff..3f54af58 100644 > --- a/test/README > +++ b/test/README > @@ -43,7 +43,6 @@ these system tools. Most often the tests are written, reviewed and > tested on Linux system so such portability issues arise from time to > time. > > - > Running Tests > ------------- > The easiest way to run tests is to say "make test", (or simply run the > @@ -105,9 +104,14 @@ to with emacs, e.g. > > make test TEST_CC=gcc TEST_CFLAGS="-g -O2" > > +Parallel Execution > +------------------ > +If either the moreutils or GNU "parallel" utility is available all > +tests will be run in parallel. If the NOTMUCH_TEST_SERIALIZE variable > +is non-null all tests will be executed sequentially. > + > Quiet Execution > --------------- > - > Normally, when new script starts and when test PASSes you get a message > printed on screen. This printing can be disabled by setting the > NOTMUCH_TEST_QUIET variable to a non-null value. Message on test > diff --git a/test/notmuch-test b/test/notmuch-test > index a4b7a1eb..bd3e080a 100755 > --- a/test/notmuch-test > +++ b/test/notmuch-test > @@ -40,7 +40,7 @@ fi > > trap 'e=$?; kill $!; exit $e' HUP INT TERM > # Run the tests > -if command -v parallel >/dev/null ; then > +if test -z "$NOTMUCH_TEST_SERIALIZE" && command -v parallel >/dev/null ; then > if parallel -h | grep -q GNU ; then > echo "INFO: running tests with GNU parallel" > printf '%s\n' $TESTS | $TEST_TIMEOUT_CMD parallel > -- > 2.20.1 > > _______________________________________________ > notmuch mailing list > notmuch@notmuchmail.org > https://notmuchmail.org/mailman/listinfo/notmuch ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] tests: environment variable to specify that tests should be serialized 2019-05-06 19:15 ` [PATCH] tests: environment variable to specify that tests should be serialized Tomi Ollila @ 2019-05-06 20:58 ` Daniel Kahn Gillmor 0 siblings, 0 replies; 22+ messages in thread From: Daniel Kahn Gillmor @ 2019-05-06 20:58 UTC (permalink / raw) To: Tomi Ollila, Rollins, Jameson, Notmuch Mail [-- Attachment #1: Type: text/plain, Size: 1056 bytes --] On Mon 2019-05-06 22:15:49 +0300, Tomi Ollila wrote: > While I like this parallelization option, and hope a version (could be even > David's smtp_dummy change) of it could be available in notmuch repository > as soon as possible, I would not like it being default -- just like make -J > is not default... > > ... it being default, unsuspicious user running `make test` might have his > multitasking maching eating too much resources for a particular purpose and > slowing everything else. if we're voting, i vote in favor of parallel defaults wherever possible, with a way to force serialization. I'd much rather have my machine finish the thing i just asked it to do in less time by default, than wait around twiddling my thumbs while the machine has spare cycles. The context switch for my mind is way more expensive than the context switches going on within the computer. The change from 2 minutes to < 30 seconds is a game-changer in terms of keeping me focused while i'm developing. make it easy to do that! --dkg [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 227 bytes --] ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/3] tests: remove some redundant pre-cleanup of the corpus MAIL_DIR 2019-05-04 20:57 ` [PATCH 1/3] tests: remove some redundant pre-cleanup of the corpus MAIL_DIR Rollins, Jameson 2019-05-04 20:57 ` [PATCH 2/3] tests: remove entangling corpus index optimization Rollins, Jameson @ 2019-05-05 10:22 ` Tomi Ollila 1 sibling, 0 replies; 22+ messages in thread From: Tomi Ollila @ 2019-05-05 10:22 UTC (permalink / raw) To: Rollins, Jameson, Notmuch Mail On Sat, May 04 2019, Jameson Rollins wrote: This change LGTM, marked trivial, removed needs-review > From: Jameson Graef Rollins <jrollins@finestructure.net> > > add_email_corpus itself does an rm -rf $MAIL_DIR, so these are not necessary. > --- > test/T100-search-by-folder.sh | 1 - > test/T650-regexp-query.sh | 1 - > 2 files changed, 2 deletions(-) > > diff --git a/test/T100-search-by-folder.sh b/test/T100-search-by-folder.sh > index a090f3d2..409cfdcc 100755 > --- a/test/T100-search-by-folder.sh > +++ b/test/T100-search-by-folder.sh > @@ -56,7 +56,6 @@ output=$(notmuch search folder:duplicate/bad/olds | notmuch_search_sanitize) > test_expect_equal "$output" "thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; Bears (inbox unread)" > > # folder: and path: searches with full corpus > -rm -rf $MAIL_DIR > add_email_corpus > > # add some more dupes > diff --git a/test/T650-regexp-query.sh b/test/T650-regexp-query.sh > index 92334ba0..43af3b47 100755 > --- a/test/T650-regexp-query.sh > +++ b/test/T650-regexp-query.sh > @@ -47,7 +47,6 @@ output=$(notmuch search 'path:/^bad$/' | notmuch_search_sanitize) > test_expect_equal "$output" "thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; To the bone (inbox unread)" > > # Use "standard" corpus from here on. > -rm -rf $MAIL_DIR > add_email_corpus > > notmuch search --output=messages from:cworth > cworth.msg-ids > -- > 2.20.1 > > _______________________________________________ > notmuch mailing list > notmuch@notmuchmail.org > https://notmuchmail.org/mailman/listinfo/notmuch ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: parallelize test suite 2019-05-04 20:57 parallelize test suite Rollins, Jameson 2019-05-04 20:57 ` [PATCH 1/3] tests: remove some redundant pre-cleanup of the corpus MAIL_DIR Rollins, Jameson @ 2019-05-04 21:33 ` Rollins, Jameson 2019-05-04 21:39 ` Daniel Kahn Gillmor 2019-05-07 10:26 ` David Bremner 3 siblings, 0 replies; 22+ messages in thread From: Rollins, Jameson @ 2019-05-04 21:33 UTC (permalink / raw) To: Notmuch Mail On Sat, May 04 2019, "Rollins, Jameson" <jrollins@caltech.edu> wrote: > This is a simple patch series that will run the entire test suite in > parallel if either the moreutils or GNU parallel utility is > available. On my 8-core machine the full test suite will now run in > under 20 seconds, which is a pretty huge improvement. Sorry I actually meant 12 seconds, not 20. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: parallelize test suite 2019-05-04 20:57 parallelize test suite Rollins, Jameson 2019-05-04 20:57 ` [PATCH 1/3] tests: remove some redundant pre-cleanup of the corpus MAIL_DIR Rollins, Jameson 2019-05-04 21:33 ` parallelize test suite Rollins, Jameson @ 2019-05-04 21:39 ` Daniel Kahn Gillmor 2019-05-04 22:53 ` David Bremner 2019-05-07 10:26 ` David Bremner 3 siblings, 1 reply; 22+ messages in thread From: Daniel Kahn Gillmor @ 2019-05-04 21:39 UTC (permalink / raw) To: Rollins, Jameson, Notmuch Mail [-- Attachment #1: Type: text/plain, Size: 650 bytes --] On Sat 2019-05-04 20:57:43 +0000, Rollins, Jameson wrote: > This is a simple patch series that will run the entire test suite in > parallel if either the moreutils or GNU parallel utility is > available. On my 8-core machine the full test suite will now run in > under 20 seconds, which is a pretty huge improvement. I've reviewed this series, and it is a *very* nice cleanup; the latency reduction makes it much more pleasant to develop notmuch safely. My computer is weaker than jamie's (i have an older 4-core i5 processor), but with this series applied, the test suite went from 1m48s to 0m29s for me. Please apply this series! --dkg [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 227 bytes --] ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: parallelize test suite 2019-05-04 21:39 ` Daniel Kahn Gillmor @ 2019-05-04 22:53 ` David Bremner 2019-05-05 15:22 ` Daniel Kahn Gillmor 0 siblings, 1 reply; 22+ messages in thread From: David Bremner @ 2019-05-04 22:53 UTC (permalink / raw) To: Daniel Kahn Gillmor, Rollins, Jameson, Notmuch Mail Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes: > On Sat 2019-05-04 20:57:43 +0000, Rollins, Jameson wrote: >> This is a simple patch series that will run the entire test suite in >> parallel if either the moreutils or GNU parallel utility is >> available. On my 8-core machine the full test suite will now run in >> under 20 seconds, which is a pretty huge improvement. > > I've reviewed this series, and it is a *very* nice cleanup; the latency > reduction makes it much more pleasant to develop notmuch safely. > > My computer is weaker than jamie's (i have an older 4-core i5 > processor), but with this series applied, the test suite went from 1m48s > to 0m29s for me. > Last time we discussed parallel test running, there we concerns about multiple versions of certain servers colliding with each other. This still seems to be at least a theoretical issue with smtp-dummy, although a glance suggests that it might only currently be used in T310-emacs.sh. I'm not sure what a robust solution is here. - gpg-agent - emacs - dtach Did I miss any other background processes run by the test suite? I can imagine gpg-agent is managed OK these days since it's started automagically by gpg. emacs seems to use the current process id in the socket name, so that also should be OK, although it should maybe be replaced with something more robust to avoid problems with pid rollover. I _think_ including the test name in the emacs server would do the trick The dtach socket is in the tmp.T* directory, so that should be OK. I wonder if a good solution would be to make running the test suite in parallel be opt-in (e.g. by configuration option). Or at least have a way to disable it for situations like CI and autobuilders. d ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: parallelize test suite 2019-05-04 22:53 ` David Bremner @ 2019-05-05 15:22 ` Daniel Kahn Gillmor 2019-05-05 16:44 ` Tomi Ollila 0 siblings, 1 reply; 22+ messages in thread From: Daniel Kahn Gillmor @ 2019-05-05 15:22 UTC (permalink / raw) To: David Bremner, Rollins, Jameson, Notmuch Mail [-- Attachment #1: Type: text/plain, Size: 2026 bytes --] On Sat 2019-05-04 19:53:23 -0300, David Bremner wrote: > Last time we discussed parallel test running, there we concerns about > multiple versions of certain servers colliding with each other. This > still seems to be at least a theoretical issue with smtp-dummy, although > a glance suggests that it might only currently be used in T310-emacs.sh. > I'm not sure what a robust solution is here. > > - gpg-agent > - emacs > - dtach > > Did I miss any other background processes run by the test suite? > > I can imagine gpg-agent is managed OK these days since it's started > automagically by gpg. gpg-agent is fine, because it's isolated by $GNUPGHOME, and each test uses a distinct $GNUPGHOME (see GNUPGHOME="${TEST_TMPDIR}/gnupg in test-lib.sh) > emacs seems to use the current process id in the socket name, so that > also should be OK, although it should maybe be replaced with something > more robust to avoid problems with pid rollover. I _think_ including the > test name in the emacs server would do the trick I would have no objection to this improvement in isolation of the emacs server processes, but i think the pid rollover race condition is so minor that i don't think it sholud be a blocker for the adoption of this series. > The dtach socket is in the tmp.T* directory, so that should be OK. > > I wonder if a good solution would be to make running the test suite in > parallel be opt-in (e.g. by configuration option). Or at least have a > way to disable it for situations like CI and autobuilders. I agree that making it possible to force serialized tests would be good. I'd prefer that running tests in parallel be the default, though i wouldn't object to a ./configure --serialize-tests option if someone wants to implement it. I actually think that CI and autobuilders *should* exercise the parallel tests, as annoying as that might be initially, because it seems likely to catch any other potential entanglements. thanks for the review! --dkg [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 227 bytes --] ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: parallelize test suite 2019-05-05 15:22 ` Daniel Kahn Gillmor @ 2019-05-05 16:44 ` Tomi Ollila 2019-05-06 21:39 ` Daniel Kahn Gillmor 0 siblings, 1 reply; 22+ messages in thread From: Tomi Ollila @ 2019-05-05 16:44 UTC (permalink / raw) To: Daniel Kahn Gillmor, David Bremner, Rollins, Jameson, Notmuch Mail On Sun, May 05 2019, Daniel Kahn Gillmor wrote: > On Sat 2019-05-04 19:53:23 -0300, David Bremner wrote: >> Last time we discussed parallel test running, there we concerns about >> multiple versions of certain servers colliding with each other. This >> still seems to be at least a theoretical issue with smtp-dummy, although >> a glance suggests that it might only currently be used in T310-emacs.sh. we,d need to change 25025 with something else -- configured in the test script ? -- same could be used as part of emacs server socket ? >> I'm not sure what a robust solution is here. >> >> - gpg-agent >> - emacs >> - dtach >> >> Did I miss any other background processes run by the test suite? >> >> I can imagine gpg-agent is managed OK these days since it's started >> automagically by gpg. > > gpg-agent is fine, because it's isolated by $GNUPGHOME, and each test > uses a distinct $GNUPGHOME (see GNUPGHOME="${TEST_TMPDIR}/gnupg in > test-lib.sh) > >> emacs seems to use the current process id in the socket name, so that >> also should be OK, although it should maybe be replaced with something >> more robust to avoid problems with pid rollover. I _think_ including the >> test name in the emacs server would do the trick > > I would have no objection to this improvement in isolation of the emacs > server processes, but i think the pid rollover race condition is so > minor that i don't think it sholud be a blocker for the adoption of this > series. when test code done correctly (test_done ends it), the shell launching emacs holds the pid $$ until emacs exits, so the pid is not reused in name for any emacs process (but if bash died leaving emacs running then we'd have chance for collisions...) > >> The dtach socket is in the tmp.T* directory, so that should be OK. >> >> I wonder if a good solution would be to make running the test suite in >> parallel be opt-in (e.g. by configuration option). Or at least have a >> way to disable it for situations like CI and autobuilders. > > I agree that making it possible to force serialized tests would be good. > > I'd prefer that running tests in parallel be the default, though i > wouldn't object to a ./configure --serialize-tests option if someone > wants to implement it. no need for configure option, one can just remove parallel(1) from system ;) ... well, we could use ${PARALLEL:=parallel} in script for users to shadow it out in some other way... > I actually think that CI and autobuilders *should* exercise the parallel > tests, as annoying as that might be initially, because it seems likely > to catch any other potential entanglements. ... after it has been proven a bit more to work... One thing more, the "perverse" pre-caching done in add_email_corpus can be retained -- just do that step *before* going to parallelism... > > thanks for the review! > > --dkg Tomi ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: parallelize test suite 2019-05-05 16:44 ` Tomi Ollila @ 2019-05-06 21:39 ` Daniel Kahn Gillmor 0 siblings, 0 replies; 22+ messages in thread From: Daniel Kahn Gillmor @ 2019-05-06 21:39 UTC (permalink / raw) To: Tomi Ollila, David Bremner, Rollins, Jameson, Notmuch Mail [-- Attachment #1: Type: text/plain, Size: 975 bytes --] On Sun 2019-05-05 19:44:01 +0300, Tomi Ollila wrote: >> I actually think that CI and autobuilders *should* exercise the parallel >> tests, as annoying as that might be initially, because it seems likely >> to catch any other potential entanglements. > > ... after it has been proven a bit more to work... i tend to think the other way around -- the way to prove it is to push it into the CI pipelines on the development branch, rather than trying to prove it on individual developer systems. > One thing more, the "perverse" pre-caching done in add_email_corpus > can be retained -- just do that step *before* going to parallelism... You could do that, but i'm not sure how much you'd gain from it -- its current implementation is all mixed up with the parallelized tests, anyway. If you wanted to propose a concrete re-optimization like this, i'd be happy to review it, test it and report back on the speedup gained. thanks for thinking this through, Tomi! --dkg [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 227 bytes --] ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: parallelize test suite 2019-05-04 20:57 parallelize test suite Rollins, Jameson ` (2 preceding siblings ...) 2019-05-04 21:39 ` Daniel Kahn Gillmor @ 2019-05-07 10:26 ` David Bremner 3 siblings, 0 replies; 22+ messages in thread From: David Bremner @ 2019-05-07 10:26 UTC (permalink / raw) To: Rollins, Jameson, Notmuch Mail "Rollins, Jameson" <jrollins@caltech.edu> writes: > This is a simple patch series that will run the entire test suite in > parallel if either the moreutils or GNU parallel utility is > available. On my 8-core machine the full test suite will now run in > under 20 seconds, which is a pretty huge improvement. I've taken the plunge and merged this series to master. I also posted an update to smtp-dummy following Tomi's suggestion to bind to port 0. d ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2019-05-10 10:17 UTC | newest] Thread overview: 22+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-05-04 20:57 parallelize test suite Rollins, Jameson 2019-05-04 20:57 ` [PATCH 1/3] tests: remove some redundant pre-cleanup of the corpus MAIL_DIR Rollins, Jameson 2019-05-04 20:57 ` [PATCH 2/3] tests: remove entangling corpus index optimization Rollins, Jameson 2019-05-04 20:57 ` [PATCH 3/3] tests: run all tests in parallel, if available Rollins, Jameson 2019-05-06 4:44 ` [PATCH] tests: environment variable to specify that tests should be serialized Rollins, Jameson 2019-05-06 10:32 ` [PATCH] test: add configurable port to smtp-dummy David Bremner 2019-05-06 19:39 ` Tomi Ollila 2019-05-06 20:55 ` Daniel Kahn Gillmor 2019-05-07 10:20 ` [PATCH] test: let the OS choose a port for smtp-dummy David Bremner 2019-05-07 12:38 ` Daniel Kahn Gillmor 2019-05-08 15:59 ` Tomi Ollila 2019-05-10 10:16 ` David Bremner 2019-05-06 19:15 ` [PATCH] tests: environment variable to specify that tests should be serialized Tomi Ollila 2019-05-06 20:58 ` Daniel Kahn Gillmor 2019-05-05 10:22 ` [PATCH 1/3] tests: remove some redundant pre-cleanup of the corpus MAIL_DIR Tomi Ollila 2019-05-04 21:33 ` parallelize test suite Rollins, Jameson 2019-05-04 21:39 ` Daniel Kahn Gillmor 2019-05-04 22:53 ` David Bremner 2019-05-05 15:22 ` Daniel Kahn Gillmor 2019-05-05 16:44 ` Tomi Ollila 2019-05-06 21:39 ` Daniel Kahn Gillmor 2019-05-07 10:26 ` David Bremner
Code repositories for project(s) associated with this public inbox https://yhetil.org/notmuch.git/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).