Re: [Bug-wget] Patch: Ports test for restrict-filename to python
Hi Elita,
Comments inline..
From 712ede22ec4eaf397dba32dc297ba9fd54356ee1 Mon Sep 17 00:00:00 2001
From: Elita Lobo loboel...@gmail.com
Date: Sat, 28 Mar 2015 23:07:44 +0530
Subject: [PATCH] Test-E-k.py: Tests -E -k feature of Wget
The commit message needs to state the base module too.
testenv/Test-E-k.py: Add new test
---
testenv/Test-E-k.py | 87 +
1 file changed, 87 insertions(+)
create mode 100755 testenv/Test-E-k.py
I see that the test hasn't been added to Makefile.am. Please add your tests to
the Makefile so that they are executed upon make check
+File_Rules = {
+SendHeader : {
+Content-Type : text/html
Indentation...
Subject: [PATCH] Test-restrict-ascii.py: Tests restrict-filename=ascii feature
Test-restrict-lowercase.py: Tests restrict-filename=lowercase feature
Test-restrict-uppercase.py: Tests restrict-filename=uppercase feature
Take a look at the following link for writing better commit messages:
http://chris.beams.io/posts/git-commit/
+
+This program tests that --restrict-file-names=ascii can be used to
This test tests the following feature.. Just being pedantic.
From 04e07c23d220ad0952f21e4655c4ccb7620aba48 Mon Sep 17 00:00:00 2001
From: Elita Lobo loboel...@gmail.com
Date: Sat, 28 Mar 2015 23:04:15 +0530
Subject: [PATCH] Test-start-pos--continue.py: Tests --start-pos --continue
I'm pretty sure we can come up with a better name for the test. It doesn't
always have to be the list of switches sent to Wget
Also, I think testing start-pos should be a test in itself. This test can
compliment it to ensure that the invalid switches don't work together.
From 16125a14b57b5615cd31eeadc700c21bcae00e89 Mon Sep 17 00:00:00 2001
From: Elita Lobo loboel...@gmail.com
Date: Sat, 28 Mar 2015 19:22:26 +0530
Subject: [PATCH] testenv/Test-lack-of-a-range-header-support.py: Tests Wget's
response against erroneous server response.
Again, I'm pretty sure, we can come up with a better name.
+
+Tests if Wget correctly downloads a file which previously failed to
download
+completely.
+
Not true. Your test checks if Wget correctly handles a 200 response to a valid
Range Header.
+File2_rules = {
+Response : 200,
+SendHeader: {
+Content-Length: len(File2)
Indentation
--
Thanking You,
Darshit Shah
pgpuIi06o0iln.pgp
Description: PGP signature
Re: [Bug-wget] Patch: Ports test for restrict-filename to python
Hi Elita,
Sorry, been a little busy lately and haven't been able to go through these
patches as quickly as I'd like.
From 029e78c91249cea8cc0c6d7c35991f5660f4319a Mon Sep 17 00:00:00 2001
From: Elita Lobo loboel...@gmail.com
Date: Tue, 17 Mar 2015 00:33:05 +0530
Subject: [PATCH] /testenv: Adds new testfiles to test suite which tests -E,
-k, restrict-filename, --start-pos, -c and 206 feature of Wget
This is still a very long commit message. Commit message titles should ideally
only be 60 characters. Add any information you want to the body of the commit
message. Please take a look at the other commit messages in the repository
create mode 100644 testenv/Test-206-failure.py
create mode 100644 testenv/Test-E-k.py
create mode 100644 testenv/Test-restrict-ascii.py
create mode 100644 testenv/Test-restrict-lowercase.py
create mode 100644 testenv/Test-restrict-uppercase.py
create mode 100644 testenv/Test-start-pos--continue.py
A single commit should only have logically connected code. If you're writing
tests for different areas, please split them into different commits.
Also, please set the executable bit for these files before committing them. That
allows us to directly execute a certain test without having the run the entire
suite.
diff --git a/testenv/Test-206-failure.py b/testenv/Test-206-failure.py
new file mode 100644
index 000..383ae68
--- /dev/null
+++ b/testenv/Test-206-failure.py
@@ -0,0 +1,54 @@
+#!/usr/bin/env python3
+from sys import exit
+from test.http_test import HTTPTest
+from misc.wget_file import WgetFile
+
+
+Tests if Wget correctly downloads a file which previously failed to
downloadcompletely.
+
+TEST_NAME = Test 206 failure
I think failure is too strong a word for this. All we're testing is the lack
of a Range Header support in the Server.
To truly catch corner cases, you should edit this test so that the server sends
a 206 response, but without the relevant Range Headers, or with bad Range
Headers.
+ File Definitions
#
Could you please trim this line to 79 characters? It's unnecessarily long
+File1 = This is to bring to your notice that
+File2 = This is to bring to your notice that Wget has correctly downloaded the
complete file
+
+File2_rules = {
+Response : 200,
+SendHeader: {
+Content-Length: len(File2)
+}
+}
Please be careful about indentation.
+exit(err)
diff --git a/testenv/Test-E-k.py b/testenv/Test-E-k.py
new file mode 100644
index 000..99e2a5d
--- /dev/null
+++ b/testenv/Test-E-k.py
@@ -0,0 +1,82 @@
+#!/usr/bin/env python3
+from sys import exit
+from test.http_test import HTTPTest
+from misc.wget_file import WgetFile
+
+
+Tests if Wget appends suffix .html to files of type application/xhtml+xml or
text/html when downloaded (specified by --adjust-extension or -E) and also checks if it
converts links in document to make them suitable for local viewing (specified by --convert-links or -k).
+
Again, unless necessary, please ensure that all your lines are only 80
characters long.
+
+
+Tests if Wget correctly starts downloading bytes from the given start
position (--start-pos=OFFSET) in the file. When ‘--start-pos’ and ‘
--continue’ are both specified, wget will emit a warning then proceed as if
‘--continue’ was absent.
+
+TEST_NAME = Test start-pos
+ File Definitions
#
+File1 = 12345678910
+File2 = This is an existing file
+File3 = 2345678910
+
+File1_rules = {
+SendHeader : {
+Response : 206
+}
+}
Any specific reason why you're sending a Response Header here? It seems to serve
to purpose at all.
--
Thanking You,
Darshit Shah
pgp4W2x0R3Wfm.pgp
Description: PGP signature
Re: [Bug-wget] Patch: Ports test for restrict-filename to python
Hi,
I have attached a new patch which included the old test files I have
created and 3 more additional test files along with the changes suggested
by Darshit and Zihang. Kindly review it and let me know if my patch
requires further improvements.
Thanks and Regards,
Elita Lobo
On Sun, Mar 15, 2015 at 12:18 AM, Darshit Shah dar...@gmail.com wrote:
Hi Elita,
Thanks for the contribution! It's a good set of patches that ports some
important tests to the Python Test Suite.
However, I had a couple of suggestions. Could you also please retain the
comments from the original tests about why that particular filename was
chosen? It adds a lot of value to the test when it explains some of the
background information.
Also, could you please edit your commit message to be in sync with the GNU
ChangeLog style commit messages? Take a look at the existing commit
messages for ideas.
--
Thanking You,
Darshit Shah
From e244aa2c7ccc2e5c58eecf81f57c199dbfddf3d4 Mon Sep 17 00:00:00 2001
From: Elita Lobo loboel...@gmail.com
Date: Sun, 15 Mar 2015 23:18:07 +0530
Subject: [PATCH] Ports test from old test suite to Python *
Test-restrict-lowercase.py: Tests --restrict-filename=lowercase feature *
Test-restrict-ascii.py: Tests --restrict-filename=ascii feature *
Test-restrict-uppercase.py: Tests --restrict-filename=uppercase feature
*Test-nonexisting-quiet.py: Tests Wget's quiet feature * Test-start-pos.py:
Tests Wget's --start-pos feature * Test-start-pos--continue.py Tests Wget's
--start-pos with -c feature * Makefile.am: Adds newly added testfiles to test
suite
---
testenv/Makefile.am | 8 -
testenv/Test-nonexisting-quiet.py | 48 ++
testenv/Test-restrict-ascii.py | 67 +
testenv/Test-restrict-lowercase.py | 57 +++
testenv/Test-restrict-uppercase.py | 57 +++
testenv/Test-start-pos--continue.py | 55 ++
testenv/Test-start-pos.py | 51
7 files changed, 342 insertions(+), 1 deletion(-)
create mode 100644 testenv/Test-nonexisting-quiet.py
create mode 100644 testenv/Test-restrict-ascii.py
create mode 100644 testenv/Test-restrict-lowercase.py
create mode 100644 testenv/Test-restrict-uppercase.py
create mode 100644 testenv/Test-start-pos--continue.py
create mode 100644 testenv/Test-start-pos.py
diff --git a/testenv/Makefile.am b/testenv/Makefile.am
index a4e0352..a275df2 100644
--- a/testenv/Makefile.am
+++ b/testenv/Makefile.am
@@ -53,7 +53,13 @@ if HAVE_PYTHON3
Test-Post.py\
Test-504.py \
Test--spider-r.py \
-Test-redirect-crash.py
+Test-redirect-crash.py \
+Test-restrict-ascii.py \
+Test-restrict-lowercase.py \
+Test-restrict-uppercase.py \
+Test-nonexisting-quiet.py \
+Test-start-pos.py \
+Test-start-pos--continue.py
# added test cases expected to fail here and under TESTS
XFAIL_TESTS =
diff --git a/testenv/Test-nonexisting-quiet.py b/testenv/Test-nonexisting-quiet.py
new file mode 100644
index 000..5b22d0c
--- /dev/null
+++ b/testenv/Test-nonexisting-quiet.py
@@ -0,0 +1,48 @@
+#!/usr/bin/env python3
+from sys import exit
+from test.http_test import HTTPTest
+from misc.wget_file import WgetFile
+
+
+Test for Wget nonexisting quiet. --quiet turns off Wget's output.
+
+TEST_NAME = Test nonexisting quiet
+ File Definitions #
+dummyfile = This is a dummy file
+
+A_File = WgetFile(dummy.html, dummyfile)
+
+WGET_OPTIONS = --quiet
+WGET_URLS = [[nonexistent]]
+
+Files = [[A_File]]
+
+ExpectedReturnCode = 8
+ExpectedDownloadedFiles = []
+
+### Pre and Post Test Hooks ###
+pre_test = {
+ServerFiles : Files,
+}
+test_options = {
+WgetCommands :WGET_OPTIONS,
+Urls :WGET_URLS
+}
+post_test = {
+ExpectedFiles: ExpectedDownloadedFiles,
+ExpectedRetcode : ExpectedReturnCode
+}
+
+err = HTTPTest (
+ name=TEST_NAME,
+ pre_hook=pre_test,
+ test_params=test_options,
+ post_hook=post_test
+).begin ()
+
+exit(err)
+
+
+
+
+
diff --git a/testenv/Test-restrict-ascii.py b/testenv/Test-restrict-ascii.py
new file mode 100644
index 000..c974f68
--- /dev/null
+++ b/testenv/Test-restrict-ascii.py
@@ -0,0 +1,67 @@
+#!/usr/bin/env python3
+from sys import exit
+from test.http_test import HTTPTest
+from misc.wget_file import WgetFile
+
+
+This program tests that --restrict-file-names=ascii can be used to
+ensure that all high-valued bytes are escaped. The sample filename was
+chosen because in former versions of
Re: [Bug-wget] Patch: Ports test for restrict-filename to python
Hi Elita,
I'm putting my comments inline
Subject: [PATCH] Ports test from old test suite to Python *
Test-restrict-lowercase.py: Tests --restrict-filename=lowercase feature *
Test-restrict-ascii.py: Tests --restrict-filename=ascii feature *
Test-restrict-uppercase.py: Tests --restrict-filename=uppercase feature
*Test-nonexisting-quiet.py: Tests Wget's quiet feature * Test-start-pos.py:
Tests Wget's --start-pos feature * Test-start-pos--continue.py Tests Wget's
--start-pos with -c feature * Makefile.am: Adds newly added testfiles to test
suite
That's one *long* commit message! Could you please take a look at the other
commit messages we've written and write yours accordingly?
--- /dev/null
+++ b/testenv/Test-nonexisting-quiet.py
@@ -0,0 +1,48 @@
+#!/usr/bin/env python3
+from sys import exit
+from test.http_test import HTTPTest
+from misc.wget_file import WgetFile
+
+
+Test for Wget nonexisting quiet. --quiet turns off Wget's output.
+
+TEST_NAME = Test nonexisting quiet
+ File Definitions
#
+dummyfile = This is a dummy file
+
+A_File = WgetFile(dummy.html, dummyfile)
+
+WGET_OPTIONS = --quiet
+WGET_URLS = [[nonexistent]]
+
+Files = [[A_File]]
+
+ExpectedReturnCode = 8
+ExpectedDownloadedFiles = []
+
+### Pre and Post Test Hooks ###
+pre_test = {
+ServerFiles : Files,
+}
+test_options = {
+WgetCommands :WGET_OPTIONS,
+Urls :WGET_URLS
+}
+post_test = {
+ExpectedFiles: ExpectedDownloadedFiles,
+ExpectedRetcode : ExpectedReturnCode
+}
+
+err = HTTPTest (
+ name=TEST_NAME,
+ pre_hook=pre_test,
+ test_params=test_options,
+ post_hook=post_test
+).begin ()
+
+exit(err)
+
+
+
+
+
I'm not sure what is the point of this test. A test for non-existent files I can
understand, but the we;re not really testing the -q option here. How would you
test for the quiet option?
Also, trailing newline characters at the end of the file.
diff --git a/testenv/Test-restrict-ascii.py b/testenv/Test-restrict-ascii.py
new file mode 100644
index 000..c974f68
--- /dev/null
+++ b/testenv/Test-restrict-ascii.py
@@ -0,0 +1,67 @@
+#!/usr/bin/env python3
+from sys import exit
+from test.http_test import HTTPTest
+from misc.wget_file import WgetFile
+
+
+This program tests that --restrict-file-names=ascii can be used to
+ensure that all high-valued bytes are escaped. The sample filename was
+chosen because in former versions of Wget, one could either choose not
+to escape any portion of the UTF-8 filename via
+--restrict-file-names=nocontrol (which would only be helpful if one
+was _on_ a UTF-8 system), or else Wget would escape _portions_ of
+characters, leaving irrelevant latin1-looking characters combined
+with percent-encoded control characters, instead of encoding all the
+bytes of an entire non-ASCII UTF-8 character.
+
+
+TEST_NAME = Restrict filename:ascii
+
+# gnosis in UTF-8 greek
+gnosis = %CE%B3%CE%BD%CF%89%CF%83%CE%B9%CF%82
+ File Definitions #
+
+mainpage =
+html
+head
+ titleSome Page Title/title
+/head
+body
+ p
+Some text...
+ /p
+/body
+/html
+
+A_File = WgetFile(gnosis + .html, mainpage)
+
+WGET_OPTIONS = --restrict-file-names=ascii
+WGET_URLS = [[gnosis + .html]]
+
If we're _always_ using `gnosis + .html`, why not add that string to the
variable directly?
+Files = [[A_File]]
+
+ExpectedReturnCode = 0
+ExpectedDownloadedFiles = [A_File]
+
+# Pre and Post Test hooks
+pre_test = {
+ServerFiles :Files,
+}
Just nitpicking here, but the alignment is off. It's off in other places too,
but I'll mention it only here. I realize that this test suite doesn't follow
PEP8 at all, and I would like to fix that at some point. But till then it's best
to stick to existing formatting style.
+test_options = {
+WgetCommands:WGET_OPTIONS,
+Urls:WGET_URLS
+}
+post_test = {
+ExpectedFiles : ExpectedDownloadedFiles,
+ExpectedRetcode : ExpectedReturnCode
+}
+
+err = HTTPTest (
+ name=TEST_NAME,
+ pre_hook=pre_test,
+ test_params=test_options,
+ post_hook=post_test
+).begin ()
+
+exit(err)
+
- snip -
diff --git a/testenv/Test-start-pos--continue.py
b/testenv/Test-start-pos--continue.py
new file mode 100644
index 000..dfc1a38
--- /dev/null
+++ b/testenv/Test-start-pos--continue.py
@@ -0,0 +1,55 @@
+#!/usr/bin/env python3
+from sys import exit
+from test.http_test import HTTPTest
+from misc.wget_file import WgetFile
+
+
+Tests if Wget correctly starts downloading bytes from the given start
position (--start-pos=OFFSET) in the file.
+
+TEST_NAME = Test start-pos
+ File Definitions
Re: [Bug-wget] Patch: Ports test for restrict-filename to python
Hi Elita, I don't mean to be a nitpicker, but how about: ``` filename = 'SomeFile.html' A_File = WgetFile(filename, mainpage) B_File = WgetFile(filename.lower(), mainpage) # also B_File = WgetFile(filename.upper(), mainpage) ``` The rationale is to hard-code as less as possible. I'm totally fine with the rest of the patch. Thank you! 2015-03-14 19:25 GMT+08:00 Elita Lobo loboel...@gmail.com: Hi, I have attached a patch below which ports the test files for restrict-filename to python. Kindly review it and let me know if I need to make any modifications. Thanks and Regards, Elita Lobo -- Regards, Chen Zihang, Computer School of Wuhan University --- 此致 陈子杭 武汉大学计算机学院
Re: [Bug-wget] Patch: Ports test for restrict-filename to python
Hi, Thanks for the suggestion. Will make the required corrections and submit it soon. Thanks and Regards, Elita Lobo On Sat, Mar 14, 2015 at 10:05 PM, 陈子杭 (Zihang Chen) chsc4...@gmail.com wrote: Hi Elita, I don't mean to be a nitpicker, but how about: ``` filename = 'SomeFile.html' A_File = WgetFile(filename, mainpage) B_File = WgetFile(filename.lower(), mainpage) # also B_File = WgetFile(filename.upper(), mainpage) ``` The rationale is to hard-code as less as possible. I'm totally fine with the rest of the patch. Thank you! 2015-03-14 19:25 GMT+08:00 Elita Lobo loboel...@gmail.com: Hi, I have attached a patch below which ports the test files for restrict-filename to python. Kindly review it and let me know if I need to make any modifications. Thanks and Regards, Elita Lobo -- Regards, Chen Zihang, Computer School of Wuhan University --- 此致 陈子杭 武汉大学计算机学院
