Hi Daniel,
Comments inline. All fixes mentioned were done in r1002372.
On Tue, Sep 28, 2010 at 3:30 PM, Daniel Shahaf <d.s_at_daniel.shahaf.name> wrote:
> pburba_at_apache.org wrote on Tue, Sep 28, 2010 at 18:38:19 -0000:
>> + # Run propget -vR svn:mergeinfo and collect the stdout.
>> + exit_code, pg_stdout, pg_stderr = svntest.actions.run_and_verify_svn(
>> + None, None, [], 'propget', SVN_PROP_MERGEINFO, '-vR', wc_dir)
>> +
>
> exit_code and pg_stderr aren't checked anywhere.
Neither is pg_stdout...but that entire statement was cruft from an
earlier version of the test; removed it.
>> + # Run propget -vR svn:mergeinfo, redirecting the stdout to a file.
>> + arglist = ['svn.exe', 'propget', SVN_PROP_MERGEINFO, '-vR', wc_dir]
>
> s/.exe// ?
Actually that should ideally be 'svntest.main.svn_binary'. Fixed that.
>> + redir_file = open(redirect_file, 'wb')
>> + pg_proc = subprocess.Popen(arglist, stdout=redir_file)
>
> Shouldn't this use the svntest/ infrastructure? Compare
> svntest.actions.check_prop().
I didn't use it for two reasons:
First, svntest.actions.check_prop() only supports finding the props on
a single path (and as far as I can tell that works fine, no issue
#3721).
Second, Issue #3721 only occurs when the output of 'svn pg -vR' is
*redirected to a file* - see
http://subversion.tigris.org/issues/show_bug.cgi?id=3721#desc1.
check_prop() is (obviously) all processes and pipes underneath the
covers, so while it may also be possible to show the bug using it, I
wrote the test to hew as closely to the actual bug I witnessed as
possible.
>> + pg_proc.wait()
>> + redir_file.close()
>> + pg_stdout_redir = open(redirect_file, 'r').readlines()
>> +
>> + # Check if the redirected output of svn pg -vR is what we expect.
>> + #
>> + # Currently this fails because the mergeinfo for the three paths is
>> + # interleaved and the lines endings are (at least on Windows) a mix
>> + # of <CR><LF> and <LF>. See
>> + # http://subversion.tigris.org/issues/show_bug.cgi?id=3721#desc1
>> + unordered_expected_output = svntest.verify.UnorderedOutput([
>> + "Properties on '" + B_path + "':\n",
>> + "Properties on '" + C_path + "':\n",
>> + "Properties on '" + D_path + "':\n",
>> + " svn:mergeinfo\n",
>> + " /subversion/branches/1.5.x:872364-874936\n",
>> + " /subversion/branches/1.5.x-34184:874657-874741\n",
>> + " /subversion/branches/1.5.x-34432:874744-874798\n",
>
> So, 'unordered' also ignores repetitions? (since the last 4 lines
> appear only once each, rather than three times each)
I think you mean the first 3 lines appear only once, all the other
lines appear 3 times each (because the test sets the same mergeinfo on
all three paths and the expected output is for svn pg -v***R***).
But yeah, this test isn't perfect as it would allow repetitions. That
is the price we pay when using svntest.verify.UnorderedOutput(), which
is required here because there is no guarantee as to the order in
which svn pg -vR will report the three paths. I tweaked the test to
check the expected number of lines in the actual redirected output, so
that we catch any dups.
Paul
Received on 2010-09-29 00:07:14 CEST