On Thu, May 7, 2009 at 10:02 AM, Yang Tse <yangsita_at_gmail.com> wrote:

> 2009/5/1, David McCreedy wrote:
>
> > The Curl test suite is very useful.
>
> Absolutely true.
>
> > This patch allows various HTTP and FTP tests to work on both ASCII and
> > non-ASCII platforms alike.
>
> I'll make some comments...
>
> This patch sets the CURLOPT_TRANSFERTEXT option, conditionally on
> CURL_DOES_CONVERSIONS definition for tests lib508.c, lib510.c,
> lib552.c, lib553.c, and lib555.c.
>
> Is there any reason for not doing the same now on nearly all lib5XX.c
> source files, even when the test data might not be non-ASCII ready
> yet, and the test might not be HTTP POST related?
>
> I don't see any harm in doing so, but I might be overlooking something.

The biggest reason that comes to mind is to maintain a mix of tests with and
without the option specified.
On TPF the user won't always specify the option so both paths through the
logic are important to hit.

A lesser reason is tidiness.
Having that option set for a test where it has no effect can be confusing...
"Why did they set THAT?"

I took the "tread lightly" approach to my changes and tried to minimize
them.
I can try adding the option to other lib5xx.c's if you like that approach
better.
But I do like the mix of tests with and without the option.
Please let me know.

>
> Regarding the string literals writing inside the lib5XX.c tests only
> as ASCII hex-dump I personally would prefer to keep this kind of
> 'writing' inside CURL_DOES_CONVERSIONS conditionals, and the existing
> representation/writing on the 'else' part of the condition.
>
> Would this still work for you?
>

Yes, it should. I'll work up new patches.

>
> Regarding the unconditional removal of the potential trailing CR using
> the <strip> or <strippart> section for test case definitions #508 and
> #555 I think this is not the way to go. We would weaken those tests
> cases for systems which don't use CURL_DOES_CONVERSIONS.
>
> Taking in consideration that you will most probably need to adjust
> other, existing or feature, test cases I think you should consider
> slightly expanding the test definition file format.
>
> You could first try to use definitions for tests 508 and 555 using
> <protocol nonewline="yes">
>
> If the above works for you, the next step would be to modify
> runtests.pl to allow handling something like <protocol
> nonewline="yes-when-curl-does-conversions">. Otherwise handling of
> <protocol nocarriagereturn="yes-when-curl-does-conversions"> will be
> needed. And documenting the new functionality in tests/FILEFORMAT.
>

I'll try these ideas out.

Thanks,

-David

>
> --
> -=[Yang]=-
>
Received on 2009-05-07