Hi Everyone,
I have just noticed that Chris has indeed provided an updated patch;
Which is in need of a review / comments etc.
I would normally place a link here - but I can;t find a convenient way to find
a message URL - so I will ping the new thread separately.
Please allow me to "steal" this one - while I am replying to ask if there is
any movement on getting message URLs appended to the bottom of mailing list
submissions?
Gavin.
On 20/01/2010, at 11:03 PM, Gavin Beau Baumanis wrote:
> 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
>>
>>
>
