Re: [Bug-wget] [PATCH 01/25] Add two Metalink/XML tests

[Matthew White]

On Sun, 11 Sep 2016 21:52:16 +0200
Giuseppe Scrivano <address@hidden> wrote:

> Hi Matthew,
> 
> thanks for all your work, I am going to comment on each patch separately:
> 
> Matthew White <address@hidden> writes:
> 
> 
> > index 0000000..041d772
> > --- /dev/null
> > +++ b/testenv/Test-metalink-xml-relpath.py
> > @@ -0,0 +1,87 @@
> > +#!/usr/bin/env python3
> > +from sys import exit
> > +from test.http_test import HTTPTest
> > +from misc.wget_file import WgetFile
> > +import re
> > +import hashlib
> > +
> > +"""
> > +    This is to test if Metalink XML file escapes current directory.
> > +"""
> > +############# File Definitions 
> > ###############################################
> > +File1 = "Would you like some Tea?"
> > +File1_lowPref = "Do not take this"
> > +File1_sha256 = hashlib.sha256 (File1.encode ('UTF-8')).hexdigest ()
> > +MetaXml = \
> > +"""<?xml version="1.0" encoding="utf-8"?>
> > +<metalink version="3.0" xmlns="http://www.metalinker.org/";>
> > +  <publisher>
> > +    <name>GNU Wget</name>
> > +  </publisher>
> > +  <license>
> > +    <name>GNU GPL</name>
> > +    <url>http://www.gnu.org/licenses/gpl.html</url>
> > +  </license>
> > +  <identity>Wget Test File 1</identity>
> > +  <version>1.2.3</version>
> > +  <description>Wget Test File 1 description</description>
> > +  <files>
> > +    <file name="../File1">
> 
> If the difference between the two files is only here, could we have only
> one test file and another template substition for file name?  Somethging
> like <file name="{{FILE}}">?

Next patches add more conditions to testenv/Test-metalink-xml-relpath.py.

Also variants are introduced, like:
* testenv/Test-metalink-xml-trust.py
* testenv/Test-metalink-xml-homepath.py
* testenv/Test-metalink-xml-homepath-trust.py
* testenv/Test-metalink-xml-abspath-trust.py
* testenv/Test-metalink-xml-relpath-trust.py

And there will be other tests with other meanings too.

So, I don't know if it's a good idea to mix specific tests together.

This topic also applies to Patch 05/25, Patch 10/25, and Patch 17/25:
http://lists.gnu.org/archive/html/bug-wget/2016-09/msg00046.html
http://lists.gnu.org/archive/html/bug-wget/2016-09/msg00050.html
http://lists.gnu.org/archive/html/bug-wget/2016-09/msg00057.html

WDYT?

> 
> 
> > +MetaXml = re.sub (r'{{FILE1_HASH}}', File1_sha256, MetaXml)
> > +MetaXml = re.sub (r'{{SRV_HOST}}', srv_host, MetaXml)
> > +MetaXml = re.sub (r'{{SRV_PORT}}', str (srv_port), MetaXml)
> > +MetaFile.content = MetaXml
> 
> should be enough to use replace instead of the re module?
> 
> MetaXml = MetaXml.replace('{{FILE1_HASH}}', File1_sha256)

Fixed. Posting after final decisions are taken about open topics in this series 
of patches.

> 
> Thanks,
> Giuseppe

Regards,
Matthew

-- 
Matthew White <address@hidden>

pgpe2XN0Ai5UY.pgp
Description: PGP signature