[PATCH] run-tests: split tests/blacklist in tests/blacklists/*

Nicolas Dumazet nicdumz at gmail.com
Wed Dec 16 22:29:12 CST 2009


2009/12/13 Gilles Moris <gilles.moris at free.fr>:
> On Thursday 10 December 2009 09:28:42 am Nicolas Dumazet wrote:

[...]

>> @@ -134,8 +133,7 @@
>>      parser.add_option("--inotify", action="store_true",
>>          help="enable inotify extension when running tests")
>>      parser.add_option("--blacklist", action="append",
>> -        help="skip tests listed in the specified section of "
>> -             "the blacklist file")
>> +        help="skip tests listed in the specified blacklist")
>>
>
> help="skip tests listed in the specified blacklist file")
> I would keep the *file* word here.

ok.

>
>>      for option, default in defaults.items():
>>          defaults[option] = int(os.environ.get(*default))
>> @@ -202,12 +200,21 @@
>>          if sys.version_info[:2] < (2, 6) or sys.version_info[:2] >= (3,
>> 0): parser.error('--py3k-warnings can only be used on Python 2.6+') if
>> options.blacklist:
>> -        configparser = ConfigParser()
>> -        configparser.read("blacklist")
>>          blacklist = dict()
>> -        for section in options.blacklist:
>> -            for (item, value) in configparser.items(section):
>> -                blacklist["test-" + item] = section
>> +        for filename in options.blacklist:
>> +            try:
>> +                f = open(os.path.join("blacklist", filename), "r")
>
> You adopted the plural for the directory, so it should be "blacklists".

Interesting. I wonder how this patch made it through.

> I would even not do the join and take the filename litterally. With shell
> completion, it's even easier to enter quickly an existing path inside the
> blacklist directory, and with the completion, you are sure the path exists.
> This simplify the code and you are not forced to put your files here.

Good idea! Completion makes my life easier.

>
>> +            except IOError, err:
>> +                if err.errno != errno.ENOENT:
>> +                    raise
>> +                print "warning: no '%s' blacklist could be found" %
>> filename +                continue
>> +
>> +            for line in f.readlines():
>> +                line = line.strip()
>> +                if line and not line.startswith('#'):
>> +                    blacklist["test-" + line] = filename
>
> Same thing here for the tests. I don't think it's very useful to strip
> the 'test-' prefix.
> If I want to quickly discard all convert tests, I could just do:
> ls test-convert-* | fgrep -v .out > my-convert-blacklist
> without having to use 'sed' to strip the prefix.
>
> Except if you have other workflow or usages in mind.

I don't particularly care. As long as in the end I have a way to flag
some tests as "bad" for inotify, any solution is good for me.
So I'm gladly including your suggestions.

Resending..!

>
> Regards.
> Gilles.


-- 
Nicolas Dumazet — NicDumZ


More information about the Mercurial-devel mailing list