Hi Daniel, Daniel Shahaf writes: > Good start. But you don't have a first sentence before the bullets (which we > want for most non-small changes), and you aren't using the standard > (function_name) > syntax. So, perhaps along these lines: > > [[[ > Add a unit test for svnrdump dumping r0. > > * subversion/tests/cmdline/svnrdump_tests_data: Add a new directory to > keep test data for svnrdump. > > * subversion/tests/cmdline/svnrdump_tests_data/asf-0.dump: Add a > dumpfile of the zeroth revision of the ASF repository. > > * subversion/tests/cmdline/svnrdump_tests.py: > (build_repos, run_test): New helper functions. ("Factored out from > XXX()", if applicable) > (asf_0): New test ("for XXX", if you like). > (test_list): Run the new test. > ]]]
Right. I'm slowly getting used to it :) I'll commit it now. > First of all, please don't verify a 100k dumpfile during every 'make check'. > It would take too long. Ah, I didn't think about that. > Second, is it really necessary to have all of these? I'd assume that all edge > cases you need to test can be fitted quite roomly in a 50-revision dumpfile. Do we already have a dumpfile with all the possible edge cases somewhere in the tree? I'll rename asf-0 as a generic zeroth-revision test then; note that I've written special code for handling revision 0, so it makes sense to test it. > (Specifically, I think that having a 10k and 100k tests is unnecessary given > that you have a 1k test; and that the 1k test in itself could be much reduced. > It's not the number of revisions that counts!) Got it. -- Ram