Hoi.
[2018-07-19 03:24] Philipp Takacs <phil...@bureaucracy.de>
> [2018-07-18 23:13] Vasilii Kolobkov <polezaivs...@ko5v.net>
> > I'm having the tests green... well technically they are black, but
> > function much better to their end on OpenBSD :)
>
> Thanks.
>
> > Yet two of them do fail, but that'd be another patch. The ali tests
> > for the blind lists fail - it's the first time i came to know of
> > this feature and frankly don't know if i'd want to work on fixing
> > that. Does anybody use them?
>
> If you talk about the test which fails on markus nightly tests[0],
> it's a known bug, but down on the priority list. I don't know if
> someone use this feature[1], but I would like to have this sane
> implemented.
My view is the same.
> > The other broken part is second to last case in test/send/test-mimeify.
> > The sizes reported for multipart/... types differ from expected
> > values. I'll be looking further into it, but wonder if it's broken
> > on your systems as well?
>
> This also a known bug and behaves realy strange, because sometimes
> the numbers differ. I have looked at this some time ago, but because
> the mime implementation is realy complex I coldn't find it.
These could have been my words as well. ;-)
If one could track that down, that would be great.
> In general I like your patch, but I have some comments:
>
> > p.s. i prefixed all the portable functions with p* so to not shadown
> > the system ones just in case.
Now that I thought about that, I wonder if we should name the
compat functions in the shell scripts the same way as we name
them in the C code: by prefixing them with ``mh''. But OTOH, these
functions in the C code usually don't provide 1:1 functionality
but rather slightly adjusted functionality. Still, the ``mh'' or
``mh_'' prefix shows quicker what's going on, IMO.
Your oppinion on this?
> > ---------------8<---------------
> > From 204ae88524838063493242f012a1b1fe2fcf2d05 Mon Sep 17 00:00:00 2001
> > From: Vasilii Kolobkov <polezaivs...@ko5v.net>
> > Date: Sun, 15 Jul 2018 23:05:09 +0200
> > Subject: [PATCH] Fix tests to run on OpenBSD
> >
> > For there's no seq(1) in OpenBSD, Use pseq function from common.sh
> > for generating sequence numbers. Credits for the function goes
> > to Markus Schnalke.
> >
> > He also had a one-liner, which having less features is so freaking
> > nice to not mention it here:
> > < /dev/zero tr \\000 \\n | sed 10q | nl -b a
> >
> > awk(1) doesn't support interval regular expression syntax ({n,m})
> > on some BSDs.
> >
> > hostname(1) has no -f flag on OpenBSD and, while invocation without
> > flags is supposed to return FQDN allright. Use the same phostname
> > function in mhsign(1).
> >
> > mktemp(1) is limited to at least 6 placeholder symbols on OpenBSD.
> > ---
> > test/common.sh | 18 ++++++++++++++++++
> > test/runtest | 4 +++-
> > test/tests/inc/test-eom-align | 2 +-
> > test/tests/mhsign/test-mhsign | 11 +++++++++--
> > test/tests/mhstore/test-filenames | 2 +-
> > test/tests/show/test-longlines | 6 ++----
> > uip/mhsign.sh | 11 ++++++++++-
> > 7 files changed, 44 insertions(+), 10 deletions(-)
> >
> > diff --git a/test/common.sh b/test/common.sh
> > index 805afea..056eac2 100644
> > --- a/test/common.sh
> > +++ b/test/common.sh
> > @@ -105,7 +105,25 @@ runandcheck()
> >
> > if [ "$diff" ]; then
> > echo "$0: $1 failed"
> > echo "$diff"
> > failed=`expr "${failed:-0}" + 1`
> > fi
> > }
> > +
> > +pseq() {
> > + start=1
> > + inc=1
> > +
> > + case $# in
> > + 1) end="$1" ;;
> > + 2) start="$1"; end="$2" ;;
> > + 3) start="$1"; inc="$2" ; end="$3" ;;
> > + *) echo "Usage: pseq end | start end | start step end" >&2; return 1 ;;
> > + esac
> > +
> > + awk 'BEGIN{
> > + for (i='"$start"'; i<='"$end"'; i+='"$inc"') {
> > + print i
> > + }
> > + }'
> > +}
>
> I'm not sure, if we realy nead this funktion, because it's used on
> two places. In the one place it's easy to remove. On the other place
> I would remove the hole test.
>
> > diff --git a/test/runtest b/test/runtest
> > index 9f35ade..37a23ac 100755
> > --- a/test/runtest
> > +++ b/test/runtest
> > @@ -1,13 +1,15 @@
> > #!/bin/sh
> >
> > set -e
> >
> > export MH_TEST_COMMON="$PWD/common.sh"
> >
> > +. ${MH_TEST_COMMON}
> > +
>
> Would be nice if this patch would also remove the source in
> the single tests.
I wonder why we haven't done so already? It seems to be pretty
obvious to load the helper scripts for all tests during the test
setup. Was it initially implemented this way due to
time-performance motivation in nmh and we just never thought
about it? Or is there another reason? If there is no other reason
and the tests work fine with the sourcing in runtest, then we
should do as Philipp suggests.
> > if [ ! -f test-temp-dir ]; then
> > echo "test-temp-dir not found: running setup-test"
> > ./setup-test
> > fi
> >
> > export MH_TEST_DIR=`cat test-temp-dir`
> >
> > @@ -32,15 +34,15 @@ cat >"$MMH/profile" <<-!
> > Inbox: +inbox
> > !
> > folder -create `mhparam inbox` >/dev/null
> > folder -create `mhparam trashfolder` >/dev/null
> > folder -create `mhparam draftfolder` >/dev/null
> >
> > # create 10 basic messages
> > -for i in `seq 1 10`;
> > +for i in `pseq 10`;
>
> Markus also mentiont to just use "1 2 3 4 5 6 7 8 9 10". I would
> say this is absolutly ok for this usage.
>
> > do
> > cat >"$MAILDIR/inbox/$i" <<-!
> > From: Test$i <test$i...@example.com>
> > To: Some User <u...@example.com>
> > Date: Fri, 29 Sep 2006 00:00:00
> > Subject: Testing message $i
> >
> > diff --git a/test/tests/inc/test-eom-align b/test/tests/inc/test-eom-align
> > index 2b6afb4..0e9c1dd 100644
> > --- a/test/tests/inc/test-eom-align
> > +++ b/test/tests/inc/test-eom-align
> > @@ -111,13 +111,13 @@ do_one_test_B () {
> >
> > # Cover a decent range around the stdio buffer size to make sure we catch
> > # any corner cases whether they relate to total message size equal to
> > # buffer size or to body size equal to buffer size.
> > START=$(($STDIO_BUFSZ - 16))
> > FINISH=$(($STDIO_BUFSZ + $HDRSZ + $FROMLINESZ + 32))
> > echo "Testing inc of files with various alignments of eom marker with
> > buffer size..."
> > -for sz in $(seq $START $FINISH); do
> > +for sz in $(pseq $START $FINISH); do
>
> On this test we need the seq, but do we need the test itself.
> This test checks if inc works correct for some cornercase around
> the bufsize[2]. I belive this test was necesare for m_getfld
> poking around in the stdio internel buffers, but with m_getfld2
> it's not realy a usefull test.
The test description is as follows:
# Test all combinations of alignment of the end-of-message delimiter
# with the end of a stdio buffer
Philipp's reasoning is sane. m_getfld2 does not do such dangerous
stuff anymore, thus this error pattern became obsolete now. We
don't fiddle with stdio buffers anymore; they'll just work. Hence
I agree to drop the test (which is nice because the test was
awkward anyways ... process meter and long-running).
In the same go, I wonder what the critical error pattern of
m_getfld2 is. We should add a test case for that pattern, instead.
> > progress_update $sz $START $FINISH
> > do_one_test_A $sz
> > do_one_test_B $sz
> > done
> > progress_done
> > diff --git a/test/tests/mhsign/test-mhsign b/test/tests/mhsign/test-mhsign
> > index 3c2bb97..4f24b09 100755
> > --- a/test/tests/mhsign/test-mhsign
> > +++ b/test/tests/mhsign/test-mhsign
> > @@ -3,14 +3,21 @@
> > #
> > # Test mhsign (correct alias expansion with -enc)
> > #
> > ######################################################
> >
> > . "$MH_TEST_COMMON"
> >
> > +phostname()
> > +{
> > + case `uname -s` in
> > + OpenBSD) hostname ;;
> > + *) hostname -f ;;
> > + esac
> > +}
>
> oh hostname, markus has summarised[3] this very good. I wouldn't
> change the test, because it does exact the same as mhsing. Maybe
> after fixing mhsign.
>
> > # setup some aliases
> >
> > cat >"$MH_TEST_DIR/.mmh/aliases" <<!
> > a1: unknownperson
> > a2: unknownper...@example.org
> > a3: Unknown Person <unknownperson>
> > @@ -62,16 +69,16 @@ Unknown Person <unknownper...@example.org>
> > "Unknown Person" <unknownper...@example.org>
> > (Unknown) <unknownper...@example.org> (Person)
> > Unknown <unknownper...@example.org>
> > unknownper...@example.org
> > !
> >
> > runandcheck "mhsign -enc $draft" <<!
> > -Could not find key for <unknownperson@`hostname -f`>
> > +Could not find key for <unknownperson@`phostname`>
> > Could not find key for <unknownper...@example.org>
> > -Could not find key for <unknownperson@`hostname -f`>
> > +Could not find key for <unknownperson@`phostname`>
> > Could not find key for <unknownper...@example.org>
> > Could not find key for <unknownper...@example.org>
> > Could not find key for <unknownper...@example.org>
> > Could not find key for <unknownper...@example.org>
> > Could not find key for <unknownper...@example.org>
> > !
> > diff --git a/test/tests/mhstore/test-filenames
> > b/test/tests/mhstore/test-filenames
> > index a1d3c3d..4cfedd3 100755
> > --- a/test/tests/mhstore/test-filenames
> > +++ b/test/tests/mhstore/test-filenames
> > @@ -7,15 +7,15 @@
> >
> > . "$MH_TEST_COMMON"
> >
> > require_locale en_US.utf-8 en_US.utf8
> > LC_ALL=en_US.UTF-8
> > export LC_ALL
> >
> > -tempdir=`TMPDIR=$MH_TEST_DIR mktemp -d -t "XXXXX"`
> > +tempdir=`TMPDIR=$MH_TEST_DIR mktemp -d -t "XXXXXX"`
>
> Why not just ``mktemp -d''?
Yes, leave `-t' out. The manpage of GNU coreutils 8.12 mktemp(1)
writes, `-t' is deprecated!
> > cd $tempdir
> >
> > msgfile=`mhpath b`
> > cat > $msgfile <<EOF
> > To: recipi...@example.com
> > From: sen...@example.com
> > Subject: mhlist test
> > diff --git a/test/tests/show/test-longlines b/test/tests/show/test-longlines
> > index 138c724..c15c1cb 100644
> > --- a/test/tests/show/test-longlines
> > +++ b/test/tests/show/test-longlines
> > @@ -9,21 +9,19 @@ set -e
> >
> > expected=$MH_TEST_DIR/$$.expected
> > actual=$MH_TEST_DIR/$$.actual
> >
> > genlongsubject() {
> > len="${1:-998}"
> > awk -v len="$len" 'BEGIN {
> > - prefix = "Subject: " len
> > + prefix = "Subject: " len " "
> > while (i++<len) {
> > s = s "x"
> > }
> > - re = ".{" length(prefix) "}."
> > - sub(re, prefix " ", s)
> > - print s
> > + print prefix substr(s, length(prefix) + 1)
> > }'
> > }
> >
> > addcr() {
> > awk '{printf($0 "\r\n")}'
> > }
> >
> > diff --git a/uip/mhsign.sh b/uip/mhsign.sh
> > index 39a3f69..03121ac 100755
> > --- a/uip/mhsign.sh
> > +++ b/uip/mhsign.sh
> > @@ -97,14 +97,23 @@ lookupkeyring() {
> > if [ $? != 0 ] ; then
> > return 1
> > fi
> > echo "$key" | sed -n '/^pub:[^idre]:/{p;q;}' | cut -d: -f5
> > return 0
> > }
> >
> > +### query FQDN depending on a host system
>
> This should be a sepprat patch. Also look at the mail mentiont before.
Yes.
> > +phostname()
> > +{
> > + case `uname -s` in
> > + OpenBSD) hostname ;;
> > + *) hostname -f ;;
> > + esac
> > +}
> > +
> > ### lookupkeys file -- set $KL to list of recipient keys
> > lookupkeys() {
> > KL=
> > status=0
> > if whom -ali -notocc -bcc "$1" >/dev/null ; then
> > echo "Encryption is not supported for BCCs" >&2
> > return 1
> > @@ -116,15 +125,15 @@ lookupkeys() {
> > addresses=`%libdir%/ap -form "=$format" "$addresses"`
> >
> > for i in $addresses ; do
> > case "$i" in
> > '|'*) echo "Ignoring pipe address" >&2
> > continue ;;
> > *@*) ;;
> > - *) i="$i@`hostname -f`" ;;
> > + *) i="$i@`phostname`" ;;
> > esac
> > if k=`lookupkeyfile "$i"` ; then
> > KL="$KL $k"
> > elif k=`lookupkeyring "$i"` ; then
> > KL="$KL $k"
> > else
> > echo "Could not find key for <$i>" >&2
> > --
> > 2.15.1
> >
> > --------------->8---------------
> >
>
>
> Philipp
>
> [0] http://marmaro.de/prog/mmh/nightly/tests-log/runalltests.log
> [1] and I don't think so
> [2] and event this is hardcoded, so not realy usefull.
> [3] https://www.mail-archive.com/mmh@marmaro.de/msg00559.html
>
> Ps: @markus your test fails on your nightly test, I don't know if this
> is a bug in your envirement or in mhl. I'll look at the weekend.
(see separate message)
meillo
