[Swan-dev] obsolete things that we still use

[Paul Wouters]

On Sun, 19 Aug 2018, Andrew Cagney wrote:

>> | done. Note that I wiped testing/scripts as it was full of cruft from
>> | freeswan style OE. We dont really need readwriteconf tests for each
>> | feature for which we have a real unit test with packet flow and all
>> | anyway.
>>
>> Without looking carefully, I'd like to make a plea for keeping
>> and maintaining readwriteconf tests.

I never said to dump all readwriteconf tests. just the ones in
testing/scripts. For instance, we do have testing/pluto/readwriteconf-01.
And I think we should be able to bundle all readwriteconf tests into one
"pluto test". Or we can port or make a new test type to do this, but
that seems overkill to me. My point was that the testing/scripts tests
were uselessly out of date and didn't take anything from even the
openswan days into account, let alone the last 7 libreswan years.

>> If I remember correctly, I was able to run them quickly, without the
>> slow machinery of starting and stopping virtual machines.  I don't
>> remember how I did it :-(
>
> Sounds a bit like algparse. which is, arguably, a case study in unit test dogma:

You can run: ipsec readwriteconf /some/file.conf | diff knowngood.conf

And I recently added the option for readwriteconf to only output a named
connection (so you dont get all the cruft that is loaded via also= but
isn't used in the conn you care about, another reason those
testing/scripts test were so obsoleted)

ipsec readwriteconf --connection foo.com /some/file.conf | diff knowngood.conf

> -  the tests algparse-*{,fips} were added testing the crap out of it
> (but everyone but me largely ignores them)

I mostly ignored it because I wasn't sure about the diff being a
temporary work in progress thing or a final new result. I tend to fix
these when I know for sure, such as enumcheck.

Note readwriteconf itself is buggy and outputs a few double entries. It
also re-adds defaults that were not specified. Some of its code is very
custom, leaving me to doubt if it would be a good diagnostic at all.

A better solution would be to have readwriteconf call add_connection()
then add a function that takes a struct connection, and turns it into a
config file output. That's not what readwriteconf does. It currently
reads a config file into a struct starter_config and then manually
writes out this struct into what it thinks would be config file output.
But this "thinking" only lives in readwriteconf (well, libipsecconf's
confwrite()). But this reality is distinct from pluto taking a whack
message and converting it to a connection.

Also, when we end up adding dbus or other support, we will have another
interface to go from "configuration" to a struct connection.

Paul