Re: [PATCH] test: add configurable port to smtp-dummy

[Tomi Ollila <tomi.ollila@iki.fi>]

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\")
> --