Dan Fandrich <[email protected]> wrote:

> 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.

Thanks.

> 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.

Agreed. Attached is an updated version that uses CURL_TESTDIR instead.
Any other name is fine with me, too.

> > 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.

It should work independently. $TESTDIR defaults to $srcdir/data
therefore $TESTDIR/.. equals $srcdir.

> >     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.

Test attached. My impression was that curl doesn't parse the
Date header, but I agree that testing that it behaves as expected
is a good idea anyway.

I also attached a patch to align the -l output again.

> >     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.

I guess I mainly went by the test name which doesn't mention this.

If the invalid HTTP-version is by design I agree that the patch
should be rejected.

Fabian
From f348dd0c2c2f64f592781a97e88854ecd42ef5f6 Mon Sep 17 00:00:00 2001
From: Fabian Keil <[email protected]>
Date: Sun, 15 Jun 2014 17:19:58 +0200
Subject: [PATCH 1/3] Allow to overwrite $TESTDIR with the environment variable
 CURL_TESTDIR

This makes it more convenient to execute custom tests that
aren't relevant for curl itself and thus not upstreamed.
---
 tests/runtests.pl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/runtests.pl b/tests/runtests.pl
index 1264c48..f761f34 100755
--- a/tests/runtests.pl
+++ b/tests/runtests.pl
@@ -153,7 +153,7 @@ my $VCURL=$CURL;   # what curl binary to use to verify the servers with
                    # just built hangs or crashes and thus prevent verification
 my $DBGCURL=$CURL; #"../src/.libs/curl";  # alternative for debugging
 my $LOGDIR="log";
-my $TESTDIR="$srcdir/data";
+my $TESTDIR = $ENV{'CURL_TESTDIR'} || "$srcdir/data";
 my $LIBDIR="./libtest";
 my $UNITDIR="./unit";
 # TODO: change this to use server_inputfilename()
-- 
1.9.0

From 7e9c5c45d3fb9cf758dcf9f5589a5af3ddb8602c Mon Sep 17 00:00:00 2001
From: Fabian Keil <[email protected]>
Date: Sun, 27 Jul 2014 12:14:50 +0200
Subject: [PATCH 2/3] runtests.pl: Pad test case numbers with up to three
 zeroes

Test case numbers with four digits have been available for a
while now.
---
 tests/runtests.pl | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/runtests.pl b/tests/runtests.pl
index f761f34..89dc5dd 100755
--- a/tests/runtests.pl
+++ b/tests/runtests.pl
@@ -3167,14 +3167,14 @@ sub singletest {
         if(!$short) {
             if($skipped{$why} <= 3) {
                 # show only the first three skips for each reason
-                logmsg sprintf("test %03d SKIPPED: $why\n", $testnum);
+                logmsg sprintf("test %04d SKIPPED: $why\n", $testnum);
             }
         }
 
         timestampskippedevents($testnum);
         return -1;
     }
-    logmsg sprintf("test %03d...", $testnum) if(!$automakestyle);
+    logmsg sprintf("test %04d...", $testnum) if(!$automakestyle);
 
     # extract the reply data
     my @reply = getpart("reply", "data");
-- 
1.9.0

Attachment: 0003-Add-test326-Date-header-whose-weekday-doesn-t-match-.patch.gz
Description: application/gzip

Attachment: signature.asc
Description: PGP signature

-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette:  http://curl.haxx.se/mail/etiquette.html

Reply via email to