[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Bug-wget] Patch: Ports test for restrict-filename to python
|
From: |
Darshit Shah |
|
Subject: |
Re: [Bug-wget] Patch: Ports test for restrict-filename to python |
|
Date: |
Mon, 16 Mar 2015 00:17:09 +0530 |
|
User-agent: |
Mutt/1.5.23 (2014-03-12) |
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 0000000..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>
+ <title>Some 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 0000000..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
#############################################################################
+File1 = "12345678910"
+File2 = "This is an existing file"
+File3 = "2345678910"
+
+File1_rules = {
+ "SendHeader" : {
+ "Response" : 206
+ }
+}
What is this header needed for?
+
+A_File = WgetFile("somefile.txt", File1, rules=File1_rules)
+B_File = WgetFile("somefile.txt", File2)
+C_File = WgetFile("somefile.txt.1", File3)
+
+WGET_OPTIONS = "--start-pos=1 --continue --debug"
--debug options are always added to Wget. You don't need to specify it again.
Could you also add a comment at the top of the file explaining how this test
also tests that --continue and --start-pos don't work together?
+"""
+ 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
#############################################################################
+File1 = "12345678910"
+File2 = "2345678910"
+
+File1_rules = {
+ "SendHeader" : {
+ "Response" : 206
+ }
+}
+
+A_File = WgetFile("File1", File1, rules=File1_rules)
+B_File = WgetFile("File1", File2)
+
+WGET_OPTIONS = "--start-pos=1"
+WGET_URLS = [["File1"]]
+
+Files = [[A_File]]
+
+ExpectedReturnCode = 0
+ExpectedDownloadedFiles = [B_File]
+
I think this test is only a subset of the previous one and hence a bit
redundant. What do you think?
Sorry if I come across as being a little too mean, but I'm a bit picky when it
comes to well formatted commits. Zihang can tell more about that. :)
--
Thanking You,
Darshit Shah
pgp5S91q6BGwM.pgp
Description: PGP signature