cURL / Mailing Lists / curl-library / Single Mail

curl-library

Re: Patches to make testing external proxies more convenient

From: Dan Fandrich <dan_at_coneharvesters.com>
Date: Sat, 26 Jul 2014 23:34:18 +0200

Hi, Fabian. I've applied a number of this batch of patches (the short,
trivial ones). That's not to say that the others don't have value, just that
I haven't thought about them much, noone else has chimed in one way or the
other, and they touch thousands of files and it's too late in the day for me
to take the risk. I have a couple of comments, though.

On Fri, Jul 04, 2014 at 03:38:39PM +0200, Fabian Keil wrote:

> Subject: [PATCH 1/6] Allow to overwrite $TESTDIR through the environment

This one seems like it could be useful. I would prefer that the environment
variable be less generic; something with a CURL prefix would be better.

> Subject: [PATCH 2/6] runtests.pl: Let runhttpserver() use $TESTDIR instead of
> $srcdir

It seems that this one must be combined with the first in order to maintain a
working test system.

> Subject: [PATCH 3/6] runtests.pl: Add a %TESTNUMBER variable to make copying
> tests more convenient

Sounds useful. There are plenty of tests with hard-coded test numbers, and there
have been a couple of cases where cut-and-pasting of tests has resulted in
errors due to test numbers not being updated at the same time.

> Subject: [PATCH 4/6] getpart.pm: Fix a comment typo

Applied.

> Subject: [PATCH 5/6] runtests.pl: Don't expect $TESTDIR/DISABLED to exist

Applied.

> Subject: [PATCH 6/6] runtests.pl: Remove filteroff() which hasn't been used
> since 2001

On Fri, Jul 25, 2014 at 04:39:37PM +0200, Fabian Keil wrote:
>
> tests: Make sure the weekday in Date headers matches the date

A reasonable idea. But, there should probably be a test added with a
deliberate weekday/date mismatch to ensure that libcurl still works
with that combination.

> tests: Fix a couple of incomplete response lines

Applied.

> tests: Consistently use no whitespace in front of tags belonging to the test

It's funny how consistently this inconsistency is found in the tests :^) It's
probably a result of many years of cutting-and-pasting a few older tests that
happened to indent these few lines. But I'm all for consistency.

> test15: 'Downgrade' protocol version to 1.1
>
> The bogus HTTP version is unrelated to what is being tested
> and changing it to a valid one allows to use the test with
> proxies that reject responses with unsupported HTTP versions.

I don't agree with this sole patch. The whole purpose of the test is to ensure
that some future, unknown HTTP version is still handled by libcurl, so this
requires that a "bogus" HTTP version is used, as this version may not be so
bogus in the future.

>>> Dan
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Received on 2014-07-26