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
Received on 2010-07-24 10:09:53 CEST