On Wed, 2009-12-09, Chris Foote wrote:
> I have incorporated both of Bert and Daniels suggestions into the
> patch (attached).
Thanks Chris. Functionally it looks good. Just some mainly cosmetic
comments below. (One's actually bug but evidently in a bit of code that
doesn't matter much in practice.)
I can make those changes myself and commit it if you like. Would that be
OK?
> This version of the patch adds the ability to specify one or more
> tests (and test numbers) to run on Windows.
>
> The --test option takes the name of the test executable (with or
> without the "-test.exe"/"_tests.py" part). It can also take test
> number(s) to be run in those tests by appending #NUM to the test
> name.
>
> e.g. win-tests.py --release --test=basic_tests.py -t client
> win-tests.py --release --test=basic_tests.py#4 -t client#2,4-6
>
> Regards
> Chris
>
> [[[
> Make it easy to (re)run specific tests on windows by adding a --test/-t
> option. The tests can also specify specific test number(s) to run.
> * win-tests.py
> (_usage_exit): Add the --test/-t option to the help.
> (tests_to_run): New. List of tests to be run by the TestHarness.
> * build/run_tests.py
> (_run_test): Break the progname at a '#' and use the rhs as a list of
> test numbers to run.
> ]]]
> Index: win-tests.py
> ===================================================================
> --- win-tests.py (revision 888785)
> +++ win-tests.py (working copy)
> @@ -61,6 +61,9 @@
> print(" -v, --verbose : talk more")
> print(" -f, --fs-type=type : filesystem type to use (fsfs is default)")
> print(" -c, --cleanup : cleanup after running a test")
> + print(" -t, --test=TEST : Run the TEST test (all is default); use")
> + print(" TEST#n to run a particular test number,")
> + print(" multiples also accepted e.g. '2,4-7'")
>
> print(" --svnserve-args=list : comma-separated list of arguments for")
> print(" svnserve")
> @@ -107,9 +110,9 @@
> dll_basename = section.name + "-" + str(gen_obj.version) + ".dll"
> svn_dlls.append(os.path.join("subversion", section.name, dll_basename))
>
> -opts, args = my_getopt(sys.argv[1:], 'hrdvcpu:f:',
> - ['release', 'debug', 'verbose', 'cleanup', 'url=',
> - 'svnserve-args=', 'fs-type=', 'asp.net-hack',
> +opts, args = my_getopt(sys.argv[1:], 'hrdvct:pu:f:',
> + ['release', 'debug', 'verbose', 'cleanup', 'test=',
> + 'url=', 'svnserve-args=', 'fs-type=', 'asp.net-hack',
> 'httpd-dir=', 'httpd-port=', 'httpd-daemon',
> 'httpd-server', 'http-library=', 'help',
> 'fsfs-packing', 'fsfs-sharding=',
> @@ -137,6 +140,7 @@
> fsfs_packing = None
> server_minor_version = None
> config_file = None
> +tests_to_run = []
>
> for opt, val in opts:
> if opt in ('-h', '--help'):
> @@ -149,6 +153,8 @@
> verbose = 1
> elif opt in ('-c', '--cleanup'):
> cleanup = 1
> + elif opt in ('-t', '--test'):
> + tests_to_run.append(val)
> elif opt in ['-r', '--release']:
> objdir = 'Release'
> elif opt in ['-d', '--debug']:
> @@ -599,6 +605,31 @@
> if daemon:
> daemon.start()
>
> +# If selected tests specified, only run them
The block of code introduced by this comment is not deciding which tests
to run, it is finding the full path and filename to any tests that were
specified as just a base name. (It took me a little while to read
through it and discover that, which shows how helpful a comment could
be.) So maybe:
# Find the full path and filename of any test that is specified just by
its base name.
> +if len(tests_to_run) != 0:
> + tests = []
> + for t in tests_to_run:
> + tns = None
> + if '#' in t:
> + t, tns = t.split('#')
> +
> + test = [x for x in all_tests if x.split('/')[-1] == t]
> + if not test and (len(t) < 9 or
> + (t[-9] != '-test.exe' and t[-9] != '_tests.py')):
That "t[-9] != ..." will always be true. I think you meant "t[-9:] !
= ...". But why not lose all the "9"s and just write
if not test and not t.endswith('-test.exe') and not
t.endswith('_tests.py'):
instead?
> + test = [x for x in all_tests if x.split('/')[-1][:-9] == t]
I can't think of a really simple way to lose the "-9" here, so comment
it:
# The lengths of '-test.exe' and of '_tests.py' are both 9.
> +
> + if not test:
> + print("Skipping test '%s', test not found." % t)
> + elif tns:
> + tests.append('%s#%s' % (test[0], tns))
> + else:
> + tests.extend(test)
> +
> + tests_to_run = tests
> +else:
> + tests_to_run = all_tests
> +
> +
> print('Testing %s configuration on %s' % (objdir, repo_loc))
> sys.path.insert(0, os.path.join(abs_srcdir, 'build'))
> import run_tests
> @@ -612,7 +643,7 @@
> old_cwd = os.getcwd()
> try:
> os.chdir(abs_builddir)
> - failed = th.run(all_tests)
> + failed = th.run(tests_to_run)
> except:
> os.chdir(old_cwd)
> raise
> Index: build/run_tests.py
> ===================================================================
> --- build/run_tests.py (revision 888785)
> +++ build/run_tests.py (working copy)
> @@ -216,12 +216,22 @@
> else:
> log = sys.stdout
>
> + test_nums = None
> + if '#' in prog:
> + prog, test_nums = prog.split('#')
> +
> progdir, progbase = os.path.split(prog)
> if self.log:
> # Using write here because we don't want even a trailing space
> test_info = '%s [%d/%d]' % (progbase, test_nr + 1, total_tests)
> - sys.stdout.write('Running all tests in %s' % (test_info, ))
> - sys.stdout.write('.'*(35 - len(test_info)))
> + if test_nums:
> + p = [x for x in ',-:' if x in test_nums] and 's' or ''
> + sys.stdout.write('Running test%s %s in %s' % (p, test_nums, test_info))
> + sys.stdout.write('.'*(44 - len(p) - len(test_info) - len(test_nums)))
> + test_nums = test_nums.split(',')
> + else:
> + sys.stdout.write('Running all tests in %s' % (test_info, ))
> + sys.stdout.write('.'*(40 - len(test_info)))
I don't think we need all this effort to try to print a neat one-line
summary that fits in a fixed-width column. It will still overflow,
depending on what list of test numbers I specify. Let's just change the
original "Running all tests in ..." to "Running tests in ..." and leave
it at that.
> log.write('START: %s\n' % progbase)
> log.flush()
> @@ -268,6 +278,9 @@
> if self.fsfs_packing is not None:
> cmdline.append('--fsfs-packing')
>
> + if test_nums:
> + cmdline.extend(test_nums)
> +
> old_cwd = os.getcwd()
> try:
> os.chdir(progdir)
- Julian
Received on 2010-01-04 11:16:34 CET