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

[Gilles Moris]

On Thursday 10 December 2009 09:28:42 am Nicolas Dumazet wrote:
> # HG changeset patch
> # User Nicolas Dumazet <nicdumz.commits at gmail.com>
> # Date 1260433291 -32400
> # Node ID 5f69e6203b4d171b467415249ad1185b1dd737b0
> # Parent  2b630e4c8f2f626f1e5d0f88646463968860f8ac
> run-tests: split tests/blacklist in tests/blacklists/*
>
> Following discussions with Gilles Morris [1], it seems that it is
> preferable to use several blacklist files in a blacklists/ directory. It is
> easier to add an unversioned file for experiments than modifying a tracked
> file.
>
> Also falling back to a simpler syntax, giving up ConfigParser, now that
> section names are not needed anymore.
>
>
> [1]
> http://www.selenic.com/pipermail/mercurial-devel/2009-December/017317.html
>

Thanks for looking at this.

[...]

> diff --git a/tests/run-tests.py b/tests/run-tests.py
> --- a/tests/run-tests.py
> +++ b/tests/run-tests.py
> @@ -41,7 +41,6 @@
>  # completes fairly quickly, includes both shell and Python scripts, and
>  # includes some scripts that run daemon processes.)
>
> -from ConfigParser import ConfigParser
>  import difflib
>  import errno
>  import optparse
> @@ -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.

>      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".
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.

> +            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.

Regards.
Gilles.

> +
>          options.blacklist = blacklist
>
>      return (options, args)
> @@ -744,9 +751,9 @@
>
>          for test in tests:
>              if options.blacklist:
> -                section = options.blacklist.get(test)
> -                if section is not None:
> -                    skips.append((test, "blacklisted (%s section)" %
> section)) +                filename = options.blacklist.get(test)
> +                if filename is not None:
> +                    skips.append((test, "blacklisted (%s)" % filename))
>                      skipped += 1
>                      continue
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel