[Swan-dev] testing: drifting reference logs

[Paul Wouters]

On Tue, 30 Dec 2014, D. Hugh Redelmeier wrote:

> | From: Antony Antony <antony at phenome.org>
> | Subject: Re: [Swan-dev] testing: more results, where 169.x.x.x is removed
> |
> | On Mon, Dec 29, 2014 at 01:56:44AM -0500, D. Hugh Redelmeier wrote:
>
> | > | New commits:
> | > | commit 11749924b5a37c3b0fd6284a7c98cf9caed98212
> | > | Author: Antony Antony <antony at phenome.org>
> | > | Date:   Sun Dec 28 14:54:42 2014 +0100
> | > |
> | > |     testing: more results, where 169.x.x.x is removed
> | >
> | > Thanks.
> | >
> | > Should all occurrences of 169.254.x.x be removed from the reference
> | > output?
> |
> | IMHO all tests that pass should be updated; the failed ones I am not
> | sure. For now I only update tests that pass for me. And no output for
> | wip tests.
>
> Why the heck would you want to keep "junk differences" in the logs?
>
> I'm not saying: adopt the log of a failed test as the reference log.
> That would be stupid and horrible.  I'm saying: do surgery on the
> reference logs that matches your new expectations.

Antony and I have a difference of opinion here, I also tend to run the
sed script to fix the output even for those tests that don't run for
whatever other reason. One reason is exactly this issue, someone else
might not know this change is "expected".

> I think that updating by editing is a very good technique.  It makes
> sure that only intended changes get made.  If you adopt the output of
> a run, you must be very careful that unintended changes accidentally
> tag along with the intended change.

It is important to run "git diff" afterwards and carefully check that
it did not do something unexpected. I've caught a few things this way
but I also missed a few things even with this method.

> | And also the git log of
> | failed test's can be confusing, these automagic updates may confuse you.
>
> Not half as confusing as the spurious failures that I have to
> repeatedly track down.

Especially if it is a commit touching 200 files with a clear message
like "disabled avahi on host, so 169.* routes are no longer there".

> | Some test cases are missing
> | from the file TESTLIST, e.g. x509-crl-05. 'make check' do not run those.
>
> That brings up another point.  Should they be deleted?  Fixed?  Marked
> as WIP?

All the (non-oe) uml tests should get converted to WIP tests. I might
spend some time on that in the next few days as our efforts to convert
uml tests has stalled.

> | From: Antony Antony <antony at phenome.org>
> | Subject: Re: [Swan-dev] ikev2-06-6msg failure
> |
> | > -002 "westnet-eastnet-ikev2" #1: Received anti-DDOS COOKIE -resending I1 with cookie payload
> |
> | It got removed in commit ca99295e. I think  that message was useful.
>
> Thanks for the pointer.
>
> | From: Paul Wouters <paul at nohats.ca>
> | Subject: Re: [Swan-dev] ikev2-06-6msg failure
> |
> | >> -002 "westnet-eastnet-ikev2" #1: Received anti-DDOS COOKIE -resending I1 with cookie payload
> |
> | It was changed to debug only because of ddos concern
>
> So please update the reference logs.

I'll fix it, but I will want a grep in the console output for the new
log line so that we confirm still that the cookie was sent.

> | Sent from my iPhone
>
> I hate top-posting.  Get a better phone :-)
>
> On the other hand, I like the quick response this phone enables.

That's the trade of :)

> Please do fix the reference logs.  But only if you are confident that
> each change is a fix.

I've spend the last weeks mostly in the invalid-ke branch and haven't
given the master repo much attention. With the invalid-ke parts merged
in now, I started looking at the aes_ctr branch to merge into master.

With that done, I'm back on master and would like to prepare it for a
release, meaning going through the full test run.

Paul