Re: [Bug-wget] Patch: Ports test for restrict-filename to python

[Darshit Shah]

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 <address@hidden>
> 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 0000000..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 
> download    completely.
> +"""
> +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 0000000..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

pgph9xDiz7cLm.pgp
Description: PGP signature