Paul Burba wrote on Tue, Sep 28, 2010 at 18:06:36 -0400:
> 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.
>
:-)
> >> + 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:
I didn't actually mean check_prop() specifically, but rather the
infrastructure it uses --- main.run_command() and main.svn_binary.
> 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.
>
Aha, so you're using Popen() directly because you need to have svn's
stdout be a file and not a pipe. I'm a bit surprised that we have a bug
that's that sensitive to trigger, but okay. :-)
> >> + 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.
>
In short, "Yes" (UnorderedOutput() does ignore repetitions), but thanks
for the detailed-as-usual replies!
> Paul
Received on 2010-09-29 00:38:59 CEST