Hi Chris,
Just thought I would chase you up with this.
Were you going to make the changes suggested by Julian - or were you happy for him to "tidy it up" and commit it?
Gavin "Beau" Baumanis
On 04/01/2010, at 9:15 PM, Julian Foad wrote:
> 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-20 13:12:03 CET