[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Bug-wget] [PATCH] Added wget http test for 503 Service unavailable
From: |
Darshit Shah |
Subject: |
Re: [Bug-wget] [PATCH] Added wget http test for 503 Service unavailable |
Date: |
Sat, 14 Mar 2015 17:09:32 +0530 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
Hi Satyam,
Great work! And thanks for the contribution. However, I have a couple of queries
about this test which I've mentioned in-line.
From ce32f9ee17fcd9544a34cf9e3656ee7e10ea289d Mon Sep 17 00:00:00 2001
From: Satyam Zode <address@hidden>
Date: Sat, 14 Mar 2015 02:46:26 +0530
Subject: [PATCH] Added wget http test for 503 Service unavailable
---
testenv/Test-503.py | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 60 insertions(+)
create mode 100644 testenv/Test-503.py
diff --git a/testenv/Test-503.py b/testenv/Test-503.py
new file mode 100644
index 0000000..7f1c3c8
--- /dev/null
+++ b/testenv/Test-503.py
@@ -0,0 +1,60 @@
+#!/usr/bin/env python3
+from sys import exit
+from test.http_test import HTTPTest
+from misc.wget_file import WgetFile
+
+"""
+ This test ensures that Wget handles a 503 Service Unavailable response
+ correctly.
+"""
What exactly is the correct response to a 503 Service Unavailable? As far I
understand, Wget currently simply treats it like any other fatal error and moves
on.
+TEST_NAME = "503 Service Unavailable"
+############# File Definitions ###############################################
+File1 = """All happy families are alike;
+Each unhappy family is unhappy in its own way"""
+File2 = "Anyone for chocochip cookies?"
+
+File1_rules = {
+ "Response" : 503
+}
+
+A_File = WgetFile ("File1", File1, rules=File1_rules)
+B_File = WgetFile ("File2", File2)
+
Why are the two files required in this test? Is the second file required to
catch any case?
+Request_List = [
+ [
+ "GET /File1",
+ "GET /File2",
+ ]
+]
+
+
+WGET_OPTIONS = "--tries=2"
How does --tries=2 help us in testing this particular scenario. I would be
averse to complicating any test un-necessarily. Tests should be minimal so that
they test only few features as possible.
+WGET_URLS = [["File1", "File2"]]
+
+Files = [[A_File, B_File]]
+
+ExpectedReturnCode = 8
Why not simply check that Wget did not attempt to download the same file again,
returned the correct code and exit?
+ExpectedDownloadedFiles = [B_File]
I would also like to quote RFC 7231 here about the 503 response code:
The 503 (Service Unavailable) status code indicates that the server
is currently unable to handle the request due to a temporary overload
or scheduled maintenance, which will likely be alleviated after some
delay. The server MAY send a Retry-After header field
(Section 7.1.3) to suggest an appropriate amount of time for the
client to wait before retrying the request.
This is in response to Tim's questions about Wget's behavior with such
responses. Based on RFC 7231, I think Wget should parse the Retry-After header
and sleep for the specified number of seconds before retrying that file. If the
Retry-After header is not available, we must treat it as a 500 Response which is
a fatal error and skip attempting that file at all.
@Satyam: I'm mostly fine with your patch, apart from a couple of clarifications
/ modifications as outlined above. However, if you're willing to, you could
modify the test to actually send a Retry-After Header and test if Wget waits for
the specified length of time before retrying. This test should fail and we will
add it to the XFAIL_TESTS as something that we need to fix in Wget.
If you'd like to move on to something else, that is perfectly alright and we'll
consider your current patch as valid test case.
--
Thanking You,
Darshit Shah
pgppNOTzJ_rZg.pgp
Description: PGP signature