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

Reply via email to